diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 366c4f9aa29b..dd06c1ca603e 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -317,7 +317,7 @@ auto Context::AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id, auto Context::LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id, SemIR::NameScopeId scope_id, ScopeIndex scope_index) - -> std::pair { + -> SemIR::ScopeLookupResult { if (!scope_id.has_value()) { // Look for a name in the specified scope or a scope nested within it only. // There are two cases where the name would be in an outer scope: @@ -353,8 +353,9 @@ auto Context::LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id, // // In this case, the class C is not a redeclaration of its parameter, but // we find the parameter in order to diagnose a redeclaration error. - return {scope_stack().LookupInLexicalScopesWithin(name_id, scope_index), - false}; + return SemIR::ScopeLookupResult::MakeWrappedLookupResult( + scope_stack().LookupInLexicalScopesWithin(name_id, scope_index), + SemIR::AccessKind::Public); } else { // We do not look into `extend`ed scopes here. A qualified name in a // declaration must specify the exact scope in which the name was originally @@ -365,10 +366,9 @@ auto Context::LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id, // // // Error, no `F` in `B`. // fn B.F() {} - auto result = LookupNameInExactScope(loc_id, name_id, scope_id, - name_scopes().Get(scope_id), - /*is_being_declared=*/true); - return {result.inst_id, result.is_poisoned}; + return LookupNameInExactScope(loc_id, name_id, scope_id, + name_scopes().Get(scope_id), + /*is_being_declared=*/true); } } @@ -390,7 +390,7 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id, LookupScope{.name_scope_id = lookup_scope_id, .specific_id = specific_id}, /*required=*/false); - non_lexical_result.inst_id.has_value()) { + non_lexical_result.scope_result.is_found()) { return non_lexical_result; } } @@ -400,14 +400,16 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id, "`{0}` used before initialization", SemIR::NameId); emitter_->Emit(node_id, UsedBeforeInitialization, name_id); return {.specific_id = SemIR::SpecificId::None, - .inst_id = SemIR::ErrorInst::SingletonInstId}; + .scope_result = SemIR::ScopeLookupResult::MakeError()}; } if (lexical_result.has_value()) { // A lexical scope never needs an associated specific. If there's a // lexically enclosing generic, then it also encloses the point of use of // the name. - return {.specific_id = SemIR::SpecificId::None, .inst_id = lexical_result}; + return {.specific_id = SemIR::SpecificId::None, + .scope_result = SemIR::ScopeLookupResult::MakeFound( + lexical_result, SemIR::AccessKind::Public)}; } // We didn't find anything at all. @@ -416,32 +418,32 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id, } return {.specific_id = SemIR::SpecificId::None, - .inst_id = SemIR::ErrorInst::SingletonInstId}; + .scope_result = SemIR::ScopeLookupResult::MakeError()}; } auto Context::LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id, SemIR::NameScopeId scope_id, SemIR::NameScope& scope, bool is_being_declared) - -> LookupNameInExactScopeResult { + -> SemIR::ScopeLookupResult { if (auto entry_id = is_being_declared ? scope.Lookup(name_id) : scope.LookupOrPoison(name_id)) { - auto entry = scope.GetEntry(*entry_id); - if (!entry.is_poisoned) { - LoadImportRef(*this, entry.inst_id); - } else if (is_being_declared) { - entry.inst_id = SemIR::InstId::None; + auto lookup_result = scope.GetEntry(*entry_id).result; + if (!lookup_result.is_poisoned()) { + LoadImportRef(*this, lookup_result.target_inst_id()); + return lookup_result; } - return {entry.inst_id, entry.access_kind, entry.is_poisoned}; + return SemIR::ScopeLookupResult::MakePoisoned(); } if (!scope.import_ir_scopes().empty()) { // TODO: Enforce other access modifiers for imports. - return {ImportNameFromOtherPackage(*this, loc, scope_id, - scope.import_ir_scopes(), name_id), - SemIR::AccessKind::Public}; + return SemIR::ScopeLookupResult::MakeWrappedLookupResult( + ImportNameFromOtherPackage(*this, loc, scope_id, + scope.import_ir_scopes(), name_id), + SemIR::AccessKind::Public); } - return {SemIR::InstId::None, SemIR::AccessKind::Public}; + return SemIR::ScopeLookupResult::MakeNotFound(); } // Prints diagnostics on invalid qualified name access. @@ -583,8 +585,9 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, // TODO: Support reporting of multiple prohibited access. llvm::SmallVector prohibited_accesses; - LookupResult result = {.specific_id = SemIR::SpecificId::None, - .inst_id = SemIR::InstId::None}; + LookupResult result = { + .specific_id = SemIR::SpecificId::None, + .scope_result = SemIR::ScopeLookupResult::MakeNotFound()}; bool has_error = false; bool is_parent_access = false; @@ -598,8 +601,9 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, auto& name_scope = name_scopes().Get(scope_id); has_error |= name_scope.has_error(); - auto [scope_result_id, access_kind, is_poisoned] = + const SemIR::ScopeLookupResult scope_result = LookupNameInExactScope(loc_id, name_id, scope_id, name_scope); + SemIR::AccessKind access_kind = scope_result.access_kind(); auto is_access_prohibited = IsAccessProhibited(access_info, access_kind, is_parent_access); @@ -608,13 +612,13 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, // multiple prohibited accesses if we can't find a suitable lookup. if (is_access_prohibited) { prohibited_accesses.push_back({ - .scope_result_id = scope_result_id, + .scope_result_id = scope_result.target_inst_id(), .access_kind = access_kind, .is_parent_access = is_parent_access, }); } - if (!scope_result_id.has_value() || is_access_prohibited) { + if (!scope_result.is_found() || is_access_prohibited) { // If nothing is found in this scope or if we encountered an invalid // access, look in its extended scopes. const auto& extended = name_scope.extended_scopes(); @@ -643,7 +647,7 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, } // If this is our second lookup result, diagnose an ambiguity. - if (result.inst_id.has_value()) { + if (result.scope_result.is_found()) { CARBON_DIAGNOSTIC( NameAmbiguousDueToExtend, Error, "ambiguous use of name `{0}` found in multiple extended scopes", @@ -651,15 +655,14 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, emitter_->Emit(loc_id, NameAmbiguousDueToExtend, name_id); // TODO: Add notes pointing to the scopes. return {.specific_id = SemIR::SpecificId::None, - .inst_id = SemIR::ErrorInst::SingletonInstId}; + .scope_result = SemIR::ScopeLookupResult::MakeError()}; } - result.inst_id = scope_result_id; + result.scope_result = scope_result; result.specific_id = specific_id; - result.is_poisoned = is_poisoned; } - if (required && !result.inst_id.has_value()) { + if (required && !result.scope_result.is_found()) { if (!has_error) { if (prohibited_accesses.empty()) { DiagnoseMemberNameNotFound(loc_id, name_id, lookup_scopes); @@ -677,9 +680,9 @@ auto Context::LookupQualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id, } } + CARBON_CHECK(!result.scope_result.is_poisoned()); return {.specific_id = SemIR::SpecificId::None, - .inst_id = SemIR::ErrorInst::SingletonInstId, - .is_poisoned = result.is_poisoned}; + .scope_result = SemIR::ScopeLookupResult::MakeError()}; } return result; @@ -699,13 +702,13 @@ static auto GetCorePackage(Context& context, SemIRLoc loc, llvm::StringRef name) auto core_name_id = SemIR::NameId::ForIdentifier(core_ident_id); // Look up `package.Core`. - auto [core_inst_id, _, is_poisoned] = context.LookupNameInExactScope( + auto core_scope_result = context.LookupNameInExactScope( loc, core_name_id, SemIR::NameScopeId::Package, context.name_scopes().Get(SemIR::NameScopeId::Package)); - if (core_inst_id.has_value()) { + if (core_scope_result.is_found()) { // We expect it to be a namespace. - if (auto namespace_inst = - context.insts().TryGetAs(core_inst_id)) { + if (auto namespace_inst = context.insts().TryGetAs( + core_scope_result.target_inst_id())) { // TODO: Decide whether to allow the case where `Core` is not a package. return namespace_inst->name_scope_id; } @@ -727,9 +730,9 @@ auto Context::LookupNameInCore(SemIRLoc loc, llvm::StringRef name) } auto name_id = SemIR::NameId::ForIdentifier(identifiers().Add(name)); - auto [inst_id, _, is_poisoned] = LookupNameInExactScope( + auto scope_result = LookupNameInExactScope( loc, name_id, core_package_id, name_scopes().Get(core_package_id)); - if (!inst_id.has_value()) { + if (!scope_result.is_found()) { CARBON_DIAGNOSTIC( CoreNameNotFound, Error, "name `Core.{0}` implicitly referenced here, but not found", @@ -739,7 +742,7 @@ auto Context::LookupNameInCore(SemIRLoc loc, llvm::StringRef name) } // Look through import_refs and aliases. - return constant_values().GetConstantInstId(inst_id); + return constant_values().GetConstantInstId(scope_result.target_inst_id()); } template diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 758218a6ecc8..f98d48a31aa3 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -45,11 +45,9 @@ struct LookupResult { // The specific in which the lookup result was found. `None` if the result // was not found in a specific. SemIR::SpecificId specific_id; - // The declaration that was found by name lookup. `None` for poisoned items. - // TODO: Make this point to the poisoning declaration. - SemIR::InstId inst_id; - // Whether the lookup found a poisoned name. - bool is_poisoned = false; + + // The result from the lookup in the scope. + SemIR::ScopeLookupResult scope_result; }; // Information about an access. @@ -73,17 +71,6 @@ class Context { using BuildDiagnosticFn = llvm::function_refContext::DiagnosticBuilder>; - struct LookupNameInExactScopeResult { - // The matching entity if found, or `None` if poisoned or not found. - SemIR::InstId inst_id; - - // The access level required to use inst_id when it's not `None`. - SemIR::AccessKind access_kind; - - // Whether a poisoned entry was found. - bool is_poisoned = false; - }; - // Stores references for work. explicit Context(DiagnosticEmitter* emitter, llvm::function_ref @@ -233,7 +220,7 @@ class Context { // TODO: For poisoned names, return the poisoning `InstId`. auto LookupNameInDecl(SemIR::LocId loc_id, SemIR::NameId name_id, SemIR::NameScopeId scope_id, ScopeIndex scope_index) - -> std::pair; + -> SemIR::ScopeLookupResult; // Performs an unqualified name lookup, returning the referenced `InstId`. auto LookupUnqualifiedName(Parse::NodeId node_id, SemIR::NameId name_id, @@ -252,7 +239,7 @@ class Context { SemIR::NameScopeId scope_id, SemIR::NameScope& scope, bool is_being_declared = false) - -> LookupNameInExactScopeResult; + -> SemIR::ScopeLookupResult; // Appends the lookup scopes corresponding to `base_const_id` to `*scopes`. // Returns `false` if not a scope. On invalid scopes, prints a diagnostic, but diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 77061847e6f3..50c64992bfe3 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -160,8 +160,8 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id, } name_scope.AddRequired({.name_id = name_context.unresolved_name_id, - .inst_id = target_id, - .access_kind = access_kind}); + .result = SemIR::ScopeLookupResult::MakeFound( + target_id, access_kind)}); } break; @@ -262,20 +262,20 @@ auto DeclNameStack::ApplyAndLookupName(NameContext& name_context, } // For identifier nodes, we need to perform a lookup on the identifier. - auto [resolved_inst_id, is_poisoned] = context_->LookupNameInDecl( + auto lookup_result = context_->LookupNameInDecl( name_context.loc_id, name_id, name_context.parent_scope_id, name_context.initial_scope_index); - if (is_poisoned) { + if (lookup_result.is_poisoned()) { name_context.unresolved_name_id = name_id; name_context.state = NameContext::State::Poisoned; - } else if (!resolved_inst_id.has_value()) { + } else if (!lookup_result.is_found()) { // Invalid indicates an unresolved name. Store it and return. name_context.unresolved_name_id = name_id; name_context.state = NameContext::State::Unresolved; } else { // Store the resolved instruction and continue for the target scope // update. - name_context.resolved_inst_id = resolved_inst_id; + name_context.resolved_inst_id = lookup_result.target_inst_id(); name_context.state = NameContext::State::Resolved; } } diff --git a/toolchain/check/handle_export.cpp b/toolchain/check/handle_export.cpp index c1637198a4e6..c12accc21941 100644 --- a/toolchain/check/handle_export.cpp +++ b/toolchain/check/handle_export.cpp @@ -78,10 +78,11 @@ auto HandleParseNode(Context& context, Parse::ExportDeclId node_id) -> bool { // diagnostic and so that cross-package imports can find it easily. auto entity_name = context.entity_names().Get(import_ref->entity_name_id); auto& parent_scope = context.name_scopes().Get(entity_name.parent_scope_id); - auto& scope_inst_id = - parent_scope.GetEntry(*parent_scope.Lookup(entity_name.name_id)).inst_id; - CARBON_CHECK(scope_inst_id == inst_id); - scope_inst_id = export_id; + auto& scope_result = + parent_scope.GetEntry(*parent_scope.Lookup(entity_name.name_id)).result; + CARBON_CHECK(scope_result.target_inst_id() == inst_id); + scope_result = SemIR::ScopeLookupResult::MakeFound( + export_id, scope_result.access_kind()); return true; } diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp index 7417e7e769ef..7b524bbe38e4 100644 --- a/toolchain/check/handle_name.cpp +++ b/toolchain/check/handle_name.cpp @@ -103,7 +103,8 @@ static auto GetIdentifierAsName(Context& context, Parse::NodeId node_id) static auto HandleNameAsExpr(Context& context, Parse::NodeId node_id, SemIR::NameId name_id) -> SemIR::InstId { auto result = context.LookupUnqualifiedName(node_id, name_id); - auto value = context.insts().Get(result.inst_id); + SemIR::InstId inst_id = result.scope_result.target_inst_id(); + auto value = context.insts().Get(inst_id); auto type_id = SemIR::GetTypeInSpecific(context.sem_ir(), result.specific_id, value.type_id()); CARBON_CHECK(type_id.has_value(), "Missing type for {0}", value); @@ -111,16 +112,15 @@ static auto HandleNameAsExpr(Context& context, Parse::NodeId node_id, // If the named entity has a constant value that depends on its specific, // store the specific too. if (result.specific_id.has_value() && - context.constant_values().Get(result.inst_id).is_symbolic()) { - result.inst_id = context.AddInst( + context.constant_values().Get(inst_id).is_symbolic()) { + inst_id = context.AddInst( node_id, {.type_id = type_id, - .inst_id = result.inst_id, + .inst_id = inst_id, .specific_id = result.specific_id}); } return context.AddInst( - node_id, - {.type_id = type_id, .name_id = name_id, .value_id = result.inst_id}); + node_id, {.type_id = type_id, .name_id = name_id, .value_id = inst_id}); } static auto HandleIdentifierName(Context& context, diff --git a/toolchain/check/impl.cpp b/toolchain/check/impl.cpp index abb8dba56b53..bb28ad0c72f4 100644 --- a/toolchain/check/impl.cpp +++ b/toolchain/check/impl.cpp @@ -416,12 +416,13 @@ auto FinishImplWitness(Context& context, SemIR::Impl& impl) -> void { CARBON_FATAL("Unexpected type: {0}", type_inst); } auto& fn = context.functions().Get(fn_type->function_id); - auto [impl_decl_id, _, is_poisoned] = context.LookupNameInExactScope( + auto lookup_result = context.LookupNameInExactScope( decl_id, fn.name_id, impl.scope_id, impl_scope); - if (impl_decl_id.has_value()) { - used_decl_ids.push_back(impl_decl_id); + if (lookup_result.is_found()) { + used_decl_ids.push_back(lookup_result.target_inst_id()); witness_block[index] = CheckAssociatedFunctionImplementation( - context, *fn_type, impl_decl_id, self_type_id, impl.witness_id); + context, *fn_type, lookup_result.target_inst_id(), self_type_id, + impl.witness_id); } else { CARBON_DIAGNOSTIC( ImplMissingFunction, Error, diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index be8259961e5d..b6c509e0daaa 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -110,8 +110,8 @@ static auto AddNamespace(Context& context, SemIR::TypeId namespace_type_id, SemIR::InstId::None, SemIR::AccessKind::Public); if (!inserted) { const auto& prev_entry = parent_scope->GetEntry(entry_id); - if (!prev_entry.is_poisoned) { - auto prev_inst_id = prev_entry.inst_id; + if (!prev_entry.result.is_poisoned()) { + auto prev_inst_id = prev_entry.result.target_inst_id(); if (auto namespace_inst = context.insts().TryGetAs(prev_inst_id)) { if (diagnose_duplicate_namespace) { @@ -152,14 +152,12 @@ static auto AddNamespace(Context& context, SemIR::TypeId namespace_type_id, // supersede the name conflict in order to avoid repeat diagnostics. Names are // poisoned optimistically by name lookup before checking for imports, so we // may be overwriting a poisoned entry here. - auto& entry = parent_scope->GetEntry(entry_id); - if (entry.is_poisoned) { - entry.is_poisoned = false; - } else if (!inserted) { - context.DiagnoseDuplicateName(namespace_id, entry.inst_id); - entry.access_kind = SemIR::AccessKind::Public; + auto& result = parent_scope->GetEntry(entry_id).result; + if (!result.is_poisoned() && !inserted) { + context.DiagnoseDuplicateName(namespace_id, result.target_inst_id()); } - entry.inst_id = namespace_id; + result = SemIR::ScopeLookupResult::MakeFound(namespace_id, + SemIR::AccessKind::Public); return {namespace_inst.name_scope_id, namespace_id, false}; } @@ -284,12 +282,14 @@ static auto AddImportRefOrMerge(Context& context, SemIR::ImportIRId ir_id, {.name_id = name_id, .parent_scope_id = parent_scope_id, .bind_index = SemIR::CompileTimeBindIndex::None}); - entry.inst_id = AddImportRef( - context, {.ir_id = ir_id, .inst_id = import_inst_id}, entity_name_id); + entry.result = SemIR::ScopeLookupResult::MakeFound( + AddImportRef(context, {.ir_id = ir_id, .inst_id = import_inst_id}, + entity_name_id), + SemIR::AccessKind::Public); return; } - auto inst_id = entry.inst_id; + auto inst_id = entry.result.target_inst_id(); auto prev_ir_inst = GetCanonicalImportIRInst(context, inst_id); VerifySameCanonicalImportIRInst(context, inst_id, prev_ir_inst, ir_id, &import_sem_ir, import_inst_id); @@ -324,8 +324,8 @@ static auto AddScopedImportRef(Context& context, .bind_index = SemIR::CompileTimeBindIndex::None}); auto import_ref_id = AddImportRef(context, import_inst, impl_entity_name_id); parent_scope.AddRequired({.name_id = name_id, - .inst_id = import_ref_id, - .access_kind = access_kind}); + .result = SemIR::ScopeLookupResult::MakeFound( + import_ref_id, access_kind)}); return import_ref_id; } @@ -340,13 +340,13 @@ static auto ImportScopeFromApiFile(Context& context, auto& impl_scope = context.name_scopes().Get(impl_scope_id); for (const auto& api_entry : api_scope.entries()) { - if (api_entry.is_poisoned) { + if (api_entry.result.is_poisoned()) { continue; } auto impl_name_id = CopyNameFromImportIR(context, api_sem_ir, api_entry.name_id); - if (auto ns = - api_sem_ir.insts().TryGetAs(api_entry.inst_id)) { + if (auto ns = api_sem_ir.insts().TryGetAs( + api_entry.result.target_inst_id())) { // Ignore cross-package imports. These will be handled through // ImportLibrariesFromOtherPackage. if (api_scope_id == SemIR::NameScopeId::Package) { @@ -358,7 +358,7 @@ static auto ImportScopeFromApiFile(Context& context, // Namespaces will be recursed into. Name scope creation is delayed in // order to avoid invalidating api_scope/impl_scope. - todo_scopes.push_back({.api_inst_id = api_entry.inst_id, + todo_scopes.push_back({.api_inst_id = api_entry.result.target_inst_id(), .api_scope_id = ns->name_scope_id, .impl_name_id = impl_name_id, .impl_parent_scope_id = impl_scope_id}); @@ -366,8 +366,8 @@ static auto ImportScopeFromApiFile(Context& context, // Add an ImportRef for other instructions. AddScopedImportRef(context, impl_scope_id, impl_scope, impl_name_id, {.ir_id = SemIR::ImportIRId::ApiForImpl, - .inst_id = api_entry.inst_id}, - api_entry.access_kind); + .inst_id = api_entry.result.target_inst_id()}, + api_entry.result.access_kind()); } } } @@ -502,7 +502,7 @@ static auto LookupNameInImport(const SemIR::File& import_ir, const auto& import_scope_entry = import_scope.GetEntry(*import_scope_entry_id); - if (import_scope_entry.access_kind != SemIR::AccessKind::Public) { + if (import_scope_entry.result.access_kind() != SemIR::AccessKind::Public) { // Ignore cross-package non-public names. return nullptr; } @@ -568,8 +568,9 @@ auto ImportNameFromOtherPackage( if (!import_scope_entry) { continue; } - auto import_inst = - import_ir.sem_ir->insts().Get(import_scope_entry->inst_id); + SemIR::InstId import_scope_inst_id = + import_scope_entry->result.target_inst_id(); + auto import_inst = import_ir.sem_ir->insts().Get(import_scope_inst_id); if (import_inst.Is()) { // This entity was added to name lookup by using an import, and is not // exported. @@ -582,12 +583,12 @@ auto ImportNameFromOtherPackage( // of as an ImportRef. if (auto import_ns = import_inst.TryAs()) { result_id = AddNamespaceFromOtherPackage(context, import_ir_id, - import_scope_entry->inst_id, + import_scope_inst_id, *import_ns, scope_id, name_id); } else { result_id = AddScopedImportRef( context, scope_id, context.name_scopes().Get(scope_id), name_id, - {.ir_id = import_ir_id, .inst_id = import_scope_entry->inst_id}, + {.ir_id = import_ir_id, .inst_id = import_scope_inst_id}, SemIR::AccessKind::Public); LoadImportRef(context, result_id); } @@ -610,7 +611,7 @@ auto ImportNameFromOtherPackage( } VerifySameCanonicalImportIRInst(context, result_id, *canonical_result_inst, import_ir_id, import_ir.sem_ir, - import_scope_entry->inst_id); + import_scope_inst_id); } return result_id; diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index a35b30d9191c..6281030a12de 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -1211,13 +1211,14 @@ static auto AddNameScopeImportRefs(ImportContext& context, const SemIR::NameScope& import_scope, SemIR::NameScope& new_scope) -> void { for (auto entry : import_scope.entries()) { - if (entry.is_poisoned) { + SemIR::ScopeLookupResult result = entry.result; + if (result.is_poisoned()) { continue; } - auto ref_id = AddImportRef(context, entry.inst_id); + auto ref_id = AddImportRef(context, result.target_inst_id()); new_scope.AddRequired({.name_id = GetLocalNameId(context, entry.name_id), - .inst_id = ref_id, - .access_kind = entry.access_kind}); + .result = SemIR::ScopeLookupResult::MakeFound( + ref_id, result.access_kind())}); } for (auto scope_inst_id : import_scope.extended_scopes()) { new_scope.AddExtendedScope(AddImportRef(context, scope_inst_id)); diff --git a/toolchain/check/member_access.cpp b/toolchain/check/member_access.cpp index 26bc9b6059df..1d002a7348bb 100644 --- a/toolchain/check/member_access.cpp +++ b/toolchain/check/member_access.cpp @@ -57,16 +57,19 @@ static auto IsInstanceMethod(const SemIR::File& sem_ir, static auto GetHighestAllowedAccess(Context& context, SemIR::LocId loc_id, SemIR::ConstantId name_scope_const_id) -> SemIR::AccessKind { - auto [_, self_type_inst_id, is_poisoned] = context.LookupUnqualifiedName( - loc_id.node_id(), SemIR::NameId::SelfType, /*required=*/false); - CARBON_CHECK(!is_poisoned); - if (!self_type_inst_id.has_value()) { + SemIR::ScopeLookupResult lookup_result = + context + .LookupUnqualifiedName(loc_id.node_id(), SemIR::NameId::SelfType, + /*required=*/false) + .scope_result; + CARBON_CHECK(!lookup_result.is_poisoned()); + if (!lookup_result.is_found()) { return SemIR::AccessKind::Public; } // TODO: Support other types for `Self`. - auto self_class_type = - context.insts().TryGetAs(self_type_inst_id); + auto self_class_type = context.insts().TryGetAs( + lookup_result.target_inst_id()); if (!self_class_type) { return SemIR::AccessKind::Public; } @@ -239,12 +242,12 @@ static auto LookupMemberNameInScope(Context& context, SemIR::LocId loc_id, context.LookupQualifiedName(loc_id, name_id, lookup_scopes, /*required=*/true, access_info); - if (!result.inst_id.has_value()) { + if (!result.scope_result.is_found()) { return SemIR::ErrorInst::SingletonInstId; } // TODO: This duplicates the work that HandleNameAsExpr does. Factor this out. - auto inst = context.insts().Get(result.inst_id); + auto inst = context.insts().Get(result.scope_result.target_inst_id()); auto type_id = SemIR::GetTypeInSpecific(context.sem_ir(), result.specific_id, inst.type_id()); CARBON_CHECK(type_id.has_value(), "Missing type for member {0}", inst); @@ -252,18 +255,23 @@ static auto LookupMemberNameInScope(Context& context, SemIR::LocId loc_id, // If the named entity has a constant value that depends on its specific, // store the specific too. if (result.specific_id.has_value() && - context.constant_values().Get(result.inst_id).is_symbolic()) { - result.inst_id = context.GetOrAddInst( - loc_id, {.type_id = type_id, - .inst_id = result.inst_id, - .specific_id = result.specific_id}); + context.constant_values() + .Get(result.scope_result.target_inst_id()) + .is_symbolic()) { + result.scope_result = SemIR::ScopeLookupResult::MakeFound( + context.GetOrAddInst( + loc_id, {.type_id = type_id, + .inst_id = result.scope_result.target_inst_id(), + .specific_id = result.specific_id}), + SemIR::AccessKind::Public); } // TODO: Use a different kind of instruction that also references the // `base_id` so that `SemIR` consumers can find it. auto member_id = context.GetOrAddInst( - loc_id, - {.type_id = type_id, .name_id = name_id, .value_id = result.inst_id}); + loc_id, {.type_id = type_id, + .name_id = name_id, + .value_id = result.scope_result.target_inst_id()}); // If member name lookup finds an associated entity name, and the scope is not // a facet type, perform impl lookup. diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index cf2c55d7b859..0810296319b0 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -169,7 +169,9 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id, auto& scope = context.name_scopes().Get(scope_id); auto entry_id = scope.Lookup(name_id); if (entry_id) { - scope.GetEntry(*entry_id).inst_id = new_inst_id; + auto& result = scope.GetEntry(*entry_id).result; + result = SemIR::ScopeLookupResult::MakeWrappedLookupResult( + new_inst_id, result.access_kind()); } } diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index b7a1e717acc7..b5fad570e4da 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -672,15 +672,15 @@ class FormatterImpl { out_ << label; } - for (auto [name_id, inst_id, access_kind, is_poisoned] : scope.entries()) { - if (is_poisoned) { + for (auto [name_id, result] : scope.entries()) { + if (result.is_poisoned()) { // TODO: Add poisoned names. continue; } Indent(); out_ << "."; FormatName(name_id); - switch (access_kind) { + switch (result.access_kind()) { case SemIR::AccessKind::Public: break; case SemIR::AccessKind::Protected: @@ -691,7 +691,7 @@ class FormatterImpl { break; } out_ << " = "; - FormatName(inst_id); + FormatName(result.is_found() ? result.target_inst_id() : InstId::None); out_ << "\n"; } diff --git a/toolchain/sem_ir/name_scope.cpp b/toolchain/sem_ir/name_scope.cpp index bd5744b7c109..79aceda1212c 100644 --- a/toolchain/sem_ir/name_scope.cpp +++ b/toolchain/sem_ir/name_scope.cpp @@ -22,10 +22,10 @@ auto NameScope::Print(llvm::raw_ostream& out) const -> void { out << ", names: {"; llvm::ListSeparator sep; for (auto entry : names_) { - if (entry.is_poisoned) { + if (entry.result.is_poisoned()) { continue; } - out << sep << entry.name_id << ": " << entry.inst_id; + out << sep << entry.name_id << ": " << entry.result.target_inst_id(); } out << "}"; @@ -33,8 +33,8 @@ auto NameScope::Print(llvm::raw_ostream& out) const -> void { } auto NameScope::AddRequired(Entry name_entry) -> void { - CARBON_CHECK(!name_entry.is_poisoned, "Cannot add a poisoned name: {0}.", - name_entry.name_id); + CARBON_CHECK(!name_entry.result.is_poisoned(), + "Cannot add a poisoned name: {0}.", name_entry.name_id); auto add_name = [&] { EntryId index(names_.size()); names_.push_back(name_entry); @@ -44,7 +44,7 @@ auto NameScope::AddRequired(Entry name_entry) -> void { if (!result.is_inserted()) { // A required name can overwrite poison. auto& name = names_[result.value().index]; - CARBON_CHECK(name.is_poisoned, "Failed to add required name: {0}", + CARBON_CHECK(name.result.is_poisoned(), "Failed to add required name: {0}", name_entry.name_id); name = name_entry; } @@ -58,18 +58,17 @@ auto NameScope::LookupOrAdd(NameId name_id, InstId inst_id, return {false, EntryId(insert_result.value())}; } - names_.push_back( - {.name_id = name_id, .inst_id = inst_id, .access_kind = access_kind}); + names_.push_back({.name_id = name_id, + .result = ScopeLookupResult::MakeWrappedLookupResult( + inst_id, access_kind)}); return {true, EntryId(names_.size() - 1)}; } auto NameScope::LookupOrPoison(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, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true}); + names_.push_back( + {.name_id = name_id, .result = ScopeLookupResult::MakePoisoned()}); return std::nullopt; } return insert_result.value(); diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index 6cb48b959617..391778d77ac8 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -18,13 +18,95 @@ enum class AccessKind : int8_t { Private, }; +// Represents the result of a name lookup. +// +// Lookup results are constructed through the `Make()` factory functions. Each +// result takes one of a few forms, depending on the function used: +// - Found when the lookup was successful returning an existing `InstId`. Can be +// constructed using `MakeFound()` or `MakeWrappedLookupResult()` with an +// existing `inst_id`. +// - Not found when the name wasn't declared or nor poisoned. Can be constructed +// using `MakeNotFound()` or using `MakeWrappedLookupResult()` with a `None` +// `inst_id`. +// - Poisoned when the name wasn't declared but was poisoned and so also +// considered to not be found in that scope. Can be constructed using +// `MakePoisoned()`. +// - Represent that an error has occurred during lookup. This is still +// considered found and the error `InstId` is considered existing. Can be +// constructed using `MakeError()` or using `MakeWrappedLookupResult()` with +// `ErrorInst::SingletonInstId`. +class ScopeLookupResult { + public: + static auto MakeFound(InstId inst_id, AccessKind access_kind) + -> ScopeLookupResult { + CARBON_CHECK(inst_id.has_value()); + return MakeWrappedLookupResult(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 MakeError() -> ScopeLookupResult { + return MakeFound(ErrorInst::SingletonInstId, AccessKind::Public); + } + + static auto MakeWrappedLookupResult(InstId inst_id, AccessKind access_kind) + -> ScopeLookupResult { + return ScopeLookupResult(inst_id, access_kind, /*is_poisoned=*/false); + } + + // True iff CreatePoisoned() was used. + auto is_poisoned() const -> bool { return is_poisoned_; } + + // 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(); + } + + // 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`. + auto target_inst_id() const -> InstId { + CARBON_CHECK(is_found()); + return inst_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; + + private: + explicit ScopeLookupResult(InstId inst_id, AccessKind access_kind, + bool is_poisoned) + : inst_id_(inst_id), + access_kind_(access_kind), + is_poisoned_(is_poisoned) {} + + InstId inst_id_; + AccessKind access_kind_; + bool is_poisoned_; +}; +static_assert(sizeof(ScopeLookupResult) == 8); + class NameScope : public Printable { public: struct Entry { NameId name_id; - InstId inst_id; - AccessKind access_kind; - bool is_poisoned = false; + ScopeLookupResult result; + + // Equality means they have the same `name_id` and equal `result`. + friend auto operator==(const Entry&, const Entry&) -> bool = default; }; static_assert(sizeof(Entry) == 12); @@ -185,9 +267,9 @@ class NameScopeStore { // The name must never conflict. auto AddRequiredName(NameScopeId scope_id, NameId name_id, InstId inst_id) -> void { - Get(scope_id).AddRequired({.name_id = name_id, - .inst_id = inst_id, - .access_kind = AccessKind::Public}); + Get(scope_id).AddRequired( + {.name_id = name_id, + .result = ScopeLookupResult::MakeFound(inst_id, AccessKind::Public)}); } // Returns the requested name scope. diff --git a/toolchain/sem_ir/name_scope_test.cpp b/toolchain/sem_ir/name_scope_test.cpp index b27ddcacc825..820b7690e4e8 100644 --- a/toolchain/sem_ir/name_scope_test.cpp +++ b/toolchain/sem_ir/name_scope_test.cpp @@ -11,17 +11,80 @@ namespace Carbon::SemIR { namespace { using ::testing::ElementsAre; -using ::testing::Field; using ::testing::Pair; -// NOLINTNEXTLINE(modernize-use-trailing-return-type): From the macro. -MATCHER_P(NameScopeEntryEquals, entry, "") { - return ExplainMatchResult( - AllOf(Field("name_id", &NameScope::Entry::name_id, entry.name_id), - Field("inst_id", &NameScope::Entry::inst_id, entry.inst_id), - Field("access_kind", &NameScope::Entry::access_kind, - entry.access_kind)), - arg, result_listener); +TEST(ScopeLookupResult, MakeWrappedLookupResultUsingExistingInstId) { + InstId inst_id(1); + auto result = ScopeLookupResult::MakeWrappedLookupResult( + inst_id, AccessKind::Protected); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_TRUE(result.is_found()); + EXPECT_EQ(result.target_inst_id(), inst_id); + EXPECT_EQ(result.access_kind(), AccessKind::Protected); +} + +TEST(ScopeLookupResult, MakeWrappedLookupResultUsingNoneInstId) { + auto result = ScopeLookupResult::MakeWrappedLookupResult( + InstId::None, AccessKind::Protected); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_FALSE(result.is_found()); + EXPECT_DEATH(result.target_inst_id(), "is_found"); + EXPECT_EQ(result.access_kind(), AccessKind::Protected); +} + +TEST(ScopeLookupResult, MakeWrappedLookupResultUsingErrorInst) { + auto result = ScopeLookupResult::MakeWrappedLookupResult( + ErrorInst::SingletonInstId, AccessKind::Private); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_TRUE(result.is_found()); + EXPECT_EQ(result.target_inst_id(), ErrorInst::SingletonInstId); + EXPECT_EQ(result.access_kind(), AccessKind::Private); +} + +TEST(ScopeLookupResult, MakeFoundExisting) { + InstId inst_id(1); + auto result = ScopeLookupResult::MakeFound(inst_id, AccessKind::Protected); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_TRUE(result.is_found()); + EXPECT_EQ(result.target_inst_id(), inst_id); + EXPECT_EQ(result.access_kind(), AccessKind::Protected); +} + +TEST(ScopeLookupResult, MakeFoundNone) { + EXPECT_DEATH( + ScopeLookupResult::MakeFound(InstId::None, AccessKind::Protected), + "has_value"); +} + +TEST(ScopeLookupResult, MakeNotFound) { + auto result = ScopeLookupResult::MakeNotFound(); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_FALSE(result.is_found()); + EXPECT_DEATH(result.target_inst_id(), "is_found"); + EXPECT_EQ(result.access_kind(), AccessKind::Public); +} + +TEST(ScopeLookupResult, MakePoisoned) { + auto result = ScopeLookupResult::MakePoisoned(); + + EXPECT_TRUE(result.is_poisoned()); + EXPECT_FALSE(result.is_found()); + EXPECT_DEATH(result.target_inst_id(), "is_found"); + EXPECT_EQ(result.access_kind(), AccessKind::Public); +} + +TEST(ScopeLookupResult, MakeError) { + auto result = ScopeLookupResult::MakeError(); + + EXPECT_FALSE(result.is_poisoned()); + EXPECT_TRUE(result.is_found()); + EXPECT_EQ(result.target_inst_id(), ErrorInst::SingletonInstId); + EXPECT_EQ(result.access_kind(), AccessKind::Public); } TEST(NameScope, Empty) { @@ -51,35 +114,34 @@ TEST(NameScope, Lookup) { NameScopeId parent_scope_id(++id); NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); - NameScope::Entry entry1 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Public}; + NameScope::Entry entry1 = { + .name_id = NameId(++id), + .result = ScopeLookupResult::MakeFound(InstId(++id), AccessKind::Public)}; name_scope.AddRequired(entry1); NameScope::Entry entry2 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Protected}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Protected)}; name_scope.AddRequired(entry2); NameScope::Entry entry3 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Private}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Private)}; name_scope.AddRequired(entry3); auto lookup = name_scope.Lookup(entry1.name_id); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(static_cast(name_scope).GetEntry(*lookup), - NameScopeEntryEquals(entry1)); - EXPECT_THAT(static_cast(name_scope).GetEntry(*lookup), - NameScopeEntryEquals(entry1)); + EXPECT_EQ(static_cast(name_scope).GetEntry(*lookup), entry1); + EXPECT_EQ(static_cast(name_scope).GetEntry(*lookup), + entry1); lookup = name_scope.Lookup(entry2.name_id); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry2)); + EXPECT_EQ(name_scope.GetEntry(*lookup), entry2); lookup = name_scope.Lookup(entry3.name_id); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry3)); + EXPECT_EQ(name_scope.GetEntry(*lookup), entry3); NameId unknown_name_id(++id); lookup = name_scope.Lookup(unknown_name_id); @@ -94,35 +156,34 @@ TEST(NameScope, LookupOrPoison) { NameScopeId parent_scope_id(++id); NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); - NameScope::Entry entry1 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Public}; + NameScope::Entry entry1 = { + .name_id = NameId(++id), + .result = ScopeLookupResult::MakeFound(InstId(++id), AccessKind::Public)}; name_scope.AddRequired(entry1); NameScope::Entry entry2 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Protected}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Protected)}; name_scope.AddRequired(entry2); NameScope::Entry entry3 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Private}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Private)}; name_scope.AddRequired(entry3); auto lookup = name_scope.LookupOrPoison(entry1.name_id); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(static_cast(name_scope).GetEntry(*lookup), - NameScopeEntryEquals(entry1)); - EXPECT_THAT(static_cast(name_scope).GetEntry(*lookup), - NameScopeEntryEquals(entry1)); + 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); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry2)); + EXPECT_EQ(name_scope.GetEntry(*lookup), entry2); lookup = name_scope.LookupOrPoison(entry3.name_id); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry3)); + EXPECT_EQ(name_scope.GetEntry(*lookup), entry3); NameId unknown_name_id(++id); lookup = name_scope.LookupOrPoison(unknown_name_id); @@ -137,55 +198,61 @@ TEST(NameScope, LookupOrAdd) { NameScopeId parent_scope_id(++id); NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); - NameScope::Entry entry1 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Public}; + NameScope::Entry entry1 = { + .name_id = NameId(++id), + .result = ScopeLookupResult::MakeFound(InstId(++id), AccessKind::Public)}; { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry1.name_id, entry1.inst_id, entry1.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry1.name_id, entry1.result.target_inst_id(), + entry1.result.access_kind()); EXPECT_TRUE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry1)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry1); } NameScope::Entry entry2 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Protected}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Protected)}; { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry2.name_id, entry2.inst_id, entry2.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry2.name_id, entry2.result.target_inst_id(), + entry2.result.access_kind()); EXPECT_TRUE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry2)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry2); } NameScope::Entry entry3 = {.name_id = NameId(++id), - .inst_id = InstId(++id), - .access_kind = AccessKind::Private}; + .result = ScopeLookupResult::MakeFound( + InstId(++id), AccessKind::Private)}; { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry3.name_id, entry3.inst_id, entry3.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry3.name_id, entry3.result.target_inst_id(), + entry3.result.access_kind()); EXPECT_TRUE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry3)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry3); } { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry1.name_id, entry1.inst_id, entry1.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry1.name_id, entry1.result.target_inst_id(), + entry1.result.access_kind()); EXPECT_FALSE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry1)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry1); } { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry2.name_id, entry2.inst_id, entry2.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry2.name_id, entry2.result.target_inst_id(), + entry2.result.access_kind()); EXPECT_FALSE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry2)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry2); } { - auto [added, entry_id] = name_scope.LookupOrAdd( - entry3.name_id, entry3.inst_id, entry3.access_kind); + auto [added, entry_id] = + name_scope.LookupOrAdd(entry3.name_id, entry3.result.target_inst_id(), + entry3.result.access_kind()); EXPECT_FALSE(added); - EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry3)); + EXPECT_EQ(name_scope.GetEntry(entry_id), entry3); } } @@ -199,35 +266,26 @@ TEST(NameScope, Poison) { NameId poison1(++id); EXPECT_EQ(name_scope.LookupOrPoison(poison1), std::nullopt); - EXPECT_THAT(name_scope.entries(), - ElementsAre(NameScopeEntryEquals( - NameScope::Entry({.name_id = poison1, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true})))); + EXPECT_THAT( + name_scope.entries(), + ElementsAre(NameScope::Entry( + {.name_id = poison1, .result = ScopeLookupResult::MakePoisoned()}))); NameId poison2(++id); EXPECT_EQ(name_scope.LookupOrPoison(poison2), std::nullopt); - EXPECT_THAT(name_scope.entries(), - ElementsAre(NameScopeEntryEquals(NameScope::Entry( - {.name_id = poison1, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true})), - NameScopeEntryEquals(NameScope::Entry( - {.name_id = poison2, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true})))); + EXPECT_THAT( + name_scope.entries(), + ElementsAre( + NameScope::Entry({.name_id = poison1, + .result = ScopeLookupResult::MakePoisoned()}), + NameScope::Entry({.name_id = poison2, + .result = ScopeLookupResult::MakePoisoned()}))); auto lookup = name_scope.Lookup(poison1); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT( - name_scope.GetEntry(*lookup), - NameScopeEntryEquals(NameScope::Entry({.name_id = poison1, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true}))); + EXPECT_THAT(name_scope.GetEntry(*lookup), + NameScope::Entry({.name_id = poison1, + .result = ScopeLookupResult::MakePoisoned()})); } TEST(NameScope, AddRequiredAfterPoison) { @@ -242,26 +300,22 @@ TEST(NameScope, AddRequiredAfterPoison) { InstId inst_id(++id); EXPECT_EQ(name_scope.LookupOrPoison(name_id), std::nullopt); - EXPECT_THAT(name_scope.entries(), - ElementsAre(NameScopeEntryEquals( - NameScope::Entry({.name_id = name_id, - .inst_id = InstId::None, - .access_kind = AccessKind::Public, - .is_poisoned = true})))); + EXPECT_THAT( + name_scope.entries(), + ElementsAre(NameScope::Entry( + {.name_id = name_id, .result = ScopeLookupResult::MakePoisoned()}))); - NameScope::Entry entry = {.name_id = name_id, - .inst_id = inst_id, - .access_kind = AccessKind::Private}; + 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); ASSERT_NE(lookup, std::nullopt); - EXPECT_THAT( - name_scope.GetEntry(*lookup), - NameScopeEntryEquals(NameScope::Entry({.name_id = name_id, - .inst_id = inst_id, - .access_kind = AccessKind::Private, - .is_poisoned = false}))); + EXPECT_EQ(name_scope.GetEntry(*lookup), + NameScope::Entry({.name_id = name_id, + .result = ScopeLookupResult::MakeFound( + inst_id, AccessKind::Private)})); } TEST(NameScope, ExtendedScopes) {