Improve diagnostics for missing qualified names. (#4638)

Mention the scope in which the name wasn't found. --------- Co-authored-by: Geoff Romer <[email protected]>
carbon-language · Dec 9, 2024 · e75ef34 · e75ef34
1 parent 4e3b8c9
commit e75ef34
Show file tree

Hide file tree

Showing 23 changed files with 1,243 additions and 25 deletions.
diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp
@@ -222,6 +222,54 @@ auto Context::DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id)
   emitter_->Emit(loc, NameNotFound, name_id);
 }
 
+// Given an instruction associated with a scope and a `SpecificId` for that
+// scope, returns an instruction that describes the specific scope.
+static auto GetInstForSpecificScope(Context& context, SemIR::InstId inst_id,
+                                    SemIR::SpecificId specific_id)
+    -> SemIR::InstId {
+  if (!specific_id.is_valid()) {
+    return inst_id;
+  }
+  auto inst = context.insts().Get(inst_id);
+  CARBON_KIND_SWITCH(inst) {
+    case CARBON_KIND(SemIR::ClassDecl class_decl): {
+      return context.types().GetInstId(
+          context.GetClassType(class_decl.class_id, specific_id));
+    }
+    case CARBON_KIND(SemIR::InterfaceDecl interface_decl): {
+      return context.types().GetInstId(
+          context.GetInterfaceType(interface_decl.interface_id, specific_id));
+    }
+    default: {
+      // Don't know how to form a specific for this generic scope.
+      // TODO: Handle more cases.
+      return SemIR::InstId::Invalid;
+    }
+  }
+}
+
+auto Context::DiagnoseMemberNameNotFound(
+    SemIRLoc loc, SemIR::NameId name_id,
+    llvm::ArrayRef<LookupScope> lookup_scopes) -> void {
+  if (lookup_scopes.size() == 1 &&
+      lookup_scopes.front().name_scope_id.is_valid()) {
+    const auto& scope = name_scopes().Get(lookup_scopes.front().name_scope_id);
+    if (auto specific_inst_id = GetInstForSpecificScope(
+            *this, scope.inst_id(), lookup_scopes.front().specific_id);
+        specific_inst_id.is_valid()) {
+      CARBON_DIAGNOSTIC(MemberNameNotFoundInScope, Error,
+                        "member name `{0}` not found in {1}", SemIR::NameId,
+                        InstIdAsType);
+      emitter_->Emit(loc, MemberNameNotFoundInScope, name_id, specific_inst_id);
+      return;
+    }
+  }
+
+  CARBON_DIAGNOSTIC(MemberNameNotFound, Error, "member name `{0}` not found",
+                    SemIR::NameId);
+  emitter_->Emit(loc, MemberNameNotFound, name_id);
+}
+
 auto Context::NoteAbstractClass(SemIR::ClassId class_id,
                                 DiagnosticBuilder& builder) -> void {
   const auto& class_info = classes().Get(class_id);
@@ -590,7 +638,7 @@ auto Context::LookupQualifiedName(SemIRLoc loc, SemIR::NameId name_id,
   if (required && !result.inst_id.is_valid()) {
     if (!has_error) {
       if (prohibited_accesses.empty()) {
-        DiagnoseNameNotFound(loc, name_id);
+        DiagnoseMemberNameNotFound(loc, name_id, lookup_scopes);
       } else {
         //  TODO: We should report multiple prohibited accesses in case we don't
         //  find a valid lookup. Reporting the last one should suffice for now.
@@ -1361,6 +1409,11 @@ auto Context::GetSingletonType(SemIR::InstId singleton_id) -> SemIR::TypeId {
   return type_id;
 }
 
+auto Context::GetClassType(SemIR::ClassId class_id,
+                           SemIR::SpecificId specific_id) -> SemIR::TypeId {
+  return GetCompleteTypeImpl<SemIR::ClassType>(*this, class_id, specific_id);
+}
+
 auto Context::GetFunctionType(SemIR::FunctionId fn_id,
                               SemIR::SpecificId specific_id) -> SemIR::TypeId {
   return GetCompleteTypeImpl<SemIR::FunctionType>(*this, fn_id, specific_id);

diff --git a/toolchain/check/context.h b/toolchain/check/context.h
@@ -248,6 +248,11 @@ class Context {
   // Prints a diagnostic for a missing name.
   auto DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id) -> void;
 
+  // Prints a diagnostic for a missing qualified name.
+  auto DiagnoseMemberNameNotFound(SemIRLoc loc, SemIR::NameId name_id,
+                                  llvm::ArrayRef<LookupScope> lookup_scopes)
+      -> void;
+
   // Adds a note to a diagnostic explaining that a class is incomplete.
   auto NoteIncompleteClass(SemIR::ClassId class_id, DiagnosticBuilder& builder)
       -> void;
@@ -386,6 +391,10 @@ class Context {
   // `singleton_id` is already validated to be a singleton.
   auto GetSingletonType(SemIR::InstId singleton_id) -> SemIR::TypeId;
 
+  // Gets a class type.
+  auto GetClassType(SemIR::ClassId class_id, SemIR::SpecificId specific_id)
+      -> SemIR::TypeId;
+
   // Gets a function type. The returned type will be complete.
   auto GetFunctionType(SemIR::FunctionId fn_id, SemIR::SpecificId specific_id)
       -> SemIR::TypeId;

diff --git a/toolchain/check/testdata/alias/no_prelude/fail_local_in_namespace.carbon b/toolchain/check/testdata/alias/no_prelude/fail_local_in_namespace.carbon
@@ -20,7 +20,7 @@ fn F() -> {} {
   // CHECK:STDERR:                ^~
   // CHECK:STDERR:
   alias NS.a = {};
-  // CHECK:STDERR: fail_local_in_namespace.carbon:[[@LINE+3]]:10: error: name `a` not found [NameNotFound]
+  // CHECK:STDERR: fail_local_in_namespace.carbon:[[@LINE+3]]:10: error: member name `a` not found in `NS` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   return NS.a;
   // CHECK:STDERR:          ^~~~
   return NS.a;

diff --git a/toolchain/check/testdata/alias/no_prelude/import_access.carbon b/toolchain/check/testdata/alias/no_prelude/import_access.carbon
@@ -47,7 +47,7 @@ package Other library "[[@TEST_NAME]]";
 
 import Test library "def";
 
-// CHECK:STDERR: fail_other_def.carbon:[[@LINE+3]]:11: error: name `A` not found [NameNotFound]
+// CHECK:STDERR: fail_other_def.carbon:[[@LINE+3]]:11: error: member name `A` not found in `Test` [MemberNameNotFoundInScope]
 // CHECK:STDERR: var inst: Test.A = {};
 // CHECK:STDERR:           ^~~~~~
 var inst: Test.A = {};

diff --git a/toolchain/check/testdata/class/adapter/adapt.carbon b/toolchain/check/testdata/class/adapter/adapt.carbon
@@ -39,7 +39,7 @@ class AdaptNotExtend {
 
 fn F(a: AdaptNotExtend) {
   // `Adapted` is not extended, so lookup for `F` finds nothing.
-  // CHECK:STDERR: fail_not_extend.carbon:[[@LINE+3]]:3: error: name `F` not found [NameNotFound]
+  // CHECK:STDERR: fail_not_extend.carbon:[[@LINE+3]]:3: error: member name `F` not found in `AdaptNotExtend` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   a.F();
   // CHECK:STDERR:   ^~~
   a.F();

diff --git a/toolchain/check/testdata/class/adapter/extend_adapt.carbon b/toolchain/check/testdata/class/adapter/extend_adapt.carbon
@@ -95,7 +95,7 @@ class StructAdapter {
 
 fn F(a: StructAdapter) -> i32 {
   // TODO: This should be allowed.
-  // CHECK:STDERR: fail_todo_adapt_struct.carbon:[[@LINE+4]]:10: error: name `b` not found [NameNotFound]
+  // CHECK:STDERR: fail_todo_adapt_struct.carbon:[[@LINE+4]]:10: error: member name `b` not found in `StructAdapter` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   return a.b;
   // CHECK:STDERR:          ^~~
   // CHECK:STDERR:
@@ -131,7 +131,7 @@ class IntAdapter {
 
 fn F(a: IntAdapter) -> i32 {
   // Builtin types have no member names.
-  // CHECK:STDERR: fail_adapt_builtin.carbon:[[@LINE+3]]:10: error: name `foo` not found [NameNotFound]
+  // CHECK:STDERR: fail_adapt_builtin.carbon:[[@LINE+3]]:10: error: member name `foo` not found in `IntAdapter` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   return a.foo;
   // CHECK:STDERR:          ^~~~~
   return a.foo;

diff --git a/toolchain/check/testdata/class/adapter/fail_adapt_bad_decl.carbon b/toolchain/check/testdata/class/adapter/fail_adapt_bad_decl.carbon
@@ -23,7 +23,7 @@ class Bad {
   adapt 100;
 }
 
-// CHECK:STDERR: fail_not_type.carbon:[[@LINE+4]]:18: error: name `F` not found [NameNotFound]
+// CHECK:STDERR: fail_not_type.carbon:[[@LINE+4]]:18: error: member name `F` not found in `Bad` [MemberNameNotFoundInScope]
 // CHECK:STDERR: fn Use(b: Bad) { b.F(); }
 // CHECK:STDERR:                  ^~~
 // CHECK:STDERR:

diff --git a/toolchain/check/testdata/class/fail_base_bad_type.carbon b/toolchain/check/testdata/class/fail_base_bad_type.carbon
@@ -174,7 +174,7 @@ fn AccessMemberWithInvalidBaseFinal_WithMember(p: DeriveFromFinal*) -> i32 {
 }
 
 fn AccessMemberWithInvalidBaseFinal_NoMember(p: DeriveFromFinal*) -> i32 {
-  // CHECK:STDERR: fail_derive_from_final.carbon:[[@LINE+3]]:10: error: name `b` not found [NameNotFound]
+  // CHECK:STDERR: fail_derive_from_final.carbon:[[@LINE+3]]:10: error: member name `b` not found in `DeriveFromFinal` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   return (*p).b;
   // CHECK:STDERR:          ^~~~~~
   return (*p).b;

diff --git a/toolchain/check/testdata/class/fail_unknown_member.carbon b/toolchain/check/testdata/class/fail_unknown_member.carbon
@@ -14,7 +14,7 @@ class Class {
 
 fn G(c: Class) -> i32 {
   // TODO: Mention the scope in which we looked for the name.
-  // CHECK:STDERR: fail_unknown_member.carbon:[[@LINE+3]]:10: error: name `something` not found [NameNotFound]
+  // CHECK:STDERR: fail_unknown_member.carbon:[[@LINE+3]]:10: error: member name `something` not found in `Class` [MemberNameNotFoundInScope]
   // CHECK:STDERR:   return c.something;
   // CHECK:STDERR:          ^~~~~~~~~~~
   return c.something;