JSSpecCompiler: Make function arguments parsing much simpler

In a nutshell, when we understand that the expression is a function
call (this happens at '(' after an expression), stop parsing expression
and start parsing function arguments. This makes
`FunctionCallCanonicalizationPass` and the workaround for zero argument
function calls obsolete.

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/AST.h

@@ -139,7 +139,6 @@ public:
         This,
         True,
         Undefined,
-        ZeroArgumentFunctionCall,
         // Update WellKnownNode::dump_tree after adding an entry here
     };
 
@@ -156,7 +155,6 @@ private:
 };
 
 inline Tree const error_tree = make_ref_counted<ErrorNode>();
-inline Tree const zero_argument_function_call = make_ref_counted<WellKnownNode>(WellKnownNode::ZeroArgumentFunctionCall);
 
 class ControlFlowFunctionReturn : public ControlFlowOperator {
 public:
@@ -256,7 +254,6 @@ protected:
     F(CompareNotEqual)                \
     F(Declaration)                    \
     F(Division)                       \
-    F(FunctionCall)                   \
     F(MemberAccess)                   \
     F(Minus)                          \
     F(Multiplication)                 \

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/ASTPrinting.cpp

@@ -43,7 +43,6 @@ void WellKnownNode::dump_tree(StringBuilder& builder)
         "This"sv,
         "True"sv,
         "Undefined"sv,
-        "ZeroArgumentFunctionCall"sv,
     };
     dump_node(builder, "WellKnownNode {}", type_to_name[m_type]);
 }

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/CMakeLists.txt

@@ -7,7 +7,6 @@ set(SOURCES
     Compiler/Passes/CFGBuildingPass.cpp
     Compiler/Passes/CFGSimplificationPass.cpp
     Compiler/Passes/DeadCodeEliminationPass.cpp
-    Compiler/Passes/FunctionCallCanonicalizationPass.cpp
     Compiler/Passes/IfBranchMergingPass.cpp
     Compiler/Passes/ReferenceResolvingPass.cpp
     Compiler/Passes/SSABuildingPass.cpp

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp

@@ -1,38 +0,0 @@
-/*
- * Copyright (c) 2023, Dan Klishch <danilklishch@gmail.com>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include "Compiler/Passes/FunctionCallCanonicalizationPass.h"
-#include "AST/AST.h"
-
-namespace JSSpecCompiler {
-
-void FunctionCallCanonicalizationPass::on_leave(Tree tree)
-{
-    if (auto binary_operation = as<BinaryOperation>(tree)) {
-        if (binary_operation->m_operation == BinaryOperator::FunctionCall) {
-            Vector<Tree> arguments;
-
-            auto current_tree = binary_operation->m_right;
-            while (true) {
-                auto argument_tree = as<BinaryOperation>(current_tree);
-                if (!argument_tree || argument_tree->m_operation != BinaryOperator::Comma)
-                    break;
-                arguments.append(argument_tree->m_left);
-                current_tree = argument_tree->m_right;
-            }
-            arguments.append(current_tree);
-
-            if (arguments[0] == zero_argument_function_call) {
-                VERIFY(arguments.size() == 1);
-                arguments.clear();
-            }
-
-            replace_current_node_with(make_ref_counted<FunctionCall>(binary_operation->m_left, move(arguments)));
-        }
-    }
-}
-
-}

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h

@@ -1,28 +0,0 @@
-/*
- * Copyright (c) 2023, Dan Klishch <danilklishch@gmail.com>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include "Compiler/GenericASTPass.h"
-
-namespace JSSpecCompiler {
-
-// FunctionCallCanonicalizationPass simplifies ladders of BinaryOperators nodes in the function call
-// arguments into nice and neat FunctionCall nodes.
-//
-// Ladders initially appear since I do not want to complicate expression parser, so it interprets
-// `f(a, b, c, d)` as `f "function_call_operator" (a, (b, (c, d))))`.
-class FunctionCallCanonicalizationPass : public GenericASTPass {
-public:
-    inline static constexpr StringView name = "function-call-canonicalization"sv;
-
-    using GenericASTPass::GenericASTPass;
-
-protected:
-    void on_leave(Tree tree) override;
-};
-
-}

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.cpp

@@ -146,6 +146,30 @@ TextParseErrorOr<Tree> TextParser::parse_record_direct_list_initialization()
         make_ref_counted<UnresolvedReference>(identifier.data), move(arguments));
 }
 
+// <function_arguments> :== '(' (<expr> (, <expr>)* )? ')'
+TextParseErrorOr<Vector<Tree>> TextParser::parse_function_arguments()
+{
+    auto rollback = rollback_point();
+
+    TRY(consume_token_with_type(TokenType::ParenOpen));
+
+    if (!consume_token_with_type(TokenType::ParenClose).is_error()) {
+        rollback.disarm();
+        return Vector<Tree> {};
+    }
+
+    Vector<Tree> arguments;
+    while (true) {
+        arguments.append(TRY(parse_expression()));
+
+        auto token = TRY(consume_token_with_one_of_types({ TokenType::ParenClose, TokenType::Comma }));
+        if (token.type == TokenType::ParenClose)
+            break;
+    }
+    rollback.disarm();
+    return arguments;
+}
+
 // <expr>
 TextParseErrorOr<Tree> TextParser::parse_expression()
 {
@@ -222,6 +246,7 @@ TextParseErrorOr<Tree> TextParser::parse_expression()
         if (!token_or_error.has_value())
             break;
         auto token = token_or_error.release_value();
+        bool is_consumed = false;
 
         enum {
             NoneType,
@@ -261,17 +286,15 @@ TextParseErrorOr<Tree> TextParser::parse_expression()
             break;
 
         if (token.type == TokenType::ParenOpen) {
-            if (last_element_type == ExpressionType)
-                stack.append(Token { TokenType::FunctionCall, ""sv, token.location });
-            stack.append(token);
-
-            if (m_next_token_index + 1 < m_tokens.size()
-                && m_tokens[m_next_token_index + 1].type == TokenType::ParenClose) {
-                // This is a call to function which does not take parameters. We cannot handle it
-                // normally since we need text between parenthesis to be a valid expression. As a
-                // workaround, we push an artificial tree to stack to act as an argument (it'll be
-                // removed later during function call canonicalization).
-                stack.append(zero_argument_function_call);
+            if (last_element_type == ExpressionType) {
+                // This is a function call.
+                auto arguments = TRY(parse_function_arguments());
+                is_consumed = true;
+                stack.append(Tree { make_ref_counted<FunctionCall>(stack.take_last().get<Tree>(), move(arguments)) });
+                --bracket_balance;
+            } else {
+                // This is just an opening '(' in expression.
+                stack.append(token);
             }
         } else if (token.is_pre_merged_binary_operator()) {
             THROW_PARSE_ERROR_IF(last_element_type != ExpressionType);
@@ -330,7 +353,8 @@ TextParseErrorOr<Tree> TextParser::parse_expression()
             merge_pre_merged();
         }
 
-        VERIFY(consume_token().has_value());
+        if (!is_consumed)
+            VERIFY(consume_token().has_value());
     }
 
     THROW_PARSE_ERROR_IF(stack.is_empty());

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.h

@@ -72,6 +72,7 @@ private:
     TextParseErrorOr<void> expect_eof();
 
     TextParseErrorOr<Tree> parse_record_direct_list_initialization();
+    TextParseErrorOr<Vector<Tree>> parse_function_arguments();
     TextParseErrorOr<Tree> parse_expression();
     TextParseErrorOr<Tree> parse_condition();
     TextParseErrorOr<Tree> parse_return_statement();

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Token.h

@@ -33,7 +33,6 @@ constexpr i32 closing_bracket_precedence = 18;
     F(Enumerator, -1, Invalid, Invalid, Invalid, "enumerator")                       \
     F(Equals, 10, Invalid, CompareEqual, Invalid, "equals")                          \
     F(ExclamationMark, 3, AssertCompletion, Invalid, Invalid, "exclamation mark")    \
-    F(FunctionCall, 2, Invalid, FunctionCall, Invalid, "function call token")        \
     F(Greater, 9, Invalid, CompareGreater, Invalid, "greater than")                  \
     F(Identifier, -1, Invalid, Invalid, Invalid, "identifier")                       \
     F(Is, -1, Invalid, Invalid, Invalid, "operator is")                              \

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/simple.cpp.expectation

@@ -26,34 +26,6 @@ TreeList
   ReturnNode
     UnresolvedReference b
 
-===== AST after function-call-canonicalization =====
-f(cond1, cond2):
-TreeList
-  IfBranch
-    UnresolvedReference cond1
-    TreeList
-      BinaryOperation Declaration
-        UnresolvedReference a
-        MathematicalConstant 1
-      IfBranch
-        UnresolvedReference cond2
-        TreeList
-          BinaryOperation Declaration
-            UnresolvedReference b
-            UnresolvedReference a
-      ElseIfBranch Else
-        TreeList
-          BinaryOperation Declaration
-            UnresolvedReference b
-            MathematicalConstant 3
-  ElseIfBranch Else
-    TreeList
-      BinaryOperation Declaration
-        UnresolvedReference b
-        MathematicalConstant 4
-  ReturnNode
-    UnresolvedReference b
-
 ===== AST after if-branch-merging =====
 f(cond1, cond2):
 TreeList

Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/main.cpp

@@ -11,7 +11,6 @@
 #include "Compiler/Passes/CFGBuildingPass.h"
 #include "Compiler/Passes/CFGSimplificationPass.h"
 #include "Compiler/Passes/DeadCodeEliminationPass.h"
-#include "Compiler/Passes/FunctionCallCanonicalizationPass.h"
 #include "Compiler/Passes/IfBranchMergingPass.h"
 #include "Compiler/Passes/ReferenceResolvingPass.h"
 #include "Compiler/Passes/SSABuildingPass.h"
@@ -118,7 +117,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         pipeline.add_step(adopt_own_if_nonnull(new CppParsingStep()));
     else
         pipeline.add_step(adopt_own_if_nonnull(new SpecParsingStep()));
-    pipeline.add_compilation_pass<FunctionCallCanonicalizationPass>();
     pipeline.add_compilation_pass<IfBranchMergingPass>();
     pipeline.add_compilation_pass<ReferenceResolvingPass>();
     pipeline.add_compilation_pass<CFGBuildingPass>();