From d14bb7e91e0dfe12b09165b02b74fab6c884417b Mon Sep 17 00:00:00 2001 From: Dan Klishch Date: Sun, 21 Jan 2024 13:51:05 -0500 Subject: [PATCH] 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. --- .../CodeGenerators/JSSpecCompiler/AST/AST.h | 3 -- .../JSSpecCompiler/AST/ASTPrinting.cpp | 1 - .../JSSpecCompiler/CMakeLists.txt | 1 - .../FunctionCallCanonicalizationPass.cpp | 38 --------------- .../Passes/FunctionCallCanonicalizationPass.h | 28 ----------- .../JSSpecCompiler/Parser/TextParser.cpp | 48 ++++++++++++++----- .../JSSpecCompiler/Parser/TextParser.h | 1 + .../JSSpecCompiler/Parser/Token.h | 1 - .../Tests/simple.cpp.expectation | 28 ----------- .../CodeGenerators/JSSpecCompiler/main.cpp | 2 - 10 files changed, 37 insertions(+), 114 deletions(-) delete mode 100644 Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp delete mode 100644 Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/AST.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/AST.h index cf053a8bf8..e5d473b2aa 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/AST.h +++ b/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(); -inline Tree const zero_argument_function_call = make_ref_counted(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) \ diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/ASTPrinting.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/ASTPrinting.cpp index bb2dc687d2..6647c15ed7 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/AST/ASTPrinting.cpp +++ b/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]); } diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/CMakeLists.txt b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/CMakeLists.txt index a442a0bb5e..24cf938865 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/CMakeLists.txt +++ b/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 diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp deleted file mode 100644 index 7aa00d5ee6..0000000000 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.cpp +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (c) 2023, Dan Klishch - * - * 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(tree)) { - if (binary_operation->m_operation == BinaryOperator::FunctionCall) { - Vector arguments; - - auto current_tree = binary_operation->m_right; - while (true) { - auto argument_tree = as(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(binary_operation->m_left, move(arguments))); - } - } -} - -} diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h deleted file mode 100644 index ccaecdbf0c..0000000000 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Compiler/Passes/FunctionCallCanonicalizationPass.h +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright (c) 2023, Dan Klishch - * - * 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; -}; - -} diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.cpp index ca2f1ee220..51718cade4 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.cpp @@ -146,6 +146,30 @@ TextParseErrorOr TextParser::parse_record_direct_list_initialization() make_ref_counted(identifier.data), move(arguments)); } +// :== '(' ( (, )* )? ')' +TextParseErrorOr> 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 {}; + } + + Vector 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; +} + // TextParseErrorOr TextParser::parse_expression() { @@ -222,6 +246,7 @@ TextParseErrorOr 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 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(stack.take_last().get(), 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 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()); diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.h index 41d4d38410..72d04de6b5 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.h +++ b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/TextParser.h @@ -72,6 +72,7 @@ private: TextParseErrorOr expect_eof(); TextParseErrorOr parse_record_direct_list_initialization(); + TextParseErrorOr> parse_function_arguments(); TextParseErrorOr parse_expression(); TextParseErrorOr parse_condition(); TextParseErrorOr parse_return_statement(); diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Token.h b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Token.h index 5499a546ad..bd25bb99eb 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Parser/Token.h +++ b/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") \ diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/simple.cpp.expectation b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/simple.cpp.expectation index 29c218c0f0..1bff94b123 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/Tests/simple.cpp.expectation +++ b/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 diff --git a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/main.cpp b/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/main.cpp index 5bd0fb647c..950a239037 100644 --- a/Meta/Lagom/Tools/CodeGenerators/JSSpecCompiler/main.cpp +++ b/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 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(); pipeline.add_compilation_pass(); pipeline.add_compilation_pass(); pipeline.add_compilation_pass();