From f75b4d322f30a00e243d69040af0992330653add Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Fri, 3 Dec 2021 13:52:31 -0800 Subject: [PATCH] Handle use-before-declare in static name lookup (#967) Also remove inheritance from NamedEntity for some classes that don't need it. --- executable_semantics/ast/BUILD | 1 - executable_semantics/ast/ast.h | 3 - executable_semantics/ast/ast_rtti.txt | 12 +- executable_semantics/ast/declaration.h | 30 +- executable_semantics/ast/member.h | 2 +- executable_semantics/ast/statement.h | 11 - executable_semantics/ast/static_scope.cpp | 8 +- executable_semantics/ast/static_scope.h | 2 +- executable_semantics/interpreter/BUILD | 6 + .../interpreter/exec_program.cpp | 2 +- .../interpreter/resolve_names.cpp | 346 +++++++----------- .../interpreter/resolve_names.h | 2 +- .../name_lookup/fail_choice_duplicate.carbon | 2 +- .../fail_use_before_declare.carbon | 2 +- 14 files changed, 163 insertions(+), 266 deletions(-) diff --git a/executable_semantics/ast/BUILD b/executable_semantics/ast/BUILD index c7ac2271d764..31482625f19a 100644 --- a/executable_semantics/ast/BUILD +++ b/executable_semantics/ast/BUILD @@ -10,7 +10,6 @@ cc_library( deps = [ ":declaration", ":library_name", - ":static_scope", ], ) diff --git a/executable_semantics/ast/ast.h b/executable_semantics/ast/ast.h index 5a16cfae9249..2e016223d633 100644 --- a/executable_semantics/ast/ast.h +++ b/executable_semantics/ast/ast.h @@ -9,7 +9,6 @@ #include "executable_semantics/ast/declaration.h" #include "executable_semantics/ast/library_name.h" -#include "executable_semantics/ast/static_scope.h" #include "executable_semantics/common/nonnull.h" namespace Carbon { @@ -24,8 +23,6 @@ struct AST { std::vector imports; // The file's ordered declarations. std::vector> declarations; - // Names declared at the top level of the file. - StaticScope static_scope; // Synthesized call to `Main`. Injected after parsing. std::optional> main_call; }; diff --git a/executable_semantics/ast/ast_rtti.txt b/executable_semantics/ast/ast_rtti.txt index 2798cd135d56..8604dd0dacad 100644 --- a/executable_semantics/ast/ast_rtti.txt +++ b/executable_semantics/ast/ast_rtti.txt @@ -10,13 +10,13 @@ abstract class Pattern : AstNode; class TuplePattern : Pattern; class AlternativePattern : Pattern; class ExpressionPattern : Pattern; -abstract class Declaration : AstNode, NamedEntity; - class FunctionDeclaration : Declaration; - class ClassDeclaration : Declaration; - class ChoiceDeclaration : Declaration; +abstract class Declaration : AstNode; + class FunctionDeclaration : Declaration, NamedEntity; + class ClassDeclaration : Declaration, NamedEntity; + class ChoiceDeclaration : Declaration, NamedEntity; class VariableDeclaration : Declaration; class GenericBinding : AstNode, NamedEntity; -class AlternativeSignature : AstNode, NamedEntity; +class AlternativeSignature : AstNode; abstract class Statement : AstNode; class ExpressionStatement : Statement; class Assign : Statement; @@ -52,4 +52,4 @@ abstract class Expression : AstNode; class IntrinsicExpression : Expression; class UnimplementedExpression : Expression; abstract class Member : AstNode; - class FieldMember : Member, NamedEntity; + class FieldMember : Member; diff --git a/executable_semantics/ast/declaration.h b/executable_semantics/ast/declaration.h index 9183027e8c58..d7b7da6de9dc 100644 --- a/executable_semantics/ast/declaration.h +++ b/executable_semantics/ast/declaration.h @@ -21,8 +21,6 @@ namespace Carbon { -class StaticScope; - // Abstract base class of all AST nodes representing patterns. // // Declaration and its derived classes support LLVM-style RTTI, including @@ -31,7 +29,7 @@ class StaticScope; // every concrete derived class must have a corresponding enumerator // in `Kind`; see https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html for // details. -class Declaration : public virtual AstNode, public NamedEntity { +class Declaration : public virtual AstNode { public: ~Declaration() override = 0; @@ -177,7 +175,7 @@ class ReturnTerm { SourceLocation source_loc_; }; -class FunctionDeclaration : public Declaration { +class FunctionDeclaration : public Declaration, public NamedEntity { public: FunctionDeclaration(SourceLocation source_loc, std::string name, std::vector> deduced_params, @@ -212,20 +210,15 @@ class FunctionDeclaration : public Declaration { auto body() const -> std::optional> { return body_; } auto body() -> std::optional> { return body_; } - // Only contains function parameters. Scoped variables are in the body. - auto static_scope() const -> const StaticScope& { return static_scope_; } - auto static_scope() -> StaticScope& { return static_scope_; } - private: std::string name_; std::vector> deduced_parameters_; Nonnull param_pattern_; ReturnTerm return_term_; std::optional> body_; - StaticScope static_scope_; }; -class ClassDeclaration : public Declaration { +class ClassDeclaration : public Declaration, public NamedEntity { public: ClassDeclaration(SourceLocation source_loc, std::string name, std::vector> members) @@ -240,17 +233,12 @@ class ClassDeclaration : public Declaration { auto name() const -> const std::string& { return name_; } auto members() const -> llvm::ArrayRef> { return members_; } - // Contains class members. Scoped variables are in the body. - auto static_scope() const -> const StaticScope& { return static_scope_; } - auto static_scope() -> StaticScope& { return static_scope_; } - private: std::string name_; std::vector> members_; - StaticScope static_scope_; }; -class AlternativeSignature : public virtual AstNode, public NamedEntity { +class AlternativeSignature : public virtual AstNode { public: AlternativeSignature(SourceLocation source_loc, std::string name, Nonnull signature) @@ -273,7 +261,7 @@ class AlternativeSignature : public virtual AstNode, public NamedEntity { Nonnull signature_; }; -class ChoiceDeclaration : public Declaration { +class ChoiceDeclaration : public Declaration, public NamedEntity { public: ChoiceDeclaration(SourceLocation source_loc, std::string name, std::vector> alternatives) @@ -294,20 +282,12 @@ class ChoiceDeclaration : public Declaration { return alternatives_; } - // Contains the alternatives. - auto static_scope() const -> const StaticScope& { return static_scope_; } - auto static_scope() -> StaticScope& { return static_scope_; } - private: std::string name_; std::vector> alternatives_; - StaticScope static_scope_; }; // Global variable definition implements the Declaration concept. -// -// TODO: this should not inherit from NamedEntity, because names should -// always resolve to the underlying binding, not the VariableDeclaration. class VariableDeclaration : public Declaration { public: VariableDeclaration(SourceLocation source_loc, diff --git a/executable_semantics/ast/member.h b/executable_semantics/ast/member.h index 9ae137b3ce1f..c9f8e6a54028 100644 --- a/executable_semantics/ast/member.h +++ b/executable_semantics/ast/member.h @@ -23,7 +23,7 @@ namespace Carbon { // every concrete derived class must have a corresponding enumerator // in `Kind`; see https://llvm.org/docs/HowToSetUpLLVMStyleRTTI.html for // details. -class Member : public virtual AstNode, public NamedEntity { +class Member : public virtual AstNode { public: ~Member() override = 0; diff --git a/executable_semantics/ast/statement.h b/executable_semantics/ast/statement.h index 177568f4e983..badba400f32b 100644 --- a/executable_semantics/ast/statement.h +++ b/executable_semantics/ast/statement.h @@ -20,7 +20,6 @@ namespace Carbon { class FunctionDeclaration; -class StaticScope; class Statement : public virtual AstNode { public: @@ -60,12 +59,8 @@ class Block : public Statement { return statements_; } - auto static_scope() const -> const StaticScope& { return static_scope_; } - auto static_scope() -> StaticScope& { return static_scope_; } - private: std::vector> statements_; - StaticScope static_scope_; }; class ExpressionStatement : public Statement { @@ -283,15 +278,9 @@ class Match : public Statement { auto statement() const -> const Statement& { return *statement_; } auto statement() -> Statement& { return *statement_; } - // Contains names for the pattern and statement. Note that when the - // statement is a block, it gains its own scope. - auto static_scope() const -> const StaticScope& { return static_scope_; } - auto static_scope() -> StaticScope& { return static_scope_; } - private: Nonnull pattern_; Nonnull statement_; - StaticScope static_scope_; }; Match(SourceLocation source_loc, Nonnull expression, diff --git a/executable_semantics/ast/static_scope.cpp b/executable_semantics/ast/static_scope.cpp index 99406bd8c417..9db5ba1f76d0 100644 --- a/executable_semantics/ast/static_scope.cpp +++ b/executable_semantics/ast/static_scope.cpp @@ -11,10 +11,11 @@ namespace Carbon { NamedEntity::~NamedEntity() = default; void StaticScope::Add(std::string name, Nonnull entity) { - if (!declared_names_.insert({name, entity}).second) { + auto [it, success] = declared_names_.insert({name, entity}); + if (!success && it->second != entity) { FATAL_COMPILATION_ERROR(entity->source_loc()) << "Duplicate name `" << name << "` also found at " - << declared_names_[name]->source_loc(); + << it->second->source_loc(); } } @@ -24,8 +25,7 @@ auto StaticScope::Resolve(const std::string& name, std::optional> result = TryResolve(name, source_loc); if (!result.has_value()) { - FATAL_COMPILATION_ERROR(source_loc) - << "'" << name << "' is not declared in this scope"; + FATAL_COMPILATION_ERROR(source_loc) << "could not resolve '" << name << "'"; } return *result; } diff --git a/executable_semantics/ast/static_scope.h b/executable_semantics/ast/static_scope.h index bd9a9deb6b7a..791d5b906b7b 100644 --- a/executable_semantics/ast/static_scope.h +++ b/executable_semantics/ast/static_scope.h @@ -34,7 +34,7 @@ class NamedEntity : public virtual AstNode { class StaticScope { public: // Defines `name` to be `entity` in this scope, or reports a compilation error - // if `name` is already defined in this scope. + // if `name` is already defined to be a different entity in this scope. void Add(std::string name, Nonnull entity); // Make `parent` a parent of this scope. diff --git a/executable_semantics/interpreter/BUILD b/executable_semantics/interpreter/BUILD index ec0046c068e5..63e98856f0d9 100644 --- a/executable_semantics/interpreter/BUILD +++ b/executable_semantics/interpreter/BUILD @@ -149,6 +149,12 @@ cc_library( deps = [ "//common:check", "//executable_semantics/ast", + "//executable_semantics/ast:declaration", + "//executable_semantics/ast:expression", + "//executable_semantics/ast:member", + "//executable_semantics/ast:pattern", + "//executable_semantics/ast:statement", + "//executable_semantics/ast:static_scope", "@llvm-project//llvm:Support", ], ) diff --git a/executable_semantics/interpreter/exec_program.cpp b/executable_semantics/interpreter/exec_program.cpp index 4f65fdf4350f..dea3b1544302 100644 --- a/executable_semantics/interpreter/exec_program.cpp +++ b/executable_semantics/interpreter/exec_program.cpp @@ -30,7 +30,7 @@ void ExecProgram(Nonnull arena, AST ast, bool trace) { arena->New(source_loc)); // Although name resolution is currently done once, generic programming // (particularly templates) may require more passes. - ResolveNames(arena, ast); + ResolveNames(ast); ResolveControlFlow(ast); TypeChecker(arena, trace).TypeCheck(ast); if (trace) { diff --git a/executable_semantics/interpreter/resolve_names.cpp b/executable_semantics/interpreter/resolve_names.cpp index 3d2c4cc03f1f..3c298eb7a6e0 100644 --- a/executable_semantics/interpreter/resolve_names.cpp +++ b/executable_semantics/interpreter/resolve_names.cpp @@ -4,223 +4,129 @@ #include "executable_semantics/interpreter/resolve_names.h" +#include + #include "executable_semantics/ast/declaration.h" +#include "executable_semantics/ast/expression.h" +#include "executable_semantics/ast/member.h" +#include "executable_semantics/ast/pattern.h" +#include "executable_semantics/ast/statement.h" +#include "executable_semantics/ast/static_scope.h" #include "llvm/Support/Casting.h" using llvm::cast; namespace Carbon { -namespace { +// Adds the names exposed by the given AST node to enclosing_scope. +static void AddExposedNames(const Declaration& declaration, + StaticScope& enclosing_scope); +static void AddExposedNames(const Member& member, StaticScope& enclosing_scope); -// Populates names for a pattern. See PopulateNamesInDeclaration for overall -// flow. -void PopulateNamesInPattern(const Pattern& pattern, StaticScope& static_scope) { - switch (pattern.kind()) { - case PatternKind::AlternativePattern: { - const auto& alt = cast(pattern); - PopulateNamesInPattern(alt.arguments(), static_scope); - break; - } - case PatternKind::BindingPattern: { - const auto& binding = cast(pattern); - if (binding.name().has_value()) { - static_scope.Add(*binding.name(), &binding); - } - break; - } - case PatternKind::TuplePattern: { - const auto& tuple = cast(pattern); - for (auto* field : tuple.fields()) { - PopulateNamesInPattern(*field, static_scope); - } - break; - } - case PatternKind::AutoPattern: - case PatternKind::ExpressionPattern: - // These don't add names. - break; - } -} - -// Populates names for a statement. See PopulateNamesInDeclaration for overall -// flow. -void PopulateNamesInStatement(Arena* arena, - std::optional> opt_statement, - StaticScope& static_scope) { - if (!opt_statement.has_value()) { - return; - } - Statement& statement = **opt_statement; - switch (statement.kind()) { - case StatementKind::Block: { - // Defines a new scope for names. - auto& block = cast(statement); - block.static_scope().AddParent(&static_scope); - for (const auto& statement : block.statements()) { - PopulateNamesInStatement(arena, statement, block.static_scope()); - } - break; - } - case StatementKind::Continuation: { - // Defines a new name and contains a block. - auto& cont = cast(statement); - static_scope.Add(cont.continuation_variable(), &cont); - PopulateNamesInStatement(arena, &cont.body(), static_scope); - break; - } - case StatementKind::VariableDefinition: { - // Defines a new name. - const auto& var = cast(statement); - PopulateNamesInPattern(var.pattern(), static_scope); - break; - } - case StatementKind::If: { - // Contains blocks. - auto& if_stmt = cast(statement); - PopulateNamesInStatement(arena, &if_stmt.then_block(), static_scope); - PopulateNamesInStatement(arena, if_stmt.else_block(), static_scope); - break; - } - case StatementKind::While: { - // Contains a block. - auto& while_stmt = cast(statement); - PopulateNamesInStatement(arena, &while_stmt.body(), static_scope); - break; - } - case StatementKind::Match: { - // Contains blocks. - auto& match = cast(statement); - for (auto& clause : match.clauses()) { - clause.static_scope().AddParent(&static_scope); - PopulateNamesInPattern(clause.pattern(), clause.static_scope()); - PopulateNamesInStatement(arena, &clause.statement(), - clause.static_scope()); - } - break; - } - case StatementKind::Assign: - case StatementKind::Await: - case StatementKind::Break: - case StatementKind::Continue: - case StatementKind::ExpressionStatement: - case StatementKind::Return: - case StatementKind::Run: - // Neither contains names nor a scope. - break; - } -} - -// Populates names for a member. See PopulateNamesInDeclaration for overall -// flow. -void PopulateNamesInMember(const Member& member, StaticScope& static_scope) { +static void AddExposedNames(const Member& member, + StaticScope& enclosing_scope) { switch (member.kind()) { case MemberKind::FieldMember: { const auto& field = cast(member); if (field.binding().name().has_value()) { - static_scope.Add(*field.binding().name(), &member); + enclosing_scope.Add(*field.binding().name(), &field.binding()); } break; } } } -// Populates declared names at scoped boundaries, such as file-level or -// function bodies. This doesn't currently recurse into expressions, but -// likely will in the future in order to resolve names in lambdas. -void PopulateNamesInDeclaration(Arena* arena, Declaration& declaration, - StaticScope& static_scope) { +static void AddExposedNames(const Declaration& declaration, + StaticScope& enclosing_scope) { switch (declaration.kind()) { case DeclarationKind::FunctionDeclaration: { auto& func = cast(declaration); - func.static_scope().AddParent(&static_scope); - static_scope.Add(func.name(), &declaration); - for (Nonnull param : func.deduced_parameters()) { - func.static_scope().Add(param->name(), param); - } - PopulateNamesInPattern(func.param_pattern(), func.static_scope()); - PopulateNamesInStatement(arena, func.body(), func.static_scope()); + enclosing_scope.Add(func.name(), &func); break; } case DeclarationKind::ClassDeclaration: { auto& class_decl = cast(declaration); - class_decl.static_scope().AddParent(&static_scope); - static_scope.Add(class_decl.name(), &declaration); - for (auto* member : class_decl.members()) { - PopulateNamesInMember(*member, class_decl.static_scope()); - } + enclosing_scope.Add(class_decl.name(), &class_decl); break; } case DeclarationKind::ChoiceDeclaration: { auto& choice = cast(declaration); - choice.static_scope().AddParent(&static_scope); - static_scope.Add(choice.name(), &declaration); - for (Nonnull alt : choice.alternatives()) { - choice.static_scope().Add(alt->name(), alt); - } - // Populate name into declared_names. - // Init the choice's declared_names, and populate it with the - // alternatives. + enclosing_scope.Add(choice.name(), &choice); break; } case DeclarationKind::VariableDeclaration: auto& var = cast(declaration); if (var.binding().name().has_value()) { - static_scope.Add(*(var.binding().name()), &var.binding()); + enclosing_scope.Add(*(var.binding().name()), &var.binding()); } return; } } -// Populates the named_entity member of all IdentifierExpressions in the -// subtree rooted at `expression`, whose nearest enclosing scope is -// `enclosing_scope`. -static void ResolveNamesInExpression(Expression& expression, - const StaticScope& enclosing_scope) { +// Traverses the sub-AST rooted at the given node, resolving all names within +// it using enclosing_scope, and updating enclosing_scope to add names to +// it as they become available. In scopes where names are only visible below +// their point of declaration (such as block scopes in C++), this is implemented +// as a single pass, recursively calling ResolveNames on the elements of the +// scope in order. In scopes where names are also visible above their point of +// declaration (such as class scopes in C++), this requires two passes: first +// calling AddExposedNames on each element of the scope to populate a +// StaticScope, and then calling ResolveNames on each element, passing it the +// already-populated StaticScope. +static void ResolveNames(Expression& expression, + const StaticScope& enclosing_scope); +static void ResolveNames(Pattern& pattern, StaticScope& enclosing_scope); +static void ResolveNames(Statement& statement, StaticScope& enclosing_scope); +static void ResolveNames(Member& member, StaticScope& enclosing_scope); +static void ResolveNames(Declaration& declaration, + StaticScope& enclosing_scope); + +static void ResolveNames(Expression& expression, + const StaticScope& enclosing_scope) { switch (expression.kind()) { case ExpressionKind::CallExpression: { auto& call = cast(expression); - ResolveNamesInExpression(call.function(), enclosing_scope); - ResolveNamesInExpression(call.argument(), enclosing_scope); + ResolveNames(call.function(), enclosing_scope); + ResolveNames(call.argument(), enclosing_scope); break; } case ExpressionKind::FunctionTypeLiteral: { auto& fun_type = cast(expression); - ResolveNamesInExpression(fun_type.parameter(), enclosing_scope); - ResolveNamesInExpression(fun_type.return_type(), enclosing_scope); + ResolveNames(fun_type.parameter(), enclosing_scope); + ResolveNames(fun_type.return_type(), enclosing_scope); break; } case ExpressionKind::FieldAccessExpression: - ResolveNamesInExpression( - cast(expression).aggregate(), enclosing_scope); + ResolveNames(cast(expression).aggregate(), + enclosing_scope); break; case ExpressionKind::IndexExpression: { auto& index = cast(expression); - ResolveNamesInExpression(index.aggregate(), enclosing_scope); - ResolveNamesInExpression(index.offset(), enclosing_scope); + ResolveNames(index.aggregate(), enclosing_scope); + ResolveNames(index.offset(), enclosing_scope); break; } case ExpressionKind::PrimitiveOperatorExpression: for (Nonnull operand : cast(expression).arguments()) { - ResolveNamesInExpression(*operand, enclosing_scope); + ResolveNames(*operand, enclosing_scope); } break; case ExpressionKind::TupleLiteral: for (Nonnull field : cast(expression).fields()) { - ResolveNamesInExpression(*field, enclosing_scope); + ResolveNames(*field, enclosing_scope); } break; case ExpressionKind::StructLiteral: for (FieldInitializer& init : cast(expression).fields()) { - ResolveNamesInExpression(init.expression(), enclosing_scope); + ResolveNames(init.expression(), enclosing_scope); } break; case ExpressionKind::StructTypeLiteral: for (FieldInitializer& init : cast(expression).fields()) { - ResolveNamesInExpression(init.expression(), enclosing_scope); + ResolveNames(init.expression(), enclosing_scope); } break; case ExpressionKind::IdentifierExpression: { @@ -230,8 +136,8 @@ static void ResolveNamesInExpression(Expression& expression, break; } case ExpressionKind::IntrinsicExpression: - ResolveNamesInExpression(cast(expression).args(), - enclosing_scope); + ResolveNames(cast(expression).args(), + enclosing_scope); break; case ExpressionKind::BoolTypeLiteral: case ExpressionKind::BoolLiteral: @@ -247,98 +153,102 @@ static void ResolveNamesInExpression(Expression& expression, } } -// Equivalent to ResolveNamesInExpression, but operates on patterns. -static void ResolveNamesInPattern(Pattern& pattern, - const StaticScope& enclosing_scope) { +static void ResolveNames(Pattern& pattern, StaticScope& enclosing_scope) { switch (pattern.kind()) { - case PatternKind::BindingPattern: - ResolveNamesInPattern(cast(pattern).type(), - enclosing_scope); + case PatternKind::BindingPattern: { + auto& binding = cast(pattern); + ResolveNames(binding.type(), enclosing_scope); + if (binding.name().has_value()) { + enclosing_scope.Add(*binding.name(), &binding); + } break; + } case PatternKind::TuplePattern: for (Nonnull field : cast(pattern).fields()) { - ResolveNamesInPattern(*field, enclosing_scope); + ResolveNames(*field, enclosing_scope); } break; case PatternKind::AlternativePattern: { auto& alternative = cast(pattern); - ResolveNamesInExpression(alternative.choice_type(), enclosing_scope); - ResolveNamesInPattern(alternative.arguments(), enclosing_scope); + ResolveNames(alternative.choice_type(), enclosing_scope); + ResolveNames(alternative.arguments(), enclosing_scope); break; } case PatternKind::ExpressionPattern: - ResolveNamesInExpression(cast(pattern).expression(), - enclosing_scope); + ResolveNames(cast(pattern).expression(), + enclosing_scope); break; case PatternKind::AutoPattern: break; } } -// Equivalent to ResolveNamesInExpression, but operates on statements. -static void ResolveNamesInStatement(Statement& statement, - const StaticScope& enclosing_scope) { +static void ResolveNames(Statement& statement, StaticScope& enclosing_scope) { switch (statement.kind()) { case StatementKind::ExpressionStatement: - ResolveNamesInExpression( - cast(statement).expression(), enclosing_scope); + ResolveNames(cast(statement).expression(), + enclosing_scope); break; case StatementKind::Assign: { auto& assign = cast(statement); - ResolveNamesInExpression(assign.lhs(), enclosing_scope); - ResolveNamesInExpression(assign.rhs(), enclosing_scope); + ResolveNames(assign.lhs(), enclosing_scope); + ResolveNames(assign.rhs(), enclosing_scope); break; } case StatementKind::VariableDefinition: { auto& def = cast(statement); - ResolveNamesInPattern(def.pattern(), enclosing_scope); - ResolveNamesInExpression(def.init(), enclosing_scope); + ResolveNames(def.init(), enclosing_scope); + ResolveNames(def.pattern(), enclosing_scope); break; } case StatementKind::If: { auto& if_stmt = cast(statement); - ResolveNamesInExpression(if_stmt.condition(), enclosing_scope); - ResolveNamesInStatement(if_stmt.then_block(), enclosing_scope); + ResolveNames(if_stmt.condition(), enclosing_scope); + ResolveNames(if_stmt.then_block(), enclosing_scope); if (if_stmt.else_block().has_value()) { - ResolveNamesInStatement(**if_stmt.else_block(), enclosing_scope); + ResolveNames(**if_stmt.else_block(), enclosing_scope); } break; } case StatementKind::Return: - ResolveNamesInExpression(cast(statement).expression(), - enclosing_scope); + ResolveNames(cast(statement).expression(), enclosing_scope); break; case StatementKind::Block: { - // TODO: this will resolve usages to declarations that occur later in - // the block. Figure out how to avoid that. auto& block = cast(statement); + StaticScope block_scope; + block_scope.AddParent(&enclosing_scope); for (Nonnull sub_statement : block.statements()) { - ResolveNamesInStatement(*sub_statement, block.static_scope()); + ResolveNames(*sub_statement, block_scope); } break; } case StatementKind::While: { auto& while_stmt = cast(statement); - ResolveNamesInExpression(while_stmt.condition(), enclosing_scope); - ResolveNamesInStatement(while_stmt.body(), enclosing_scope); + ResolveNames(while_stmt.condition(), enclosing_scope); + ResolveNames(while_stmt.body(), enclosing_scope); break; } case StatementKind::Match: { auto& match = cast(statement); - ResolveNamesInExpression(match.expression(), enclosing_scope); + ResolveNames(match.expression(), enclosing_scope); for (Match::Clause& clause : match.clauses()) { - ResolveNamesInPattern(clause.pattern(), clause.static_scope()); - ResolveNamesInStatement(clause.statement(), clause.static_scope()); + StaticScope clause_scope; + clause_scope.AddParent(&enclosing_scope); + ResolveNames(clause.pattern(), clause_scope); + ResolveNames(clause.statement(), clause_scope); } break; } - case StatementKind::Continuation: - ResolveNamesInStatement(cast(statement).body(), - enclosing_scope); + case StatementKind::Continuation: { + auto& continuation = cast(statement); + enclosing_scope.Add(continuation.continuation_variable(), &continuation); + StaticScope continuation_scope; + continuation_scope.AddParent(&enclosing_scope); + ResolveNames(cast(statement).body(), continuation_scope); break; + } case StatementKind::Run: - ResolveNamesInExpression(cast(statement).argument(), - enclosing_scope); + ResolveNames(cast(statement).argument(), enclosing_scope); break; case StatementKind::Await: case StatementKind::Break: @@ -347,65 +257,81 @@ static void ResolveNamesInStatement(Statement& statement, } } -// Recurses through a declaration to find and resolve IdentifierExpressions -// using declared_names. -void ResolveNamesInDeclaration(Declaration& declaration, - const StaticScope& enclosing_scope) { +static void ResolveNames(Member& member, StaticScope& enclosing_scope) { + switch (member.kind()) { + case MemberKind::FieldMember: + ResolveNames(cast(member).binding(), enclosing_scope); + } +} + +static void ResolveNames(Declaration& declaration, + StaticScope& enclosing_scope) { switch (declaration.kind()) { case DeclarationKind::FunctionDeclaration: { auto& function = cast(declaration); + StaticScope function_scope; + function_scope.AddParent(&enclosing_scope); for (Nonnull binding : function.deduced_parameters()) { - ResolveNamesInExpression(binding->type(), function.static_scope()); + function_scope.Add(binding->name(), binding); + ResolveNames(binding->type(), function_scope); } - ResolveNamesInPattern(function.param_pattern(), function.static_scope()); + ResolveNames(function.param_pattern(), function_scope); if (function.return_term().type_expression().has_value()) { - ResolveNamesInExpression(**function.return_term().type_expression(), - function.static_scope()); + ResolveNames(**function.return_term().type_expression(), + function_scope); } if (function.body().has_value()) { - ResolveNamesInStatement(**function.body(), function.static_scope()); + ResolveNames(**function.body(), function_scope); } break; } case DeclarationKind::ClassDeclaration: { auto& class_decl = cast(declaration); + StaticScope class_scope; + class_scope.AddParent(&enclosing_scope); for (Nonnull member : class_decl.members()) { - switch (member->kind()) { - case MemberKind::FieldMember: - ResolveNamesInPattern(cast(member)->binding(), - class_decl.static_scope()); - } + AddExposedNames(*member, class_scope); + } + for (Nonnull member : class_decl.members()) { + ResolveNames(*member, class_scope); } break; } case DeclarationKind::ChoiceDeclaration: { auto& choice = cast(declaration); + // Alternative names are never used unqualified, so we don't need to + // add the alternatives to a scope, or introduce a new scope; we only + // need to check for duplicates. + std::set alternative_names; for (Nonnull alternative : choice.alternatives()) { - ResolveNamesInExpression(alternative->signature(), - choice.static_scope()); + ResolveNames(alternative->signature(), enclosing_scope); + if (!alternative_names.insert(alternative->name()).second) { + FATAL_COMPILATION_ERROR(alternative->source_loc()) + << "Duplicate name `" << alternative->name() + << "` in choice type"; + } } break; } case DeclarationKind::VariableDeclaration: { auto& var = cast(declaration); - ResolveNamesInPattern(var.binding(), enclosing_scope); - ResolveNamesInExpression(var.initializer(), enclosing_scope); + ResolveNames(var.binding(), enclosing_scope); + ResolveNames(var.initializer(), enclosing_scope); break; } } } -} // namespace - -void ResolveNames(Arena* arena, AST& ast) { +void ResolveNames(AST& ast) { + StaticScope file_scope; for (auto declaration : ast.declarations) { - PopulateNamesInDeclaration(arena, *declaration, ast.static_scope); + AddExposedNames(*declaration, file_scope); } for (auto declaration : ast.declarations) { - ResolveNamesInDeclaration(*declaration, ast.static_scope); + ResolveNames(*declaration, file_scope); } if (ast.main_call.has_value()) { - ResolveNamesInExpression(**ast.main_call, ast.static_scope); + ResolveNames(**ast.main_call, file_scope); } } diff --git a/executable_semantics/interpreter/resolve_names.h b/executable_semantics/interpreter/resolve_names.h index 4624bac39fdd..80e3b9938424 100644 --- a/executable_semantics/interpreter/resolve_names.h +++ b/executable_semantics/interpreter/resolve_names.h @@ -11,7 +11,7 @@ namespace Carbon { // Resolves names (IdentifierExpressions) in the AST. -void ResolveNames(Nonnull, AST& ast); +void ResolveNames(AST& ast); } // namespace Carbon diff --git a/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon b/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon index 4dd854ab252d..81d4627f5a60 100644 --- a/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon +++ b/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon @@ -7,7 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon:16: Duplicate name `None` also found at {{.*}}/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon:15 +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/name_lookup/fail_choice_duplicate.carbon:16: Duplicate name `None` in choice type package ExecutableSemanticsTest api; diff --git a/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon b/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon index 361858b83ba4..bc94ed993042 100644 --- a/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon +++ b/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon @@ -7,7 +7,7 @@ // RUN: not executable_semantics --trace %s 2>&1 | \ // RUN: FileCheck --match-full-lines --allow-unused-prefixes %s // AUTOUPDATE: executable_semantics %s -// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon:15: could not find `x` +// CHECK: COMPILATION ERROR: {{.*}}/executable_semantics/testdata/name_lookup/fail_use_before_declare.carbon:15: could not resolve 'x' package ExecutableSemanticsTest api;