Add alias name to name resolution after processing the target to avoid self-referencing name crash (#1295)

Fixes #1294

explorer/ast/static_scope.cpp

@@ -9,17 +9,28 @@
 
 namespace Carbon {
 
-auto StaticScope::Add(std::string name, ValueNodeView entity)
-    -> ErrorOr<Success> {
-  auto [it, success] = declared_names_.insert({name, entity});
-  if (!success && it->second != entity) {
-    return CompilationError(entity.base().source_loc())
-           << "Duplicate name `" << name << "` also found at "
-           << it->second.base().source_loc();
+auto StaticScope::Add(const std::string& name, ValueNodeView entity,
+                      bool usable) -> ErrorOr<Success> {
+  auto [it, inserted] = declared_names_.insert({name, {entity, usable}});
+  if (!inserted) {
+    if (it->second.entity != entity) {
+      return CompilationError(entity.base().source_loc())
+             << "Duplicate name `" << name << "` also found at "
+             << it->second.entity.base().source_loc();
+    }
+    CARBON_CHECK(usable || !it->second.usable)
+        << entity.base().source_loc() << " attempting to mark a usable name `"
+        << name << "` as unusable";
   }
   return Success();
 }
 
+void StaticScope::MarkUsable(const std::string& name) {
+  auto it = declared_names_.find(name);
+  CARBON_CHECK(it != declared_names_.end()) << name << " not found";
+  it->second.usable = true;
+}
+
 auto StaticScope::Resolve(const std::string& name,
                           SourceLocation source_loc) const
     -> ErrorOr<ValueNodeView> {
@@ -36,7 +47,12 @@ auto StaticScope::TryResolve(const std::string& name,
     -> ErrorOr<std::optional<ValueNodeView>> {
   auto it = declared_names_.find(name);
   if (it != declared_names_.end()) {
-    return std::make_optional(it->second);
+    if (!it->second.usable) {
+      return CompilationError(source_loc)
+             << "'" << name
+             << "' is not usable until after it has been completely declared";
+    }
+    return std::make_optional(it->second.entity);
   }
   std::optional<ValueNodeView> result;
   for (Nonnull<const StaticScope*> parent : parent_scopes_) {

explorer/ast/static_scope.h

@@ -153,7 +153,13 @@ class StaticScope {
  public:
   // Defines `name` to be `entity` in this scope, or reports a compilation error
   // if `name` is already defined to be a different entity in this scope.
-  auto Add(std::string name, ValueNodeView entity) -> ErrorOr<Success>;
+  // If `usable` is `false`, `name` cannot yet be referenced and `Resolve()`
+  // methods will fail for it.
+  auto Add(const std::string& name, ValueNodeView entity, bool usable = true)
+      -> ErrorOr<Success>;
+
+  // Marks `name` as usable.
+  void MarkUsable(const std::string& name);
 
   // Make `parent` a parent of this scope.
   // REQUIRES: `parent` is not already a parent of this scope.
@@ -174,8 +180,12 @@ class StaticScope {
   auto TryResolve(const std::string& name, SourceLocation source_loc) const
       -> ErrorOr<std::optional<ValueNodeView>>;
 
+  struct Entry {
+    ValueNodeView entity;
+    bool usable = false;
+  };
   // Maps locally declared names to their entities.
-  std::unordered_map<std::string, ValueNodeView> declared_names_;
+  std::unordered_map<std::string, Entry> declared_names_;
 
   // A list of scopes used for name lookup within this scope.
   std::vector<Nonnull<StaticScope*>> parent_scopes_;

explorer/interpreter/resolve_names.cpp

@@ -66,7 +66,8 @@ static auto AddExposedNames(const Declaration& declaration,
     }
     case DeclarationKind::AliasDeclaration: {
       auto& alias = cast<AliasDeclaration>(declaration);
-      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(alias.name(), &alias));
+      CARBON_RETURN_IF_ERROR(
+          enclosing_scope.Add(alias.name(), &alias, /*usable=*/false));
       break;
     }
   }
@@ -458,8 +459,9 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope)
     }
 
     case DeclarationKind::AliasDeclaration: {
-      CARBON_RETURN_IF_ERROR(ResolveNames(
-          cast<AliasDeclaration>(declaration).target(), enclosing_scope));
+      auto& alias = cast<AliasDeclaration>(declaration);
+      CARBON_RETURN_IF_ERROR(ResolveNames(alias.target(), enclosing_scope));
+      enclosing_scope.MarkUsable(alias.name());
       break;
     }
   }

explorer/testdata/alias/fail_self_alias.carbon

@@ -0,0 +1,18 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// RUN: %{not} %{explorer} %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes=false %s
+// RUN: %{not} %{explorer} --parser_debug --trace_file=- %s 2>&1 | \
+// RUN:   %{FileCheck} --match-full-lines --allow-unused-prefixes %s
+// AUTOUPDATE: %{explorer} %s
+
+package ExplorerTest api;
+
+// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/alias/fail_self_alias.carbon:[[@LINE+1]]: 'a' is not usable until after it has been completely declared
+alias a = a;
+
+fn Main() -> i32 {
+  return 0;
+}