When diagnosing poisoned name, point to the declared name instead of the entire declaration (#4938)

See
https://discord.com/channels/655572317891461132/655578254970716160/1339007384361762857.

Part of #4622.

toolchain/check/context.cpp

@@ -225,14 +225,14 @@ auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
 }
 
 auto Context::DiagnosePoisonedName(SemIR::LocId poisoning_loc_id,
-                                   SemIR::InstId decl_inst_id) -> void {
+                                   SemIR::LocId decl_name_loc_id) -> void {
   CARBON_CHECK(poisoning_loc_id.has_value(),
                "Trying to diagnose poisoned name with no poisoning location");
   CARBON_DIAGNOSTIC(NameUseBeforeDecl, Error,
                     "name used before it was declared");
   CARBON_DIAGNOSTIC(NameUseBeforeDeclNote, Note, "declared here");
   emitter_->Build(poisoning_loc_id, NameUseBeforeDecl)
-      .Note(decl_inst_id, NameUseBeforeDeclNote)
+      .Note(decl_name_loc_id, NameUseBeforeDeclNote)
       .Emit();
 }
 

toolchain/check/context.h

@@ -265,7 +265,7 @@ class Context {
 
   // Prints a diagnostic for a poisoned name when it's later declared.
   auto DiagnosePoisonedName(SemIR::LocId poisoning_loc_id,
-                            SemIR::InstId decl_inst_id) -> void;
+                            SemIR::LocId decl_name_loc_id) -> void;
 
   // Prints a diagnostic for a missing name.
   auto DiagnoseNameNotFound(SemIRLoc loc, SemIR::NameId name_id) -> void;

toolchain/check/decl_name_stack.cpp

@@ -175,7 +175,8 @@ auto DeclNameStack::AddNameOrDiagnose(NameContext name_context,
                                       SemIR::InstId target_id,
                                       SemIR::AccessKind access_kind) -> void {
   if (name_context.state == DeclNameStack::NameContext::State::Poisoned) {
-    context_->DiagnosePoisonedName(name_context.poisoning_loc_id, target_id);
+    context_->DiagnosePoisonedName(name_context.poisoning_loc_id,
+                                   name_context.loc_id);
   } else if (auto id = name_context.prev_inst_id(); id.has_value()) {
     context_->DiagnoseDuplicateName(target_id, id);
   } else {

toolchain/check/handle_class.cpp

@@ -112,7 +112,7 @@ static auto MergeOrAddName(Context& context, Parse::AnyClassDeclId node_id,
   if (lookup_result.is_poisoned()) {
     // This is a declaration of a poisoned name.
     context.DiagnosePoisonedName(lookup_result.poisoning_loc_id(),
-                                 class_decl_id);
+                                 name_context.loc_id);
     return;
   }
 

toolchain/check/handle_function.cpp

@@ -273,7 +273,7 @@ static auto BuildFunctionDecl(Context& context,
 
   if (name_context.state == DeclNameStack::NameContext::State::Poisoned) {
     context.DiagnosePoisonedName(name_context.poisoning_loc_id,
-                                 function_info.latest_decl_id());
+                                 name_context.loc_id);
   } else {
     TryMergeRedecl(context, node_id, name_context.prev_inst_id(), function_decl,
                    function_info, is_definition);

toolchain/check/handle_interface.cpp

@@ -66,7 +66,7 @@ static auto BuildInterfaceDecl(Context& context,
   if (lookup_result.is_poisoned()) {
     // This is a declaration of a poisoned name.
     context.DiagnosePoisonedName(lookup_result.poisoning_loc_id(),
-                                 interface_decl_id);
+                                 name_context.loc_id);
   } else if (lookup_result.is_found()) {
     SemIR::InstId existing_id = lookup_result.target_inst_id();
     if (auto existing_interface_decl =

toolchain/check/handle_namespace.cpp

@@ -47,7 +47,7 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool {
                                                 SemIR::AccessKind::Public);
   if (lookup_result.is_poisoned()) {
     context.DiagnosePoisonedName(lookup_result.poisoning_loc_id(),
-                                 namespace_id);
+                                 name_context.loc_id);
   } else if (lookup_result.is_found()) {
     SemIR::InstId existing_inst_id = lookup_result.target_inst_id();
     if (auto existing =

toolchain/check/testdata/function/declaration/no_prelude/name_poisoning.carbon

@@ -45,9 +45,9 @@ fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
 // context without qualification.
-// CHECK:STDERR: fail_poison_class_without_usage.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_class_without_usage.carbon:[[@LINE+4]]:9: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N.C {}
-// CHECK:STDERR: ^~~~~~~~~~~
+// CHECK:STDERR:         ^
 // CHECK:STDERR:
 class N.C {}
 
@@ -66,9 +66,9 @@ fn N.F1(x: I);
 
 // Should fail here since I was poisoned for namespace N when it was used in N
 // context without qualification.
-// CHECK:STDERR: fail_poison_interface_without_usage.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_interface_without_usage.carbon:[[@LINE+4]]:13: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: interface N.I {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~
+// CHECK:STDERR:             ^
 // CHECK:STDERR:
 interface N.I {}
 
@@ -87,9 +87,9 @@ fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
 // context without qualification.
-// CHECK:STDERR: fail_poison_namespace_without_usage.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_namespace_without_usage.carbon:[[@LINE+4]]:13: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: namespace N.C;
-// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR:             ^
 // CHECK:STDERR:
 namespace N.C;
 
@@ -131,9 +131,9 @@ fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
 // context without qualification.
-// CHECK:STDERR: fail_poison_function_without_usage.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_function_without_usage.carbon:[[@LINE+4]]:6: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: fn N.C();
-// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR:      ^
 // CHECK:STDERR:
 fn N.C();
 
@@ -169,9 +169,9 @@ fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
 // context without qualification.
-// CHECK:STDERR: fail_poison_with_usage.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_with_usage.carbon:[[@LINE+4]]:9: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N.C {}
-// CHECK:STDERR: ^~~~~~~~~~~
+// CHECK:STDERR:         ^
 // CHECK:STDERR:
 class N.C {}
 
@@ -203,27 +203,27 @@ class N1.N2.N3.D1 {
       // CHECK:STDERR:               ^
       fn F(x: C);
 
-      // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:7: note: declared here [NameUseBeforeDeclNote]
+      // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:13: note: declared here [NameUseBeforeDeclNote]
       // CHECK:STDERR:       class C {}
-      // CHECK:STDERR:       ^~~~~~~~~
+      // CHECK:STDERR:             ^
       // CHECK:STDERR:
       // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-6]]:15: error: name used before it was declared [NameUseBeforeDecl]
       // CHECK:STDERR:       fn F(x: C);
       // CHECK:STDERR:               ^
       class C {}
     }
-    // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:5: note: declared here [NameUseBeforeDeclNote]
+    // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:11: note: declared here [NameUseBeforeDeclNote]
     // CHECK:STDERR:     class C {}
-    // CHECK:STDERR:     ^~~~~~~~~
+    // CHECK:STDERR:           ^
     // CHECK:STDERR:
     // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-15]]:15: error: name used before it was declared [NameUseBeforeDecl]
     // CHECK:STDERR:       fn F(x: C);
     // CHECK:STDERR:               ^
     class C {}
   }
-  // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:3: note: declared here [NameUseBeforeDeclNote]
+  // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:9: note: declared here [NameUseBeforeDeclNote]
   // CHECK:STDERR:   class C {}
-  // CHECK:STDERR:   ^~~~~~~~~
+  // CHECK:STDERR:         ^
   // CHECK:STDERR:
   // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-24]]:15: error: name used before it was declared [NameUseBeforeDecl]
   // CHECK:STDERR:       fn F(x: C);
@@ -231,27 +231,27 @@ class N1.N2.N3.D1 {
   class C {}
 }
 
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:10: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N1.C {}
-// CHECK:STDERR: ^~~~~~~~~~~~
+// CHECK:STDERR:          ^
 // CHECK:STDERR:
 // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-34]]:15: error: name used before it was declared [NameUseBeforeDecl]
 // CHECK:STDERR:       fn F(x: C);
 // CHECK:STDERR:               ^
 class N1.C {}
 
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:17: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: interface N1.N2.C {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:                 ^
 // CHECK:STDERR:
 // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-43]]:15: error: name used before it was declared [NameUseBeforeDecl]
 // CHECK:STDERR:       fn F(x: C);
 // CHECK:STDERR:               ^
 interface N1.N2.C {}
 
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+4]]:16: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N1.N2.N3.C {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:                ^
 // CHECK:STDERR:
 class N1.N2.N3.C {}
 
@@ -334,17 +334,17 @@ namespace N;
 fn N.F(x: C);
 
 // TODO: We should ideally only produce one diagnostic here.
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+7]]:7: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class C {}
-// CHECK:STDERR: ^~~~~~~~~
+// CHECK:STDERR:       ^
 // CHECK:STDERR:
 // CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE-7]]:11: error: name used before it was declared [NameUseBeforeDecl]
 // CHECK:STDERR: fn N.F(x: C);
 // CHECK:STDERR:           ^
 class C {}
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+4]]:9: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N.C {}
-// CHECK:STDERR: ^~~~~~~~~~~
+// CHECK:STDERR:         ^
 // CHECK:STDERR:
 class N.C {}
 
@@ -361,9 +361,9 @@ fn F() {
     // CHECK:STDERR:            ^
     var v: A;
 
-    // CHECK:STDERR: fail_poison_with_lexical_result.carbon:[[@LINE+4]]:5: note: declared here [NameUseBeforeDeclNote]
+    // CHECK:STDERR: fail_poison_with_lexical_result.carbon:[[@LINE+4]]:11: note: declared here [NameUseBeforeDeclNote]
     // CHECK:STDERR:     class A {}
-    // CHECK:STDERR:     ^~~~~~~~~
+    // CHECK:STDERR:           ^
     // CHECK:STDERR:
     class A {}
   }

toolchain/check/testdata/interface/no_prelude/import_access.carbon

@@ -110,9 +110,9 @@ impl package Test library "[[@TEST_NAME]]";
 // CHECK:STDERR:         ^~~~~~~
 fn F(i: Forward*) {}
 
-// CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+4]]:11: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: interface Forward {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
+// CHECK:STDERR:           ^~~~~~~
 // CHECK:STDERR:
 interface Forward {}