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

@@ -10,7 +10,6 @@ cc_library(
     deps = [
         ":declaration",
         ":library_name",
-        ":static_scope",
     ],
 )
 

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<LibraryName> imports;
   // The file's ordered declarations.
   std::vector<Nonnull<Declaration*>> declarations;
-  // Names declared at the top level of the file.
-  StaticScope static_scope;
   // Synthesized call to `Main`. Injected after parsing.
   std::optional<Nonnull<CallExpression*>> main_call;
 };

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;

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<Nonnull<GenericBinding*>> deduced_params,
@@ -212,20 +210,15 @@ class FunctionDeclaration : public Declaration {
   auto body() const -> std::optional<Nonnull<const Block*>> { return body_; }
   auto body() -> std::optional<Nonnull<Block*>> { 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<Nonnull<GenericBinding*>> deduced_parameters_;
   Nonnull<TuplePattern*> param_pattern_;
   ReturnTerm return_term_;
   std::optional<Nonnull<Block*>> body_;
-  StaticScope static_scope_;
 };
 
-class ClassDeclaration : public Declaration {
+class ClassDeclaration : public Declaration, public NamedEntity {
  public:
   ClassDeclaration(SourceLocation source_loc, std::string name,
                    std::vector<Nonnull<Member*>> members)
@@ -240,17 +233,12 @@ class ClassDeclaration : public Declaration {
   auto name() const -> const std::string& { return name_; }
   auto members() const -> llvm::ArrayRef<Nonnull<Member*>> { 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<Nonnull<Member*>> 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<Expression*> signature)
@@ -273,7 +261,7 @@ class AlternativeSignature : public virtual AstNode, public NamedEntity {
   Nonnull<Expression*> signature_;
 };
 
-class ChoiceDeclaration : public Declaration {
+class ChoiceDeclaration : public Declaration, public NamedEntity {
  public:
   ChoiceDeclaration(SourceLocation source_loc, std::string name,
                     std::vector<Nonnull<AlternativeSignature*>> 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<Nonnull<AlternativeSignature*>> 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,

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;
 

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<Nonnull<Statement*>> 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*> pattern_;
     Nonnull<Statement*> statement_;
-    StaticScope static_scope_;
   };
 
   Match(SourceLocation source_loc, Nonnull<Expression*> expression,

executable_semantics/ast/static_scope.cpp

@@ -11,10 +11,11 @@ namespace Carbon {
 NamedEntity::~NamedEntity() = default;
 
 void StaticScope::Add(std::string name, Nonnull<const NamedEntity*> 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<Nonnull<const NamedEntity*>> 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;
 }

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<const NamedEntity*> entity);
 
   // Make `parent` a parent of this scope.

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",
     ],
 )

executable_semantics/interpreter/exec_program.cpp

@@ -30,7 +30,7 @@ void ExecProgram(Nonnull<Arena*> arena, AST ast, bool trace) {
       arena->New<TupleLiteral>(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) {

executable_semantics/interpreter/resolve_names.cpp

@@ -4,223 +4,129 @@
 
 #include "executable_semantics/interpreter/resolve_names.h"
 
+#include <set>
+
 #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 {
-
-// 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<AlternativePattern>(pattern);
-      PopulateNamesInPattern(alt.arguments(), static_scope);
-      break;
-    }
-    case PatternKind::BindingPattern: {
-      const auto& binding = cast<BindingPattern>(pattern);
-      if (binding.name().has_value()) {
-        static_scope.Add(*binding.name(), &binding);
-      }
-      break;
-    }
-    case PatternKind::TuplePattern: {
-      const auto& tuple = cast<TuplePattern>(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<Nonnull<Statement*>> 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<Block>(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<Continuation>(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<VariableDefinition>(statement);
-      PopulateNamesInPattern(var.pattern(), static_scope);
-      break;
-    }
-    case StatementKind::If: {
-      // Contains blocks.
-      auto& if_stmt = cast<If>(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<While>(statement);
-      PopulateNamesInStatement(arena, &while_stmt.body(), static_scope);
-      break;
-    }
-    case StatementKind::Match: {
-      // Contains blocks.
-      auto& match = cast<Match>(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;
-  }
-}
+// 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 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<FieldMember>(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<FunctionDeclaration>(declaration);
-      func.static_scope().AddParent(&static_scope);
-      static_scope.Add(func.name(), &declaration);
-      for (Nonnull<const GenericBinding*> 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<ClassDeclaration>(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<ChoiceDeclaration>(declaration);
-      choice.static_scope().AddParent(&static_scope);
-      static_scope.Add(choice.name(), &declaration);
-      for (Nonnull<const AlternativeSignature*> 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<VariableDeclaration>(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<CallExpression>(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<FunctionTypeLiteral>(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<FieldAccessExpression>(expression).aggregate(), enclosing_scope);
+      ResolveNames(cast<FieldAccessExpression>(expression).aggregate(),
+                   enclosing_scope);
       break;
     case ExpressionKind::IndexExpression: {
       auto& index = cast<IndexExpression>(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<Expression*> operand :
            cast<PrimitiveOperatorExpression>(expression).arguments()) {
-        ResolveNamesInExpression(*operand, enclosing_scope);
+        ResolveNames(*operand, enclosing_scope);
       }
       break;
     case ExpressionKind::TupleLiteral:
       for (Nonnull<Expression*> field :
            cast<TupleLiteral>(expression).fields()) {
-        ResolveNamesInExpression(*field, enclosing_scope);
+        ResolveNames(*field, enclosing_scope);
       }
       break;
     case ExpressionKind::StructLiteral:
       for (FieldInitializer& init : cast<StructLiteral>(expression).fields()) {
-        ResolveNamesInExpression(init.expression(), enclosing_scope);
+        ResolveNames(init.expression(), enclosing_scope);
       }
       break;
     case ExpressionKind::StructTypeLiteral:
       for (FieldInitializer& init :
            cast<StructTypeLiteral>(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<IntrinsicExpression>(expression).args(),
-                               enclosing_scope);
+      ResolveNames(cast<IntrinsicExpression>(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<BindingPattern>(pattern).type(),
-                            enclosing_scope);
+    case PatternKind::BindingPattern: {
+      auto& binding = cast<BindingPattern>(pattern);
+      ResolveNames(binding.type(), enclosing_scope);
+      if (binding.name().has_value()) {
+        enclosing_scope.Add(*binding.name(), &binding);
+      }
       break;
+    }
     case PatternKind::TuplePattern:
       for (Nonnull<Pattern*> field : cast<TuplePattern>(pattern).fields()) {
-        ResolveNamesInPattern(*field, enclosing_scope);
+        ResolveNames(*field, enclosing_scope);
       }
       break;
     case PatternKind::AlternativePattern: {
       auto& alternative = cast<AlternativePattern>(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<ExpressionPattern>(pattern).expression(),
-                               enclosing_scope);
+      ResolveNames(cast<ExpressionPattern>(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<ExpressionStatement>(statement).expression(), enclosing_scope);
+      ResolveNames(cast<ExpressionStatement>(statement).expression(),
+                   enclosing_scope);
       break;
     case StatementKind::Assign: {
       auto& assign = cast<Assign>(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<VariableDefinition>(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<If>(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<Return>(statement).expression(),
-                               enclosing_scope);
+      ResolveNames(cast<Return>(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<Block>(statement);
+      StaticScope block_scope;
+      block_scope.AddParent(&enclosing_scope);
       for (Nonnull<Statement*> sub_statement : block.statements()) {
-        ResolveNamesInStatement(*sub_statement, block.static_scope());
+        ResolveNames(*sub_statement, block_scope);
       }
       break;
     }
     case StatementKind::While: {
       auto& while_stmt = cast<While>(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<Match>(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<Continuation>(statement).body(),
-                              enclosing_scope);
+    case StatementKind::Continuation: {
+      auto& continuation = cast<Continuation>(statement);
+      enclosing_scope.Add(continuation.continuation_variable(), &continuation);
+      StaticScope continuation_scope;
+      continuation_scope.AddParent(&enclosing_scope);
+      ResolveNames(cast<Continuation>(statement).body(), continuation_scope);
       break;
+    }
     case StatementKind::Run:
-      ResolveNamesInExpression(cast<Run>(statement).argument(),
-                               enclosing_scope);
+      ResolveNames(cast<Run>(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<FieldMember>(member).binding(), enclosing_scope);
+  }
+}
+
+static void ResolveNames(Declaration& declaration,
+                         StaticScope& enclosing_scope) {
   switch (declaration.kind()) {
     case DeclarationKind::FunctionDeclaration: {
       auto& function = cast<FunctionDeclaration>(declaration);
+      StaticScope function_scope;
+      function_scope.AddParent(&enclosing_scope);
       for (Nonnull<GenericBinding*> 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<ClassDeclaration>(declaration);
+      StaticScope class_scope;
+      class_scope.AddParent(&enclosing_scope);
       for (Nonnull<Member*> member : class_decl.members()) {
-        switch (member->kind()) {
-          case MemberKind::FieldMember:
-            ResolveNamesInPattern(cast<FieldMember>(member)->binding(),
-                                  class_decl.static_scope());
-        }
+        AddExposedNames(*member, class_scope);
+      }
+      for (Nonnull<Member*> member : class_decl.members()) {
+        ResolveNames(*member, class_scope);
       }
       break;
     }
     case DeclarationKind::ChoiceDeclaration: {
       auto& choice = cast<ChoiceDeclaration>(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<std::string_view> alternative_names;
       for (Nonnull<AlternativeSignature*> 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<VariableDeclaration>(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);
   }
 }
 

executable_semantics/interpreter/resolve_names.h

@@ -11,7 +11,7 @@
 namespace Carbon {
 
 // Resolves names (IdentifierExpressions) in the AST.
-void ResolveNames(Nonnull<Arena*>, AST& ast);
+void ResolveNames(AST& ast);
 
 }  // namespace Carbon
 

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;
 

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;