Give a different diagnostic if a name is used before it's declared. (#1364)

Instead of complaining that the name is not completely declared, say it's not
declared at all yet.

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>

explorer/ast/static_scope.cpp

@@ -12,26 +12,34 @@
 namespace Carbon {
 
 auto StaticScope::Add(const std::string& name, ValueNodeView entity,
-                      bool usable) -> ErrorOr<Success> {
-  auto [it, inserted] = declared_names_.insert({name, {entity, usable}});
+                      NameStatus status /* = NameStatus::Usable*/)
+    -> ErrorOr<Success> {
+  auto [it, inserted] = declared_names_.insert({name, {entity, status}});
   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";
-    it->second.usable |= usable;
+    if (static_cast<int>(status) > static_cast<int>(it->second.status)) {
+      it->second.status = status;
+    }
   }
   return Success();
 }
 
+void StaticScope::MarkDeclared(const std::string& name) {
+  auto it = declared_names_.find(name);
+  CARBON_CHECK(it != declared_names_.end()) << name << " not found";
+  if (it->second.status == NameStatus::KnownButNotDeclared) {
+    it->second.status = NameStatus::DeclaredButNotUsable;
+  }
+}
+
 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;
+  it->second.status = NameStatus::Usable;
 }
 
 auto StaticScope::Resolve(const std::string& name,
@@ -50,12 +58,17 @@ auto StaticScope::TryResolve(const std::string& name,
     -> ErrorOr<std::optional<ValueNodeView>> {
   auto it = declared_names_.find(name);
   if (it != declared_names_.end()) {
-    if (!it->second.usable) {
-      return CompilationError(source_loc)
-             << "'" << name
-             << "' is not usable until after it has been completely declared";
+    switch (it->second.status) {
+      case NameStatus::KnownButNotDeclared:
+        return CompilationError(source_loc)
+               << "'" << name << "' has not been declared yet";
+      case NameStatus::DeclaredButNotUsable:
+        return CompilationError(source_loc)
+               << "'" << name
+               << "' is not usable until after it has been completely declared";
+      case NameStatus::Usable:
+        return std::make_optional(it->second.entity);
     }
-    return std::make_optional(it->second.entity);
   }
   std::optional<ValueNodeView> result;
   for (Nonnull<const StaticScope*> parent : parent_scopes_) {

explorer/ast/static_scope.h

@@ -151,14 +151,29 @@ class ValueNodeView {
 // child scope.
 class StaticScope {
  public:
+  // The status of a name. Later enumerators with higher values correspond to
+  // more completely declared names.
+  enum class NameStatus {
+    // The name is known to exist in this scope, and any lookups finding it
+    // should be rejected because it's not declared yet.
+    KnownButNotDeclared,
+    // We've started processing a declaration of this name, but it's not yet
+    // fully declared, so any lookups finding it should be rejected.
+    DeclaredButNotUsable,
+    // The name is usable in this context.
+    Usable,
+  };
+
   // 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.
   // 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>;
+  auto Add(const std::string& name, ValueNodeView entity,
+           NameStatus status = NameStatus::Usable) -> ErrorOr<Success>;
 
-  // Marks `name` as usable.
+  // Marks `name` as being past its point of declaration.
+  void MarkDeclared(const std::string& name);
+  // Marks `name` as being completely declared and hence usable.
   void MarkUsable(const std::string& name);
 
   // Make `parent` a parent of this scope.
@@ -191,7 +206,7 @@ class StaticScope {
 
   struct Entry {
     ValueNodeView entity;
-    bool usable = false;
+    NameStatus status;
   };
   // Maps locally declared names to their entities.
   std::unordered_map<std::string, Entry> declared_names_;

explorer/interpreter/resolve_names.cpp

@@ -24,8 +24,9 @@ static auto AddExposedNames(const Declaration& declaration,
   switch (declaration.kind()) {
     case DeclarationKind::InterfaceDeclaration: {
       auto& iface_decl = cast<InterfaceDeclaration>(declaration);
-      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(iface_decl.name(), &iface_decl,
-                                                 /*usable=*/false));
+      CARBON_RETURN_IF_ERROR(
+          enclosing_scope.Add(iface_decl.name(), &iface_decl,
+                              StaticScope::NameStatus::KnownButNotDeclared));
       break;
     }
     case DeclarationKind::ImplDeclaration: {
@@ -34,27 +35,30 @@ static auto AddExposedNames(const Declaration& declaration,
     }
     case DeclarationKind::FunctionDeclaration: {
       auto& func = cast<FunctionDeclaration>(declaration);
-      CARBON_RETURN_IF_ERROR(
-          enclosing_scope.Add(func.name(), &func, /*usable=*/false));
+      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(
+          func.name(), &func, StaticScope::NameStatus::KnownButNotDeclared));
       break;
     }
     case DeclarationKind::ClassDeclaration: {
       auto& class_decl = cast<ClassDeclaration>(declaration);
-      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(class_decl.name(), &class_decl,
-                                                 /*usable=*/false));
+      CARBON_RETURN_IF_ERROR(
+          enclosing_scope.Add(class_decl.name(), &class_decl,
+                              StaticScope::NameStatus::KnownButNotDeclared));
       break;
     }
     case DeclarationKind::ChoiceDeclaration: {
       auto& choice = cast<ChoiceDeclaration>(declaration);
       CARBON_RETURN_IF_ERROR(
-          enclosing_scope.Add(choice.name(), &choice, /*usable=*/false));
+          enclosing_scope.Add(choice.name(), &choice,
+                              StaticScope::NameStatus::KnownButNotDeclared));
       break;
     }
     case DeclarationKind::VariableDeclaration: {
       auto& var = cast<VariableDeclaration>(declaration);
       if (var.binding().name() != AnonymousName) {
-        CARBON_RETURN_IF_ERROR(enclosing_scope.Add(
-            var.binding().name(), &var.binding(), /*usable=*/false));
+        CARBON_RETURN_IF_ERROR(
+            enclosing_scope.Add(var.binding().name(), &var.binding(),
+                                StaticScope::NameStatus::KnownButNotDeclared));
       }
       break;
     }
@@ -65,8 +69,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, /*usable=*/false));
+      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(
+          alias.name(), &alias, StaticScope::NameStatus::KnownButNotDeclared));
       break;
     }
   }
@@ -412,8 +416,9 @@ static auto ResolveNames(Statement& statement, StaticScope& enclosing_scope)
     }
     case StatementKind::Continuation: {
       auto& continuation = cast<Continuation>(statement);
-      CARBON_RETURN_IF_ERROR(enclosing_scope.Add(
-          continuation.name(), &continuation, /*usable=*/false));
+      CARBON_RETURN_IF_ERROR(
+          enclosing_scope.Add(continuation.name(), &continuation,
+                              StaticScope::NameStatus::DeclaredButNotUsable));
       StaticScope continuation_scope;
       continuation_scope.AddParent(&enclosing_scope);
       CARBON_RETURN_IF_ERROR(ResolveNames(cast<Continuation>(statement).body(),
@@ -461,6 +466,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope,
       auto& iface = cast<InterfaceDeclaration>(declaration);
       StaticScope iface_scope;
       iface_scope.AddParent(&enclosing_scope);
+      enclosing_scope.MarkDeclared(iface.name());
       if (iface.params().has_value()) {
         CARBON_RETURN_IF_ERROR(ResolveNames(**iface.params(), iface_scope));
       }
@@ -496,6 +502,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope,
       auto& function = cast<FunctionDeclaration>(declaration);
       StaticScope function_scope;
       function_scope.AddParent(&enclosing_scope);
+      enclosing_scope.MarkDeclared(function.name());
       for (Nonnull<GenericBinding*> binding : function.deduced_parameters()) {
         CARBON_RETURN_IF_ERROR(ResolveNames(*binding, function_scope));
       }
@@ -520,6 +527,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope,
       auto& class_decl = cast<ClassDeclaration>(declaration);
       StaticScope class_scope;
       class_scope.AddParent(&enclosing_scope);
+      enclosing_scope.MarkDeclared(class_decl.name());
       if (class_decl.type_params().has_value()) {
         CARBON_RETURN_IF_ERROR(
             ResolveNames(**class_decl.type_params(), class_scope));
@@ -532,6 +540,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope,
     }
     case DeclarationKind::ChoiceDeclaration: {
       auto& choice = cast<ChoiceDeclaration>(declaration);
+      enclosing_scope.MarkDeclared(choice.name());
       // 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.
@@ -564,6 +573,7 @@ static auto ResolveNames(Declaration& declaration, StaticScope& enclosing_scope,
 
     case DeclarationKind::AliasDeclaration: {
       auto& alias = cast<AliasDeclaration>(declaration);
+      enclosing_scope.MarkDeclared(alias.name());
       CARBON_RETURN_IF_ERROR(ResolveNames(alias.target(), enclosing_scope));
       enclosing_scope.MarkUsable(alias.name());
       break;

explorer/testdata/name_lookup/fail_class_fn_use_before_declaration.carbon

@@ -21,7 +21,7 @@ class A {
     return 0;
   }
 
-  // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/name_lookup/fail_class_fn_use_before_declaration.carbon:[[@LINE+1]]: 'I' is not usable until after it has been completely declared
+  // CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/name_lookup/fail_class_fn_use_before_declaration.carbon:[[@LINE+1]]: 'I' has not been declared yet
   fn H() -> I() {
     return 0;
   }

explorer/testdata/name_lookup/fail_var_use_before_declaration.carbon

@@ -13,7 +13,7 @@ package ExplorerTest api;
 // Test that a global variable may not depend on a later global.
 // Error expected.
 
-// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/name_lookup/fail_var_use_before_declaration.carbon:[[@LINE+1]]: 'y' is not usable until after it has been completely declared
+// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/name_lookup/fail_var_use_before_declaration.carbon:[[@LINE+1]]: 'y' has not been declared yet
 var x: i32 = y;
 
 var y: i32 = 0;