From d3c7ca313162fe3fd5fb06a9b79be01670e3fc21 Mon Sep 17 00:00:00 2001 From: Jon Meow <46229924+jonmeow@users.noreply.github.com> Date: Thu, 12 Aug 2021 09:52:32 -0700 Subject: [PATCH] Remove FunctionDefinition's default constructor (#735) This opens up a path for switching `int line_num` to a `Location loc`, which I want to do for tracking filenames of code. But I don't think we should have a default constructor on `Location` to avoid mistakes, and switching FunctionDefinition to an arena alloc seems more consistent anyways. --- executable_semantics/ast/declaration.h | 10 +++++----- executable_semantics/ast/function_definition.h | 1 - executable_semantics/interpreter/typecheck.cpp | 2 +- executable_semantics/syntax/parser.ypp | 14 +++++++------- executable_semantics/syntax/syntax_helpers.cpp | 12 +++++++----- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/executable_semantics/ast/declaration.h b/executable_semantics/ast/declaration.h index 682b4ad110bc..29e01781c5c8 100644 --- a/executable_semantics/ast/declaration.h +++ b/executable_semantics/ast/declaration.h @@ -72,18 +72,18 @@ class Declaration { class FunctionDeclaration : public Declaration { public: - FunctionDeclaration(FunctionDefinition definition) - : Declaration(Kind::FunctionDeclaration, definition.line_num), - definition(std::move(definition)) {} + FunctionDeclaration(const FunctionDefinition* definition) + : Declaration(Kind::FunctionDeclaration, definition->line_num), + definition(definition) {} static auto classof(const Declaration* decl) -> bool { return decl->Tag() == Kind::FunctionDeclaration; } - auto Definition() const -> const FunctionDefinition& { return definition; } + auto Definition() const -> const FunctionDefinition& { return *definition; } private: - FunctionDefinition definition; + const FunctionDefinition* definition; }; class StructDeclaration : public Declaration { diff --git a/executable_semantics/ast/function_definition.h b/executable_semantics/ast/function_definition.h index d2b66f462720..4bc8daae76ea 100644 --- a/executable_semantics/ast/function_definition.h +++ b/executable_semantics/ast/function_definition.h @@ -21,7 +21,6 @@ struct GenericBinding { }; struct FunctionDefinition { - FunctionDefinition() = default; FunctionDefinition(int line_num, std::string name, std::vector deduced_params, const TuplePattern* param_pattern, diff --git a/executable_semantics/interpreter/typecheck.cpp b/executable_semantics/interpreter/typecheck.cpp index 46caa780696e..aa1684005075 100644 --- a/executable_semantics/interpreter/typecheck.cpp +++ b/executable_semantics/interpreter/typecheck.cpp @@ -978,7 +978,7 @@ auto MakeTypeChecked(const Declaration& d, const TypeEnv& types, const Env& values) -> const Declaration* { switch (d.Tag()) { case Declaration::Kind::FunctionDeclaration: - return global_arena->New(*TypeCheckFunDef( + return global_arena->New(TypeCheckFunDef( &cast(d).Definition(), types, values)); case Declaration::Kind::StructDeclaration: { diff --git a/executable_semantics/syntax/parser.ypp b/executable_semantics/syntax/parser.ypp index 4d6a2be6fdac..2011d898e8c7 100644 --- a/executable_semantics/syntax/parser.ypp +++ b/executable_semantics/syntax/parser.ypp @@ -93,8 +93,8 @@ void Carbon::Parser::error(const location_type&, const std::string& message) { %token string_literal %type designator %type declaration -%type function_declaration -%type function_definition +%type function_declaration +%type function_definition %type > declaration_list %type statement %type if_statement @@ -509,7 +509,7 @@ deduced_params: function_definition: FN identifier deduced_params maybe_empty_tuple_pattern return_type block { - $$ = FunctionDefinition( + $$ = global_arena->New( yylineno, $2, $3, $4, global_arena->New($5.first), $5.second, $6); @@ -518,7 +518,7 @@ function_definition: { // The return type is not considered "omitted" because it's automatic from // the expression. - $$ = FunctionDefinition( + $$ = global_arena->New( yylineno, $2, $3, $4, global_arena->New(yylineno), true, global_arena->New(yylineno, $6, true)); @@ -527,7 +527,7 @@ function_definition: function_declaration: FN identifier deduced_params maybe_empty_tuple_pattern return_type ";" { - $$ = FunctionDefinition( + $$ = global_arena->New( yylineno, $2, $3, $4, global_arena->New($5.first), $5.second, nullptr); } @@ -566,9 +566,9 @@ alternative_list: ; declaration: function_definition - { $$ = global_arena->New(std::move($1)); } + { $$ = global_arena->New($1); } | function_declaration - { $$ = global_arena->New(std::move($1)); } + { $$ = global_arena->New($1); } | STRUCT identifier "{" member_list "}" { $$ = global_arena->New(yylineno, $2, $4); diff --git a/executable_semantics/syntax/syntax_helpers.cpp b/executable_semantics/syntax/syntax_helpers.cpp index 8d8d657aea6b..47ccfce1d51f 100644 --- a/executable_semantics/syntax/syntax_helpers.cpp +++ b/executable_semantics/syntax/syntax_helpers.cpp @@ -26,11 +26,13 @@ static void AddIntrinsics(std::list* fs) { global_arena->New( IntrinsicExpression::IntrinsicKind::Print), false); - auto* print = global_arena->New(FunctionDefinition( - -1, "Print", std::vector(), - global_arena->New(-1, print_fields), - global_arena->New(global_arena->New(-1)), - /*is_omitted_return_type=*/false, print_return)); + auto* print = global_arena->New( + global_arena->New( + -1, "Print", std::vector(), + global_arena->New(-1, print_fields), + global_arena->New( + global_arena->New(-1)), + /*is_omitted_return_type=*/false, print_return)); fs->insert(fs->begin(), print); }