From c67920e63162f9890d62df8ae15fe9ac86e8f18b Mon Sep 17 00:00:00 2001 From: Boaz Brickner Date: Mon, 3 Feb 2025 21:01:09 +0100 Subject: [PATCH] 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 | 21 +-- toolchain/check/context.h | 11 +- toolchain/check/decl_name_stack.cpp | 6 +- toolchain/check/decl_name_stack.h | 4 + toolchain/check/handle_class.cpp | 3 +- toolchain/check/handle_function.cpp | 3 +- toolchain/check/handle_interface.cpp | 3 +- toolchain/check/handle_namespace.cpp | 3 +- toolchain/check/impl.cpp | 3 +- .../no_prelude/name_poisoning.carbon | 138 +++++++++++------- .../interface/no_prelude/import_access.carbon | 8 +- toolchain/sem_ir/name_scope.cpp | 7 +- toolchain/sem_ir/name_scope.h | 62 +++++--- toolchain/sem_ir/name_scope_test.cpp | 94 +++++++++--- 14 files changed, 246 insertions(+), 120 deletions(-) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 9417e4665ce6..921c6b2ab71d 100644 --- a/toolchain/check/context.cpp +++ b/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); } diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 6679c237d1ec..f717ca8a051f 100644 --- a/toolchain/check/context.h +++ b/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; diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 6f26b8fa9bfc..8f47010d1d0e 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/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. diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index bf1493d20000..7a3f93d1d210 100644 --- a/toolchain/check/decl_name_stack.h +++ b/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 diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index afb5acd4ccf7..c59b35aef9ce 100644 --- a/toolchain/check/handle_class.cpp +++ b/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; } diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 2c170c9454d8..566427356b08 100644 --- a/toolchain/check/handle_function.cpp +++ b/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); diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 4d4796e099b5..6698a6498896 100644 --- a/toolchain/check/handle_interface.cpp +++ b/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 = diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index ed50404af221..87710efe6032 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/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 = diff --git a/toolchain/check/impl.cpp b/toolchain/check/impl.cpp index 2155d8d54db7..23e88d9dad61 100644 --- a/toolchain/check/impl.cpp +++ b/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( diff --git a/toolchain/check/testdata/function/declaration/no_prelude/name_poisoning.carbon b/toolchain/check/testdata/function/declaration/no_prelude/name_poisoning.carbon index 73c0e43fc238..4e5b0c02ed16 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/name_poisoning.carbon +++ b/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 [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 [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 [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 [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 // CHECK:STDOUT: %complete_type: = 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 [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 [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 [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 [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 [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 [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: = name_ref C, [template = ] // CHECK:STDOUT: %x: = 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 // 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 { diff --git a/toolchain/check/testdata/interface/no_prelude/import_access.carbon b/toolchain/check/testdata/interface/no_prelude/import_access.carbon index 073c936fc45c..5c6ed4bcadfc 100644 --- a/toolchain/check/testdata/interface/no_prelude/import_access.carbon +++ b/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: = value_param_pattern %i.patt, runtime_param0 // CHECK:STDOUT: } { // CHECK:STDOUT: %i.param: = value_param runtime_param0 -// CHECK:STDOUT: %.loc9: type = splice_block %ptr [template = ] { +// CHECK:STDOUT: %.loc11: type = splice_block %ptr [template = ] { // CHECK:STDOUT: %Forward.ref: = name_ref Forward, [template = ] // CHECK:STDOUT: %ptr: type = ptr_type [template = ] // CHECK:STDOUT: } diff --git a/toolchain/sem_ir/name_scope.cpp b/toolchain/sem_ir/name_scope.cpp index 79aceda1212c..32ca2ba945b5 100644 --- a/toolchain/sem_ir/name_scope.cpp +++ b/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 { +auto NameScope::LookupOrPoison(LocId loc_id, NameId name_id) + -> std::optional { 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(); diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index 391778d77ac8..8c2d977f3dc8 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/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 { // 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; + auto LookupOrPoison(LocId loc_id, NameId name_id) -> std::optional; auto extended_scopes() const -> llvm::ArrayRef { return extended_scopes_; diff --git a/toolchain/sem_ir/name_scope_test.cpp b/toolchain/sem_ir/name_scope_test.cpp index 820b7690e4e8..16c358ea90f3 100644 --- a/toolchain/sem_ir/name_scope_test.cpp +++ b/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(name_scope).GetEntry(*lookup), entry1); EXPECT_EQ(static_cast(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,