JSSpecCompiler: Store arguments in declaration instead of definition

And create a struct encapsulating argument name in the preparation for
argument types and optional arguments.

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

@@ -14,8 +14,8 @@ namespace JSSpecCompiler {
 
 void ReferenceResolvingPass::process_function()
 {
-    for (auto name : m_function->m_argument_names)
-        m_function->m_local_variables.set(name, make_ref_counted<NamedVariableDeclaration>(name));
+    for (auto argument : m_function->m_arguments)
+        m_function->m_local_variables.set(argument.name, make_ref_counted<NamedVariableDeclaration>(argument.name));
     GenericASTPass::process_function();
 }
 

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

@@ -430,16 +430,16 @@ void SSABuildingPass::rename_variables(Vertex u, Vertex from)
 void SSABuildingPass::rename_variables()
 {
     HashMap<StringView, size_t> argument_index_by_name;
-    for (size_t i = 0; i < m_function->m_argument_names.size(); ++i)
-        argument_index_by_name.set(m_function->m_argument_names[i], i);
-    m_function->m_arguments.resize(m_function->m_argument_names.size());
+    for (size_t i = 0; i < m_function->m_arguments.size(); ++i)
+        argument_index_by_name.set(m_function->m_arguments[i].name, i);
+    m_function->m_ssa_arguments.resize(m_function->m_arguments.size());
 
     for (auto const& [name, var_decl] : m_function->m_local_variables) {
         make_new_ssa_variable_for(var_decl);
 
         if (auto maybe_index = argument_index_by_name.get(name); maybe_index.has_value()) {
             size_t index = maybe_index.value();
-            m_function->m_arguments[index] = m_def_stack.get(var_decl).value()[0];
+            m_function->m_ssa_arguments[index] = m_def_stack.get(var_decl).value()[0];
         }
     }
     make_new_ssa_variable_for(m_function->m_named_return_value);

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

@@ -40,15 +40,15 @@ FunctionDeclarationRef TranslationUnit::find_declaration_by_name(StringView name
     return it->value;
 }
 
-FunctionDeclaration::FunctionDeclaration(StringView name)
+FunctionDeclaration::FunctionDeclaration(StringView name, Vector<FunctionArgument>&& arguments)
     : m_name(name)
+    , m_arguments(arguments)
 {
 }
 
-FunctionDefinition::FunctionDefinition(StringView name, Tree ast, Vector<StringView>&& argument_names)
-    : FunctionDeclaration(name)
+FunctionDefinition::FunctionDefinition(StringView name, Tree ast, Vector<FunctionArgument>&& arguments)
+    : FunctionDeclaration(name, move(arguments))
     , m_ast(move(ast))
-    , m_argument_names(move(argument_names))
     , m_named_return_value(make_ref_counted<NamedVariableDeclaration>("$return"sv))
 {
 }

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

@@ -38,24 +38,28 @@ private:
     HashMap<StringView, FunctionDeclarationRef> m_function_index;
 };
 
+struct FunctionArgument {
+    StringView name;
+};
+
 class FunctionDeclaration : public RefCounted<FunctionDeclaration> {
 public:
-    FunctionDeclaration(StringView name);
+    FunctionDeclaration(StringView name, Vector<FunctionArgument>&& arguments);
 
     virtual ~FunctionDeclaration() = default;
 
     TranslationUnitRef m_translation_unit = nullptr;
     StringView m_name;
+    Vector<FunctionArgument> m_arguments;
 };
 
 class FunctionDefinition : public FunctionDeclaration {
 public:
-    FunctionDefinition(StringView name, Tree ast, Vector<StringView>&& argument_names);
+    FunctionDefinition(StringView name, Tree ast, Vector<FunctionArgument>&& arguments);
 
     void reindex_ssa_variables();
 
     Tree m_ast;
-    Vector<StringView> m_argument_names;
 
     // Populates during reference resolving
     // NOTE: The hash map here is ordered since we do not want random hash changes to break our test
@@ -67,7 +71,7 @@ public:
     RefPtr<ControlFlowGraph> m_cfg;
 
     // Fields populate during SSA building
-    Vector<SSAVariableDeclarationRef> m_arguments;
+    Vector<SSAVariableDeclarationRef> m_ssa_arguments;
     SSAVariableDeclarationRef m_return_value;
     Vector<SSAVariableDeclarationRef> m_local_ssa_variables;
 };

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

@@ -24,9 +24,9 @@ NonnullRefPtr<FunctionDefinition> CppASTConverter::convert()
     }
     auto tree = make_ref_counted<TreeList>(move(toplevel_statements));
 
-    Vector<StringView> arguments;
+    Vector<FunctionArgument> arguments;
     for (auto const& parameter : m_function->parameters())
-        arguments.append(parameter->full_name());
+        arguments.append({ .name = parameter->full_name() });
 
     return make_ref_counted<FunctionDefinition>(name, tree, move(arguments));
 }

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

@@ -249,12 +249,12 @@ void SpecParsingStep::run(TranslationUnitRef translation_unit)
 
     auto spec_function = SpecFunction::create(&m_document->root()).release_value_but_fixme_should_propagate_errors();
 
-    Vector<StringView> argument_names;
+    Vector<FunctionArgument> arguments;
     for (auto const& argument : spec_function.m_arguments)
-        argument_names.append(argument.name);
+        arguments.append({ argument.name });
 
     translation_unit->adopt_function(
-        make_ref_counted<FunctionDefinition>(spec_function.m_name, spec_function.m_algorithm.m_tree, move(argument_names)));
+        make_ref_counted<FunctionDefinition>(spec_function.m_name, spec_function.m_algorithm.m_tree, move(arguments)));
 }
 
 }

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

@@ -1,5 +1,5 @@
 ===== AST after parser =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfBranch
     UnresolvedReference cond1
@@ -27,7 +27,7 @@ TreeList
     UnresolvedReference b
 
 ===== AST after function-call-canonicalization =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfBranch
     UnresolvedReference cond1
@@ -55,7 +55,7 @@ TreeList
     UnresolvedReference b
 
 ===== AST after if-branch-merging =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     UnresolvedReference cond1
@@ -81,7 +81,7 @@ TreeList
     UnresolvedReference b
 
 ===== AST after reference-resolving =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     Var cond1
@@ -107,7 +107,7 @@ TreeList
     Var b
 
 ===== AST after cfg-building =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     Var cond1
@@ -133,7 +133,7 @@ TreeList
     Var b
 
 ===== CFG after cfg-building =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 0:
 ControlFlowBranch true=3 false=7
   Var cond1
@@ -183,7 +183,7 @@ BinaryOperation Assignment
 ControlFlowJump jump=1
 
 ===== AST after cfg-simplification =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     Var cond1
@@ -209,7 +209,7 @@ TreeList
     Var b
 
 ===== CFG after cfg-simplification =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 0:
 ControlFlowBranch true=3 false=6
   Var cond1
@@ -250,7 +250,7 @@ BinaryOperation Assignment
 ControlFlowJump jump=2
 
 ===== AST after ssa-building =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     Var cond1@0
@@ -276,7 +276,7 @@ TreeList
     Var b@2
 
 ===== CFG after ssa-building =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 0:
 ControlFlowBranch true=1 false=6
   Var cond1@0
@@ -319,7 +319,7 @@ BinaryOperation Assignment
 ControlFlowJump jump=3
 
 ===== AST after dce =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 TreeList
   IfElseIfChain
     Var cond1@0
@@ -345,7 +345,7 @@ TreeList
     Var b@2
 
 ===== CFG after dce =====
-f([ cond1, cond2 ]):
+f(cond1, cond2):
 0:
 ControlFlowBranch true=1 false=6
   Var cond1@0

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

@@ -71,6 +71,19 @@ private:
     Vector<CompilationStepWithDumpOptions> m_pipeline;
 };
 
+template<>
+struct AK::Formatter<Vector<FunctionArgument>> : AK::Formatter<StringView> {
+    ErrorOr<void> format(FormatBuilder& builder, Vector<FunctionArgument> const& arguments)
+    {
+        for (size_t i = 0; i < arguments.size(); ++i) {
+            TRY(builder.put_string(arguments[i].name));
+            if (i + 1 != arguments.size())
+                TRY(builder.put_literal(", "sv));
+        }
+        return {};
+    }
+};
+
 ErrorOr<int> serenity_main(Main::Arguments arguments)
 {
     Core::ArgsParser args_parser;
@@ -125,13 +138,13 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     // Functions referenced in DifferenceISODate
     // TODO: This is here just for testing. In a long run, we need some place, which is not
     //       `serenity_main`, to store built-in functions.
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("CompareISODate"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("CreateDateDurationRecord"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("AddISODate"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("ISODaysInMonth"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("ISODateToEpochDays"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("truncate"sv));
-    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("remainder"sv));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("CompareISODate"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("CreateDateDurationRecord"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("AddISODate"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("ISODaysInMonth"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("ISODateToEpochDays"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("truncate"sv, Vector<FunctionArgument> {}));
+    translation_unit.adopt_declaration(make_ref_counted<FunctionDeclaration>("remainder"sv, Vector<FunctionArgument> {}));
 
     for (auto const& step : pipeline.pipeline()) {
         step.step->run(&translation_unit);
@@ -144,14 +157,14 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         if (step.dump_ast) {
             outln(stderr, "===== AST after {} =====", step.step->name());
             for (auto const& function : translation_unit.functions_to_compile()) {
-                outln(stderr, "{}({}):", function->m_name, function->m_argument_names);
+                outln(stderr, "{}({}):", function->m_name, function->m_arguments);
                 outln(stderr, "{}", function->m_ast);
             }
         }
         if (step.dump_cfg && translation_unit.functions_to_compile()[0]->m_cfg != nullptr) {
             outln(stderr, "===== CFG after {} =====", step.step->name());
             for (auto const& function : translation_unit.functions_to_compile()) {
-                outln(stderr, "{}({}):", function->m_name, function->m_argument_names);
+                outln(stderr, "{}({}):", function->m_name, function->m_arguments);
                 outln(stderr, "{}", *function->m_cfg);
             }
         }