When diagnosing name used before declared, set the location of the usage (#4860)

Done by adding a poisoning location for each poisoned name.
Part of #4622.

toolchain/check/context.cpp

@@ -223,15 +223,15 @@ auto Context::DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def)
       .Emit();
 }
 
-auto Context::DiagnosePoisonedName(SemIR::InstId loc) -> void {
-  // TODO: Improve the diagnostic to replace NodeId::None with the location
-  // where the name was poisoned. See discussion in
-  // https://github.com/carbon-language/carbon-lang/pull/4654#discussion_r1876607172
+auto Context::DiagnosePoisonedName(SemIR::LocId poisoning_loc_id,
+                                   SemIR::InstId decl_inst_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(SemIR::LocId::None, NameUseBeforeDecl)
-      .Note(loc, NameUseBeforeDeclNote)
+  emitter_->Build(poisoning_loc_id, NameUseBeforeDecl)
+      .Note(decl_inst_id, NameUseBeforeDeclNote)
       .Emit();
 }
 
@@ -421,13 +421,14 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id,
           .scope_result = SemIR::ScopeLookupResult::MakeError()};
 }
 
-auto Context::LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id,
+auto Context::LookupNameInExactScope(SemIR::LocId loc_id, SemIR::NameId name_id,
                                      SemIR::NameScopeId scope_id,
                                      SemIR::NameScope& scope,
                                      bool is_being_declared)
     -> SemIR::ScopeLookupResult {
-  if (auto entry_id = is_being_declared ? scope.Lookup(name_id)
-                                        : scope.LookupOrPoison(name_id)) {
+  if (auto entry_id = is_being_declared
+                          ? scope.Lookup(name_id)
+                          : scope.LookupOrPoison(loc_id, name_id)) {
     auto lookup_result = scope.GetEntry(*entry_id).result;
     if (!lookup_result.is_poisoned()) {
       LoadImportRef(*this, lookup_result.target_inst_id());
@@ -438,7 +439,7 @@ auto Context::LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id,
   if (!scope.import_ir_scopes().empty()) {
     // TODO: Enforce other access modifiers for imports.
     return SemIR::ScopeLookupResult::MakeWrappedLookupResult(
-        ImportNameFromOtherPackage(*this, loc, scope_id,
+        ImportNameFromOtherPackage(*this, loc_id, scope_id,
                                    scope.import_ir_scopes(), name_id),
         SemIR::AccessKind::Public);
   }

toolchain/check/context.h

@@ -215,9 +215,7 @@ class Context {
 
   // Performs name lookup in a specified scope for a name appearing in a
   // declaration. If scope_id is `None`, performs lookup into the lexical scope
-  // specified by scope_index instead. If found, returns the referenced
-  // `InstId` and false. If poisoned, returns `InstId::None` and true.
-  // TODO: For poisoned names, return the poisoning `InstId`.
+  // specified by scope_index instead.
   auto LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id,
                         SemIR::NameScopeId scope_id, ScopeIndex scope_index)
       -> SemIR::ScopeLookupResult;
@@ -235,7 +233,7 @@ class Context {
   // poisoned name will be treated as if it is not declared. Otherwise, this is
   // a lookup for a name being declared, so the name will not be poisoned, but
   // poison will be returned if it's already been looked up.
-  auto LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id,
+  auto LookupNameInExactScope(SemIR::LocId loc_id, SemIR::NameId name_id,
                               SemIR::NameScopeId scope_id,
                               SemIR::NameScope& scope,
                               bool is_being_declared = false)
@@ -265,8 +263,9 @@ class Context {
   // Prints a diagnostic for a duplicate name.
   auto DiagnoseDuplicateName(SemIRLoc dup_def, SemIRLoc prev_def) -> void;
 
-  // Prints a diagnostic for a poisoned name.
-  auto DiagnosePoisonedName(SemIR::InstId loc) -> void;
+  // 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;
 
   // 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,7 @@ 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(target_id);
+    context_->DiagnosePoisonedName(name_context.poisoning_loc_id, target_id);
   } else if (auto id = name_context.prev_inst_id(); id.has_value()) {
     context_->DiagnoseDuplicateName(target_id, id);
   } else {
@@ -188,7 +188,8 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context,
                                     SemIR::AccessKind access_kind)
     -> SemIR::ScopeLookupResult {
   if (name_context.state == NameContext::State::Poisoned) {
-    return SemIR::ScopeLookupResult::MakePoisoned();
+    return SemIR::ScopeLookupResult::MakePoisoned(
+        name_context.poisoning_loc_id);
   }
   if (auto id = name_context.prev_inst_id(); id.has_value()) {
     return SemIR::ScopeLookupResult::MakeFound(id, access_kind);
@@ -267,6 +268,7 @@ auto DeclNameStack::ApplyAndLookupName(NameContext& name_context,
       name_context.initial_scope_index);
   if (lookup_result.is_poisoned()) {
     name_context.unresolved_name_id = name_id;
+    name_context.poisoning_loc_id = lookup_result.poisoning_loc_id();
     name_context.state = NameContext::State::Poisoned;
   } else if (!lookup_result.is_found()) {
     // Invalid indicates an unresolved name. Store it and return.

toolchain/check/decl_name_stack.h

@@ -156,6 +156,10 @@ class DeclNameStack {
       // The ID of an unresolved identifier.
       SemIR::NameId unresolved_name_id = SemIR::NameId::None;
     };
+
+    // When `state` is `Poisoned` (name is unresolved due to name poisoning),
+    // the poisoning location.
+    SemIR::LocId poisoning_loc_id = SemIR::LocId::None;
   };
 
   // Information about a declaration name that has been temporarily removed from

toolchain/check/handle_class.cpp

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

toolchain/check/handle_function.cpp

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

toolchain/check/handle_interface.cpp

@@ -65,7 +65,8 @@ static auto BuildInterfaceDecl(Context& context,
           introducer.modifier_set.GetAccessKind());
   if (lookup_result.is_poisoned()) {
     // This is a declaration of a poisoned name.
-    context.DiagnosePoisonedName(interface_decl_id);
+    context.DiagnosePoisonedName(lookup_result.poisoning_loc_id(),
+                                 interface_decl_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

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

toolchain/check/impl.cpp

@@ -376,7 +376,8 @@ auto FinishImplWitness(Context& context, SemIR::Impl& impl) -> void {
         }
         auto& fn = context.functions().Get(fn_type->function_id);
         auto lookup_result = context.LookupNameInExactScope(
-            decl_id, fn.name_id, impl.scope_id, impl_scope);
+            context.insts().GetLocId(decl_id), fn.name_id, impl.scope_id,
+            impl_scope);
         if (lookup_result.is_found()) {
           used_decl_ids.push_back(lookup_result.target_inst_id());
           witness_block[index] = CheckAssociatedFunctionImplementation(

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

@@ -31,7 +31,6 @@ namespace N;
 fn N.F1(x: C);
 
 // --- fail_poison_class_without_usage.carbon
-// CHECK:STDERR: fail_poison_class_without_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -39,6 +38,9 @@ class C {};
 
 namespace N;
 // Here we use C and poison N.C.
+// CHECK:STDERR: fail_poison_class_without_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F1(x: C);
+// CHECK:STDERR:            ^
 fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
@@ -50,7 +52,6 @@ fn N.F1(x: C);
 class N.C {}
 
 // --- fail_poison_interface_without_usage.carbon
-// CHECK:STDERR: fail_poison_interface_without_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -58,6 +59,9 @@ interface I {};
 
 namespace N;
 // Here we use I and poison N.I.
+// CHECK:STDERR: fail_poison_interface_without_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F1(x: I);
+// CHECK:STDERR:            ^
 fn N.F1(x: I);
 
 // Should fail here since I was poisoned for namespace N when it was used in N
@@ -69,7 +73,6 @@ fn N.F1(x: I);
 interface N.I {}
 
 // --- fail_poison_namespace_without_usage.carbon
-// CHECK:STDERR: fail_poison_namespace_without_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -77,6 +80,9 @@ class C {};
 
 namespace N;
 // Here we use C and poison N.C.
+// CHECK:STDERR: fail_poison_namespace_without_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F1(x: C);
+// CHECK:STDERR:            ^
 fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
@@ -88,7 +94,6 @@ fn N.F1(x: C);
 namespace N.C;
 
 // --- fail_poison_member_without_usage.carbon
-// CHECK:STDERR: fail_poison_member_without_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -96,6 +101,9 @@ class C1 {};
 
 class D {
   // Here we use C1 and poison D.C1.
+  // CHECK:STDERR: fail_poison_member_without_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+  // CHECK:STDERR:   fn F1(x: C1);
+  // CHECK:STDERR:            ^~
   fn F1(x: C1);
 
   class C2 {};
@@ -109,7 +117,6 @@ class D {
 }
 
 // --- fail_poison_function_without_usage.carbon
-// CHECK:STDERR: fail_poison_function_without_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -117,6 +124,9 @@ class C {};
 
 namespace N;
 // Here we use C and poison N.C.
+// CHECK:STDERR: fail_poison_function_without_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F1(x: C);
+// CHECK:STDERR:            ^
 fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
@@ -145,7 +155,6 @@ fn N.F1() -> C;
 fn N.F2() -> N.C;
 
 // --- fail_poison_with_usage.carbon
-// CHECK:STDERR: fail_poison_with_usage.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -153,6 +162,9 @@ class C {};
 
 namespace N;
 // Here we use C and poison N.C.
+// CHECK:STDERR: fail_poison_with_usage.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F1(x: C);
+// CHECK:STDERR:            ^
 fn N.F1(x: C);
 
 // Should fail here since C was poisoned for namespace N when it was used in N
@@ -168,7 +180,6 @@ class N.C {}
 fn N.F2(x: C) { N.F1(x); }
 
 // --- fail_poison_multiple_scopes.carbon
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -187,42 +198,55 @@ class N1.N2.N3.D1 {
       // * N1.N2.N3.D1.C
       // * N1.N2.N3.D1.D2.C
       // * N1.N2.N3.D1.D2.D3.C
+      // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+3]]:15: error: name used before it was declared [NameUseBeforeDecl]
+      // CHECK:STDERR:       fn F(x: C);
+      // CHECK:STDERR:               ^
       fn F(x: C);
 
-      // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+5]]:7: note: declared here [NameUseBeforeDeclNote]
+      // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:7: note: declared here [NameUseBeforeDeclNote]
       // CHECK:STDERR:       class C {}
       // CHECK:STDERR:       ^~~~~~~~~
       // CHECK:STDERR:
-      // CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
+      // 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+5]]:5: note: declared here [NameUseBeforeDeclNote]
+    // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:5: note: declared here [NameUseBeforeDeclNote]
     // CHECK:STDERR:     class C {}
     // CHECK:STDERR:     ^~~~~~~~~
     // CHECK:STDERR:
-    // CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
+    // 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+5]]:3: note: declared here [NameUseBeforeDeclNote]
+  // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:3: note: declared here [NameUseBeforeDeclNote]
   // CHECK:STDERR:   class C {}
   // CHECK:STDERR:   ^~~~~~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
+  // CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE-24]]: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+5]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class N1.C {}
 // CHECK:STDERR: ^~~~~~~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
+// 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+5]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_multiple_scopes.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: interface N1.N2.C {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_poison_multiple_scopes.carbon: error: name used before it was declared [NameUseBeforeDecl]
+// 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]
@@ -232,13 +256,15 @@ interface N1.N2.C {}
 class N1.N2.N3.C {}
 
 // --- fail_alias.carbon
-// CHECK:STDERR: fail_alias.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
 class C {}
 
 namespace N;
+// CHECK:STDERR: fail_alias.carbon:[[@LINE+7]]:13: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: alias N.C = C;
+// CHECK:STDERR:             ^
 // CHECK:STDERR: fail_alias.carbon:[[@LINE+4]]:9: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: alias N.C = C;
 // CHECK:STDERR:         ^
@@ -298,19 +324,23 @@ class X {
 library "[[@TEST_NAME]]";
 
 namespace N;
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+5]]:11: error: name `C` not found [NameNotFound]
+// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+7]]:11: error: name `C` not found [NameNotFound]
 // CHECK:STDERR: fn N.F(x: C);
 // CHECK:STDERR:           ^
 // CHECK:STDERR:
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+3]]:11: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn N.F(x: C);
+// CHECK:STDERR:           ^
 fn N.F(x: C);
 
 // TODO: We should ideally only produce one diagnostic here.
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+5]]:1: note: declared here [NameUseBeforeDeclNote]
+// CHECK:STDERR: fail_poison_when_lookup_fails.carbon:[[@LINE+7]]:1: note: declared here [NameUseBeforeDeclNote]
 // CHECK:STDERR: class C {}
 // CHECK:STDERR: ^~~~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_poison_when_lookup_fails.carbon: error: name used before it was declared [NameUseBeforeDecl]
+// 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: class N.C {}
@@ -319,7 +349,6 @@ class C {}
 class N.C {}
 
 // --- fail_poison_with_lexical_result.carbon
-// CHECK:STDERR: fail_poison_with_lexical_result.carbon: error: name used before it was declared [NameUseBeforeDecl]
 
 library "[[@TEST_NAME]]";
 
@@ -327,6 +356,9 @@ fn F() {
   class A {}
 
   class B {
+    // CHECK:STDERR: fail_poison_with_lexical_result.carbon:[[@LINE+3]]:12: error: name used before it was declared [NameUseBeforeDecl]
+    // CHECK:STDERR:     var v: A;
+    // CHECK:STDERR:            ^
     var v: A;
 
     // CHECK:STDERR: fail_poison_with_lexical_result.carbon:[[@LINE+4]]:5: note: declared here [NameUseBeforeDeclNote]
@@ -460,10 +492,10 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .C = %C.decl.loc5
+// CHECK:STDOUT:     .C = %C.decl.loc4
 // CHECK:STDOUT:     .N = %N
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc5: type = class_decl @C.1 [template = constants.%C.f79] {} {}
+// CHECK:STDOUT:   %C.decl.loc4: type = class_decl @C.1 [template = constants.%C.f79] {} {}
 // CHECK:STDOUT:   %N: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .F1 = %F1.decl
 // CHECK:STDOUT:   }
@@ -472,10 +504,10 @@ fn F() {
 // CHECK:STDOUT:     %x.param_patt: %C.f79 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %x.param: %C.f79 = value_param runtime_param0
-// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc5 [template = constants.%C.f79]
+// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc4 [template = constants.%C.f79]
 // CHECK:STDOUT:     %x: %C.f79 = bind_name x, %x.param
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc17: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
+// CHECK:STDOUT:   %C.decl.loc19: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C.1 {
@@ -509,10 +541,10 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .I = %I.decl.loc5
+// CHECK:STDOUT:     .I = %I.decl.loc4
 // CHECK:STDOUT:     .N = %N
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %I.decl.loc5: type = interface_decl @I.1 [template = constants.%I.type.733] {} {}
+// CHECK:STDOUT:   %I.decl.loc4: type = interface_decl @I.1 [template = constants.%I.type.733] {} {}
 // CHECK:STDOUT:   %N: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .F1 = %F1.decl
 // CHECK:STDOUT:   }
@@ -521,10 +553,10 @@ fn F() {
 // CHECK:STDOUT:     %x.param_patt: %I.type.733 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %x.param: %I.type.733 = value_param runtime_param0
-// CHECK:STDOUT:     %I.ref: type = name_ref I, file.%I.decl.loc5 [template = constants.%I.type.733]
+// CHECK:STDOUT:     %I.ref: type = name_ref I, file.%I.decl.loc4 [template = constants.%I.type.733]
 // CHECK:STDOUT:     %x: %I.type.733 = bind_name x, %x.param
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %I.decl.loc17: type = interface_decl @I.2 [template = constants.%I.type.4da] {} {}
+// CHECK:STDOUT:   %I.decl.loc19: type = interface_decl @I.2 [template = constants.%I.type.4da] {} {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: interface @I.1 {
@@ -627,9 +659,9 @@ fn F() {
 // CHECK:STDOUT:     %x: %C1 = bind_name x, %x.param
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %C2.decl: type = class_decl @C2 [template = constants.%C2] {} {}
-// CHECK:STDOUT:   %.loc18_9: %D.elem = field_decl C1, element0 [template]
+// CHECK:STDOUT:   %.loc20_9: %D.elem = field_decl C1, element0 [template]
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %.loc18_3: %D.elem = var_pattern %.loc18_9
+// CHECK:STDOUT:     %.loc20_3: %D.elem = var_pattern %.loc20_9
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.var: ref %D.elem = var <none>
 // CHECK:STDOUT:   %complete_type: <witness> = complete_type_witness %struct_type.C1 [template = constants.%complete_type.ec1]
@@ -665,10 +697,10 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .C = %C.decl.loc5
+// CHECK:STDOUT:     .C = %C.decl.loc4
 // CHECK:STDOUT:     .N = %N
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc5: type = class_decl @C.2 [template = constants.%C.f79] {} {}
+// CHECK:STDOUT:   %C.decl.loc4: type = class_decl @C.2 [template = constants.%C.f79] {} {}
 // CHECK:STDOUT:   %N: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .F1 = %F1.decl
 // CHECK:STDOUT:   }
@@ -677,10 +709,10 @@ fn F() {
 // CHECK:STDOUT:     %x.param_patt: %C.f79 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %x.param: %C.f79 = value_param runtime_param0
-// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc5 [template = constants.%C.f79]
+// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc4 [template = constants.%C.f79]
 // CHECK:STDOUT:     %x: %C.f79 = bind_name x, %x.param
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc17: %C.type = fn_decl @C.1 [template = constants.%C.3f2] {} {}
+// CHECK:STDOUT:   %C.decl.loc19: %C.type = fn_decl @C.1 [template = constants.%C.3f2] {} {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C.2 {
@@ -764,10 +796,10 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .C = %C.decl.loc5
+// CHECK:STDOUT:     .C = %C.decl.loc4
 // CHECK:STDOUT:     .N = %N
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc5: type = class_decl @C.1 [template = constants.%C.f79] {} {}
+// CHECK:STDOUT:   %C.decl.loc4: type = class_decl @C.1 [template = constants.%C.f79] {} {}
 // CHECK:STDOUT:   %N: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .F1 = %F1.decl
 // CHECK:STDOUT:     .F2 = %F2.decl
@@ -777,16 +809,16 @@ fn F() {
 // CHECK:STDOUT:     %x.param_patt: %C.f79 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %x.param: %C.f79 = value_param runtime_param0
-// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc5 [template = constants.%C.f79]
+// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc4 [template = constants.%C.f79]
 // CHECK:STDOUT:     %x: %C.f79 = bind_name x, %x.param
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc17: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
+// CHECK:STDOUT:   %C.decl.loc19: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
 // CHECK:STDOUT:   %F2.decl: %F2.type = fn_decl @F2 [template = constants.%F2] {
 // CHECK:STDOUT:     %x.patt: %C.f79 = binding_pattern x
 // CHECK:STDOUT:     %x.param_patt: %C.f79 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %x.param: %C.f79 = value_param runtime_param0
-// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc5 [template = constants.%C.f79]
+// CHECK:STDOUT:     %C.ref: type = name_ref C, file.%C.decl.loc4 [template = constants.%C.f79]
 // CHECK:STDOUT:     %x: %C.f79 = bind_name x, %x.param
 // CHECK:STDOUT:   }
 // CHECK:STDOUT: }
@@ -844,10 +876,10 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
-// CHECK:STDOUT:     .C = %C.decl.loc5
+// CHECK:STDOUT:     .C = %C.decl.loc4
 // CHECK:STDOUT:     .N1 = %N1
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc5: type = class_decl @C.1 [template = constants.%C.f79] {} {}
+// CHECK:STDOUT:   %C.decl.loc4: type = class_decl @C.1 [template = constants.%C.f79] {} {}
 // CHECK:STDOUT:   %N1: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .N2 = %N2
 // CHECK:STDOUT:   }
@@ -858,9 +890,9 @@ fn F() {
 // CHECK:STDOUT:     .D1 = %D1.decl
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %D1.decl: type = class_decl @D1 [template = constants.%D1] {} {}
-// CHECK:STDOUT:   %C.decl.loc49: type = class_decl @C.5 [template = constants.%C.0b8] {} {}
-// CHECK:STDOUT:   %C.decl.loc56: type = interface_decl @C.7 [template = constants.%C.type] {} {}
-// CHECK:STDOUT:   %C.decl.loc62: type = class_decl @C.6 [template = constants.%C.6f1] {} {}
+// CHECK:STDOUT:   %C.decl.loc59: type = class_decl @C.5 [template = constants.%C.0b8] {} {}
+// CHECK:STDOUT:   %C.decl.loc68: type = interface_decl @C.7 [template = constants.%C.type] {} {}
+// CHECK:STDOUT:   %C.decl.loc74: type = class_decl @C.6 [template = constants.%C.6f1] {} {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: interface @D2 {
@@ -913,7 +945,7 @@ fn F() {
 // CHECK:STDOUT:       %x.param_patt: %C.f79 = value_param_pattern %x.patt, runtime_param0
 // CHECK:STDOUT:     } {
 // CHECK:STDOUT:       %x.param: %C.f79 = value_param runtime_param0
-// CHECK:STDOUT:       %C.ref: type = name_ref C, file.%C.decl.loc5 [template = constants.%C.f79]
+// CHECK:STDOUT:       %C.ref: type = name_ref C, file.%C.decl.loc4 [template = constants.%C.f79]
 // CHECK:STDOUT:       %x: %C.f79 = bind_name x, %x.param
 // CHECK:STDOUT:     }
 // CHECK:STDOUT:     %C.decl: type = class_decl @C.2 [template = constants.%C.fef] {} {}
@@ -1216,8 +1248,8 @@ fn F() {
 // CHECK:STDOUT:     %C.ref: <error> = name_ref C, <error> [template = <error>]
 // CHECK:STDOUT:     %x: <error> = bind_name x, %x.param
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %C.decl.loc18: type = class_decl @C.1 [template = constants.%C.f79] {} {}
-// CHECK:STDOUT:   %C.decl.loc23: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
+// CHECK:STDOUT:   %C.decl.loc22: type = class_decl @C.1 [template = constants.%C.f79] {} {}
+// CHECK:STDOUT:   %C.decl.loc27: type = class_decl @C.2 [template = constants.%C.9f4] {} {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C.1 {
@@ -1269,9 +1301,9 @@ fn F() {
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @B {
-// CHECK:STDOUT:   %.loc9_10: %B.elem = field_decl v, element0 [template]
+// CHECK:STDOUT:   %.loc11_10: %B.elem = field_decl v, element0 [template]
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %.loc9_5: %B.elem = var_pattern %.loc9_10
+// CHECK:STDOUT:     %.loc11_5: %B.elem = var_pattern %.loc11_10
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.var: ref %B.elem = var <none>
 // CHECK:STDOUT:   %A.decl: type = class_decl @A.2 [template = constants.%A.9b6] {} {}
@@ -1280,7 +1312,7 @@ fn F() {
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .Self = constants.%B
-// CHECK:STDOUT:   .v = %.loc9_10
+// CHECK:STDOUT:   .v = %.loc11_10
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @A.2 {

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

@@ -101,11 +101,13 @@ fn F(i: Test.ForwardWithDef) {}
 
 impl package Test library "[[@TEST_NAME]]";
 
-// CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+5]]:9: error: name `Forward` not found [NameNotFound]
+// CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+7]]:9: error: name `Forward` not found [NameNotFound]
 // CHECK:STDERR: fn F(i: Forward*) {}
 // CHECK:STDERR:         ^~~~~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_todo_forward.impl.carbon: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+3]]:9: error: name used before it was declared [NameUseBeforeDecl]
+// CHECK:STDERR: fn F(i: Forward*) {}
+// CHECK:STDERR:         ^~~~~~~
 fn F(i: Forward*) {}
 
 // CHECK:STDERR: fail_todo_forward.impl.carbon:[[@LINE+4]]:1: note: declared here [NameUseBeforeDeclNote]
@@ -427,7 +429,7 @@ private interface Redecl {}
 // CHECK:STDOUT:     %i.param_patt: <error> = value_param_pattern %i.patt, runtime_param0
 // CHECK:STDOUT:   } {
 // CHECK:STDOUT:     %i.param: <error> = value_param runtime_param0
-// CHECK:STDOUT:     %.loc9: type = splice_block %ptr [template = <error>] {
+// CHECK:STDOUT:     %.loc11: type = splice_block %ptr [template = <error>] {
 // CHECK:STDOUT:       %Forward.ref: <error> = name_ref Forward, <error> [template = <error>]
 // CHECK:STDOUT:       %ptr: type = ptr_type <error> [template = <error>]
 // CHECK:STDOUT:     }

toolchain/sem_ir/name_scope.cpp

@@ -64,11 +64,12 @@ auto NameScope::LookupOrAdd(NameId name_id, InstId inst_id,
   return {true, EntryId(names_.size() - 1)};
 }
 
-auto NameScope::LookupOrPoison(NameId name_id) -> std::optional<EntryId> {
+auto NameScope::LookupOrPoison(LocId loc_id, NameId name_id)
+    -> std::optional<EntryId> {
   auto insert_result = name_map_.Insert(name_id, EntryId(names_.size()));
   if (insert_result.is_inserted()) {
-    names_.push_back(
-        {.name_id = name_id, .result = ScopeLookupResult::MakePoisoned()});
+    names_.push_back({.name_id = name_id,
+                      .result = ScopeLookupResult::MakePoisoned(loc_id)});
     return std::nullopt;
   }
   return insert_result.value();

toolchain/sem_ir/name_scope.h

@@ -37,28 +37,28 @@ enum class AccessKind : int8_t {
 //   `ErrorInst::SingletonInstId`.
 class ScopeLookupResult {
  public:
-  static auto MakeFound(InstId inst_id, AccessKind access_kind)
+  static auto MakeFound(InstId target_inst_id, AccessKind access_kind)
       -> ScopeLookupResult {
-    CARBON_CHECK(inst_id.has_value());
-    return MakeWrappedLookupResult(inst_id, access_kind);
+    CARBON_CHECK(target_inst_id.has_value());
+    return MakeWrappedLookupResult(target_inst_id, access_kind);
   }
 
   static auto MakeNotFound() -> ScopeLookupResult {
     return MakeWrappedLookupResult(InstId::None, AccessKind::Public);
   }
 
-  static auto MakePoisoned() -> ScopeLookupResult {
-    return ScopeLookupResult(InstId::None, AccessKind::Public,
-                             /*is_poisoned=*/true);
+  static auto MakePoisoned(LocId poisoning_loc_id) -> ScopeLookupResult {
+    return ScopeLookupResult(poisoning_loc_id);
   }
 
   static auto MakeError() -> ScopeLookupResult {
     return MakeFound(ErrorInst::SingletonInstId, AccessKind::Public);
   }
 
-  static auto MakeWrappedLookupResult(InstId inst_id, AccessKind access_kind)
+  static auto MakeWrappedLookupResult(InstId target_inst_id,
+                                      AccessKind access_kind)
       -> ScopeLookupResult {
-    return ScopeLookupResult(inst_id, access_kind, /*is_poisoned=*/false);
+    return ScopeLookupResult(target_inst_id, access_kind);
   }
 
   // True iff CreatePoisoned() was used.
@@ -67,33 +67,53 @@ class ScopeLookupResult {
   // True when lookup was successful or resulted with an error. False for
   // poisoned or not found.
   auto is_found() const -> bool {
-    return !is_poisoned() && inst_id_.has_value();
+    return !is_poisoned() && target_inst_id_.has_value();
   }
 
   // The `InstId` of the result of the lookup. Must only be called when lookup
-  // was successful e.g. `is_found()` returns true. Always returns an existing
-  // `InstId`.
+  // was successful; in other words, when `is_found()` returns true. Always
+  // returns an existing `InstId`.
   auto target_inst_id() const -> InstId {
     CARBON_CHECK(is_found());
-    return inst_id_;
+    return target_inst_id_;
+  }
+
+  // The `LocId` where the name poisoning was triggered. Must only be called
+  // when lookup returned a poisoned name; in other words, when `is_poisoned()`
+  // returns true. Always returns an existing `InstId`.
+  auto poisoning_loc_id() const -> LocId {
+    CARBON_CHECK(is_poisoned());
+    return poisoning_loc_id_;
   }
 
   auto access_kind() const -> AccessKind { return access_kind_; }
 
   // Equality means either:
   // - Both are not poisoned and have the same `InstId` and `AccessKind`.
-  // - Both are poisoned.
-  friend auto operator==(const ScopeLookupResult&, const ScopeLookupResult&)
-      -> bool = default;
+  // - Both are poisoned and have the same `LocId`.
+  friend auto operator==(const ScopeLookupResult& lhs,
+                         const ScopeLookupResult& rhs) -> bool {
+    return lhs.is_poisoned_ == rhs.is_poisoned_ &&
+           lhs.access_kind_ == rhs.access_kind_ &&
+           (lhs.is_poisoned_ ? lhs.poisoning_loc_id_ == rhs.poisoning_loc_id_
+                             : lhs.target_inst_id_ == rhs.target_inst_id_);
+  }
 
  private:
-  explicit ScopeLookupResult(InstId inst_id, AccessKind access_kind,
-                             bool is_poisoned)
-      : inst_id_(inst_id),
+  explicit ScopeLookupResult(InstId target_inst_id, AccessKind access_kind)
+      : target_inst_id_(target_inst_id),
         access_kind_(access_kind),
-        is_poisoned_(is_poisoned) {}
+        is_poisoned_(false) {}
 
-  InstId inst_id_;
+  explicit ScopeLookupResult(LocId loc_id)
+      : poisoning_loc_id_(loc_id),
+        access_kind_(AccessKind::Public),
+        is_poisoned_(true) {}
+
+  union {
+    InstId target_inst_id_;
+    LocId poisoning_loc_id_;
+  };
   AccessKind access_kind_;
   bool is_poisoned_;
 };
@@ -159,7 +179,7 @@ class NameScope : public Printable<NameScope> {
   // Searches for the given name. If found, including if a poisoned entry is
   // found, returns the corresponding EntryId. Otherwise, returns nullopt and
   // poisons the name so it can't be declared later.
-  auto LookupOrPoison(NameId name_id) -> std::optional<EntryId>;
+  auto LookupOrPoison(LocId loc_id, NameId name_id) -> std::optional<EntryId>;
 
   auto extended_scopes() const -> llvm::ArrayRef<InstId> {
     return extended_scopes_;

toolchain/sem_ir/name_scope_test.cpp

@@ -21,7 +21,9 @@ TEST(ScopeLookupResult, MakeWrappedLookupResultUsingExistingInstId) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_TRUE(result.is_found());
   EXPECT_EQ(result.target_inst_id(), inst_id);
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Protected);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakeWrappedLookupResultUsingNoneInstId) {
@@ -31,7 +33,9 @@ TEST(ScopeLookupResult, MakeWrappedLookupResultUsingNoneInstId) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_FALSE(result.is_found());
   EXPECT_DEATH(result.target_inst_id(), "is_found");
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Protected);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakeWrappedLookupResultUsingErrorInst) {
@@ -41,7 +45,9 @@ TEST(ScopeLookupResult, MakeWrappedLookupResultUsingErrorInst) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_TRUE(result.is_found());
   EXPECT_EQ(result.target_inst_id(), ErrorInst::SingletonInstId);
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Private);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakeFoundExisting) {
@@ -51,7 +57,9 @@ TEST(ScopeLookupResult, MakeFoundExisting) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_TRUE(result.is_found());
   EXPECT_EQ(result.target_inst_id(), inst_id);
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Protected);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakeFoundNone) {
@@ -66,16 +74,21 @@ TEST(ScopeLookupResult, MakeNotFound) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_FALSE(result.is_found());
   EXPECT_DEATH(result.target_inst_id(), "is_found");
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Public);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakePoisoned) {
-  auto result = ScopeLookupResult::MakePoisoned();
+  LocId loc_id(1);
+  auto result = ScopeLookupResult::MakePoisoned(loc_id);
 
   EXPECT_TRUE(result.is_poisoned());
   EXPECT_FALSE(result.is_found());
   EXPECT_DEATH(result.target_inst_id(), "is_found");
+  EXPECT_EQ(result.poisoning_loc_id(), loc_id);
   EXPECT_EQ(result.access_kind(), AccessKind::Public);
+  EXPECT_TRUE(result == result);
 }
 
 TEST(ScopeLookupResult, MakeError) {
@@ -84,7 +97,45 @@ TEST(ScopeLookupResult, MakeError) {
   EXPECT_FALSE(result.is_poisoned());
   EXPECT_TRUE(result.is_found());
   EXPECT_EQ(result.target_inst_id(), ErrorInst::SingletonInstId);
+  EXPECT_DEATH(result.poisoning_loc_id(), "is_poisoned");
   EXPECT_EQ(result.access_kind(), AccessKind::Public);
+  EXPECT_TRUE(result == result);
+}
+
+TEST(ScopeLookupResult, EqualityPoisonedDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakePoisoned(LocId(1)) ==
+               ScopeLookupResult::MakeNotFound());
+  EXPECT_FALSE(ScopeLookupResult::MakeNotFound() ==
+               ScopeLookupResult::MakePoisoned(LocId(1)));
+}
+
+TEST(ScopeLookupResult, EqualityPoisonedLocIdDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakePoisoned(LocId(1)) ==
+               ScopeLookupResult::MakePoisoned(LocId(2)));
+}
+
+TEST(ScopeLookupResult, EqualityFoundDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakeFound(InstId(1), AccessKind::Public) ==
+               ScopeLookupResult::MakeNotFound());
+  EXPECT_FALSE(ScopeLookupResult::MakeNotFound() ==
+               ScopeLookupResult::MakeFound(InstId(1), AccessKind::Public));
+}
+
+TEST(ScopeLookupResult, EqualityFoundTargetInstIdDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakeFound(InstId(1), AccessKind::Public) ==
+               ScopeLookupResult::MakeFound(InstId(2), AccessKind::Public));
+}
+
+TEST(ScopeLookupResult, EqualityFoundAccessKindDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakeFound(InstId(1), AccessKind::Public) ==
+               ScopeLookupResult::MakeFound(InstId(1), AccessKind::Protected));
+}
+
+TEST(ScopeLookupResult, EqualityErrorDifferent) {
+  EXPECT_FALSE(ScopeLookupResult::MakeNotFound() ==
+               ScopeLookupResult::MakeError());
+  EXPECT_FALSE(ScopeLookupResult::MakeError() ==
+               ScopeLookupResult::MakeNotFound());
 }
 
 TEST(NameScope, Empty) {
@@ -171,22 +222,23 @@ TEST(NameScope, LookupOrPoison) {
                                  InstId(++id), AccessKind::Private)};
   name_scope.AddRequired(entry3);
 
-  auto lookup = name_scope.LookupOrPoison(entry1.name_id);
+  LocId poisoning_loc_id(++id);
+  auto lookup = name_scope.LookupOrPoison(poisoning_loc_id, entry1.name_id);
   ASSERT_NE(lookup, std::nullopt);
   EXPECT_EQ(static_cast<NameScope&>(name_scope).GetEntry(*lookup), entry1);
   EXPECT_EQ(static_cast<const NameScope&>(name_scope).GetEntry(*lookup),
             entry1);
 
-  lookup = name_scope.LookupOrPoison(entry2.name_id);
+  lookup = name_scope.LookupOrPoison(poisoning_loc_id, entry2.name_id);
   ASSERT_NE(lookup, std::nullopt);
   EXPECT_EQ(name_scope.GetEntry(*lookup), entry2);
 
-  lookup = name_scope.LookupOrPoison(entry3.name_id);
+  lookup = name_scope.LookupOrPoison(poisoning_loc_id, entry3.name_id);
   ASSERT_NE(lookup, std::nullopt);
   EXPECT_EQ(name_scope.GetEntry(*lookup), entry3);
 
   NameId unknown_name_id(++id);
-  lookup = name_scope.LookupOrPoison(unknown_name_id);
+  lookup = name_scope.LookupOrPoison(poisoning_loc_id, unknown_name_id);
   EXPECT_EQ(lookup, std::nullopt);
 }
 
@@ -265,27 +317,33 @@ TEST(NameScope, Poison) {
   NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id);
 
   NameId poison1(++id);
-  EXPECT_EQ(name_scope.LookupOrPoison(poison1), std::nullopt);
+  LocId poisoning_loc1(++id);
+  EXPECT_EQ(name_scope.LookupOrPoison(poisoning_loc1, poison1), std::nullopt);
   EXPECT_THAT(
       name_scope.entries(),
       ElementsAre(NameScope::Entry(
-          {.name_id = poison1, .result = ScopeLookupResult::MakePoisoned()})));
+          {.name_id = poison1,
+           .result = ScopeLookupResult::MakePoisoned(poisoning_loc1)})));
 
   NameId poison2(++id);
-  EXPECT_EQ(name_scope.LookupOrPoison(poison2), std::nullopt);
+  LocId poisoning_loc2(++id);
+  EXPECT_EQ(name_scope.LookupOrPoison(poisoning_loc2, poison2), std::nullopt);
   EXPECT_THAT(
       name_scope.entries(),
       ElementsAre(
-          NameScope::Entry({.name_id = poison1,
-                            .result = ScopeLookupResult::MakePoisoned()}),
-          NameScope::Entry({.name_id = poison2,
-                            .result = ScopeLookupResult::MakePoisoned()})));
+          NameScope::Entry(
+              {.name_id = poison1,
+               .result = ScopeLookupResult::MakePoisoned(poisoning_loc1)}),
+          NameScope::Entry(
+              {.name_id = poison2,
+               .result = ScopeLookupResult::MakePoisoned(poisoning_loc2)})));
 
   auto lookup = name_scope.Lookup(poison1);
   ASSERT_NE(lookup, std::nullopt);
   EXPECT_THAT(name_scope.GetEntry(*lookup),
-              NameScope::Entry({.name_id = poison1,
-                                .result = ScopeLookupResult::MakePoisoned()}));
+              NameScope::Entry(
+                  {.name_id = poison1,
+                   .result = ScopeLookupResult::MakePoisoned(poisoning_loc1)}));
 }
 
 TEST(NameScope, AddRequiredAfterPoison) {
@@ -298,19 +356,21 @@ TEST(NameScope, AddRequiredAfterPoison) {
 
   NameId name_id(++id);
   InstId inst_id(++id);
+  LocId poisoning_loc_id(++id);
 
-  EXPECT_EQ(name_scope.LookupOrPoison(name_id), std::nullopt);
+  EXPECT_EQ(name_scope.LookupOrPoison(poisoning_loc_id, name_id), std::nullopt);
   EXPECT_THAT(
       name_scope.entries(),
       ElementsAre(NameScope::Entry(
-          {.name_id = name_id, .result = ScopeLookupResult::MakePoisoned()})));
+          {.name_id = name_id,
+           .result = ScopeLookupResult::MakePoisoned(poisoning_loc_id)})));
 
   NameScope::Entry entry = {
       .name_id = name_id,
       .result = ScopeLookupResult::MakeFound(inst_id, AccessKind::Private)};
   name_scope.AddRequired(entry);
 
-  auto lookup = name_scope.LookupOrPoison(name_id);
+  auto lookup = name_scope.LookupOrPoison(poisoning_loc_id, name_id);
   ASSERT_NE(lookup, std::nullopt);
   EXPECT_EQ(name_scope.GetEntry(*lookup),
             NameScope::Entry({.name_id = name_id,