diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 73a4cc9bec9cc..4331094ce873d 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -198,7 +198,7 @@ static auto ImportCurrentPackage(Context& context, UnitInfo& unit_info, unit_info.package_imports[import_map_lookup.value()]; if (self_import.has_load_error) { - context.name_scopes().Get(SemIR::NameScopeId::Package).has_error = true; + context.name_scopes().Get(SemIR::NameScopeId::Package).set_has_error(); } ImportLibrariesFromCurrentPackage( @@ -208,7 +208,7 @@ static auto ImportCurrentPackage(Context& context, UnitInfo& unit_info, context.scope_stack().Push( package_inst_id, SemIR::NameScopeId::Package, SemIR::SpecificId::Invalid, - context.name_scopes().Get(SemIR::NameScopeId::Package).has_error); + context.name_scopes().Get(SemIR::NameScopeId::Package).has_error()); } // Imports all other packages (excluding the current package). diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index af4767ff6ffb6..6a99ee4445f0d 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -359,16 +359,16 @@ auto Context::LookupNameInExactScope(SemIRLoc loc, SemIR::NameId name_id, SemIR::NameScopeId scope_id, const SemIR::NameScope& scope) -> std::pair { - if (auto lookup = scope.name_map.Lookup(name_id)) { - auto entry = scope.names[lookup.value()]; + if (auto entry_id = scope.Lookup(name_id)) { + auto entry = scope.GetEntry(*entry_id); LoadImportRef(*this, entry.inst_id); return {entry.inst_id, entry.access_kind}; } - if (!scope.import_ir_scopes.empty()) { + 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), + scope.import_ir_scopes(), name_id), SemIR::AccessKind::Public}; } return {SemIR::InstId::Invalid, SemIR::AccessKind::Public}; @@ -525,7 +525,7 @@ auto Context::LookupQualifiedName(SemIRLoc loc, SemIR::NameId name_id, continue; } const auto& name_scope = name_scopes().Get(scope_id); - has_error |= name_scope.has_error; + has_error |= name_scope.has_error(); auto [scope_result_id, access_kind] = LookupNameInExactScope(loc, name_id, scope_id, name_scope); @@ -546,7 +546,7 @@ auto Context::LookupQualifiedName(SemIRLoc loc, SemIR::NameId name_id, if (!scope_result_id.is_valid() || 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; + const auto& extended = name_scope.extended_scopes(); scopes.reserve(scopes.size() + extended.size()); for (auto extended_id : llvm::reverse(extended)) { // Substitute into the constant describing the extended scope to diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 25abc2e08589b..c8b795e6243c8 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -135,7 +135,7 @@ auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id, auto& name_scope = context_->name_scopes().Get(name_context.parent_scope_id); if (name_context.has_qualifiers) { - auto inst = context_->insts().Get(name_scope.inst_id); + auto inst = context_->insts().Get(name_scope.inst_id()); if (!inst.Is()) { // TODO: Point at the declaration for the scoped entity. CARBON_DIAGNOSTIC( @@ -231,7 +231,7 @@ auto DeclNameStack::ApplyNameQualifier(const NameComponent& name) -> void { if (scope_id.is_valid()) { PushNameQualifierScope(*context_, name_context.resolved_inst_id, scope_id, specific_id, - context_->name_scopes().Get(scope_id).has_error); + context_->name_scopes().Get(scope_id).has_error()); name_context.parent_scope_id = scope_id; } else { name_context.state = NameContext::State::Error; @@ -416,12 +416,12 @@ auto DeclNameStack::ResolveAsScope(const NameContext& name_context, SemIR::InstBlockId::Invalid))) { return InvalidResult; } - if (scope.is_closed_import) { + if (scope.is_closed_import()) { DiagnoseQualifiedDeclInImportedPackage(*context_, name_context.loc_id, - scope.inst_id); + scope.inst_id()); // Only error once per package. Recover by allowing this package name to // be used as a name qualifier. - scope.is_closed_import = false; + scope.set_is_closed_import(false); } return {scope_id, SemIR::SpecificId::Invalid}; } diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index b7c6018eab096..e751b99b280f2 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -410,7 +410,7 @@ auto HandleParseNode(Context& context, Parse::AdaptDeclId node_id) -> bool { // Extend the class scope with the adapted type's scope if requested. if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) { auto& class_scope = context.name_scopes().Get(class_info.scope_id); - class_scope.extended_scopes.push_back(adapted_inst_id); + class_scope.AddExtendedScope(adapted_inst_id); } return true; } @@ -555,9 +555,9 @@ auto HandleParseNode(Context& context, Parse::BaseDeclId node_id) -> bool { if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) { auto& class_scope = context.name_scopes().Get(class_info.scope_id); if (base_info.scope_id.is_valid()) { - class_scope.extended_scopes.push_back(base_info.inst_id); + class_scope.AddExtendedScope(base_info.inst_id); } else { - class_scope.has_error = true; + class_scope.set_has_error(); } } return true; diff --git a/toolchain/check/handle_export.cpp b/toolchain/check/handle_export.cpp index 0203e94df33dd..ed532f3e1923b 100644 --- a/toolchain/check/handle_export.cpp +++ b/toolchain/check/handle_export.cpp @@ -81,8 +81,8 @@ 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 lookup = parent_scope.name_map.Lookup(entity_name.name_id); - auto& scope_inst_id = parent_scope.names[lookup.value()].inst_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; diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 6de37a2e90e90..0d55083d7af94 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -76,10 +76,10 @@ static auto TryAsClassScope(Context& context, SemIR::NameScopeId scope_id) return std::nullopt; } auto& scope = context.name_scopes().Get(scope_id); - if (!scope.inst_id.is_valid()) { + if (!scope.inst_id().is_valid()) { return std::nullopt; } - return context.insts().TryGetAs(scope.inst_id); + return context.insts().TryGetAs(scope.inst_id()); } static auto GetDefaultSelfType(Context& context) -> SemIR::TypeId { @@ -145,7 +145,7 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node, CARBON_DIAGNOSTIC(ExtendImplForall, Error, "cannot `extend` a parameterized `impl`"); context.emitter().Emit(extend_node, ExtendImplForall); - parent_scope.has_error = true; + parent_scope.set_has_error(); return; } @@ -158,7 +158,7 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node, // If the explicit self type is not the default, just bail out. if (self_type_id != GetDefaultSelfType(context)) { diag.Emit(); - parent_scope.has_error = true; + parent_scope.set_has_error(); return; } @@ -176,18 +176,21 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node, if (!context.types().Is(constraint_id)) { context.TODO(node_id, "extending non-facet-type constraint"); - parent_scope.has_error = true; + parent_scope.set_has_error(); return; } - parent_scope.has_error |= !context.TryToDefineType(constraint_id, [&] { - CARBON_DIAGNOSTIC(ExtendUndefinedInterface, Error, - "`extend impl` requires a definition for facet type {0}", - InstIdAsType); - return context.emitter().Build(node_id, ExtendUndefinedInterface, - constraint_inst_id); - }); - - parent_scope.extended_scopes.push_back(constraint_inst_id); + if (!context.TryToDefineType(constraint_id, [&] { + CARBON_DIAGNOSTIC( + ExtendUndefinedInterface, Error, + "`extend impl` requires a definition for facet type {0}", + InstIdAsType); + return context.emitter().Build(node_id, ExtendUndefinedInterface, + constraint_inst_id); + })) { + parent_scope.set_has_error(); + }; + + parent_scope.AddExtendedScope(constraint_inst_id); } // Pops the parameters of an `impl`, forming a `NameComponent` with no diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index 2779855fbd5c7..5967111812c6e 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -75,7 +75,7 @@ static auto BuildAssociatedConstantDecl(Context& context, "single `:!` binding"); context.emitter().Emit(pattern.loc_id, ExpectedSymbolicBindingInAssociatedConstant); - context.name_scopes().Get(interface_info.scope_id).has_error = true; + context.name_scopes().Get(interface_info.scope_id).set_has_error(); return; } diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index cbc1eec8969e9..f4b6e4b22b4f4 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/toolchain/check/handle_namespace.cpp @@ -50,14 +50,17 @@ auto HandleParseNode(Context& context, Parse::NamespaceId node_id) -> bool { // Point at the other namespace. namespace_inst.name_scope_id = existing->name_scope_id; - if (context.name_scopes().Get(existing->name_scope_id).is_closed_import) { + if (context.name_scopes() + .Get(existing->name_scope_id) + .is_closed_import()) { // The existing name is a package name, so this is a name conflict. context.DiagnoseDuplicateName(namespace_id, existing_inst_id); // Treat this as a local namespace name from now on to avoid further // diagnostics. - context.name_scopes().Get(existing->name_scope_id).is_closed_import = - false; + context.name_scopes() + .Get(existing->name_scope_id) + .set_is_closed_import(false); } else if (existing->import_id.is_valid() && !context.insts().GetLocId(existing_inst_id).is_valid()) { // When the name conflict is an imported namespace, fill the location ID diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 4d1788dd59640..debb25096df6b 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -26,6 +26,11 @@ static auto GetImportNameForEntity(const T& entity) -> std::pair { return {entity.name_id, entity.parent_scope_id}; } +template <> +auto GetImportNameForEntity(const SemIR::NameScope& entity) + -> std::pair { + return {entity.name_id(), entity.parent_scope_id()}; +} // Returns name information for the entity, corresponding to IDs in the import // IR rather than the current IR. @@ -99,10 +104,12 @@ static auto AddNamespace(Context& context, SemIR::TypeId namespace_type_id, llvm::function_ref make_import_id) -> NamespaceResult { auto* parent_scope = &context.name_scopes().Get(parent_scope_id); - auto insert_result = - parent_scope->name_map.Insert(name_id, parent_scope->names.size()); - if (!insert_result.is_inserted()) { - auto prev_inst_id = parent_scope->names[insert_result.value()].inst_id; + auto [inserted, entry_id] = parent_scope->LookupOrAdd( + name_id, + // This InstId is temporary and would be overridden if used. + SemIR::InstId::Invalid, SemIR::AccessKind::Public); + if (!inserted) { + auto prev_inst_id = parent_scope->GetEntry(entry_id).inst_id; if (auto namespace_inst = context.insts().TryGetAs(prev_inst_id)) { if (diagnose_duplicate_namespace) { @@ -140,17 +147,12 @@ static auto AddNamespace(Context& context, SemIR::TypeId namespace_type_id, // Diagnose if there's a name conflict, but still produce the namespace to // supersede the name conflict in order to avoid repeat diagnostics. - if (!insert_result.is_inserted()) { - auto& entry = parent_scope->names[insert_result.value()]; + auto& entry = parent_scope->GetEntry(entry_id); + if (!inserted) { context.DiagnoseDuplicateName(namespace_id, entry.inst_id); - entry.inst_id = namespace_id; entry.access_kind = SemIR::AccessKind::Public; - } else { - parent_scope->names.push_back({.name_id = name_id, - .inst_id = namespace_id, - .access_kind = SemIR::AccessKind::Public}); } - + entry.inst_id = namespace_id; return {namespace_inst.name_scope_id, namespace_id, false}; } @@ -237,20 +239,20 @@ static auto CopyAncestorNameScopesFromImportIR( // The namespace hasn't been copied yet, so add it to our list. const auto& scope = import_sem_ir.name_scopes().Get(import_parent_scope_id); auto scope_inst = - import_sem_ir.insts().GetAs(scope.inst_id); + import_sem_ir.insts().GetAs(scope.inst_id()); new_namespaces.push_back(scope_inst.name_scope_id); - import_parent_scope_id = scope.parent_scope_id; + import_parent_scope_id = scope.parent_scope_id(); } // Add ancestor namespace names, starting with the outermost. for (auto import_scope_id : llvm::reverse(new_namespaces)) { auto import_scope = import_sem_ir.name_scopes().Get(import_scope_id); auto name_id = - CopyNameFromImportIR(context, import_sem_ir, import_scope.name_id); + CopyNameFromImportIR(context, import_sem_ir, import_scope.name_id()); scope_cursor = CopySingleNameScopeFromImportIR( context, namespace_type_id, &copied_namespaces, ir_id, - import_scope.inst_id, import_scope_id, scope_cursor, name_id) + import_scope.inst_id(), import_scope_id, scope_cursor, name_id) .name_scope_id; } @@ -265,25 +267,22 @@ static auto AddImportRefOrMerge(Context& context, SemIR::ImportIRId ir_id, SemIR::NameId name_id) -> void { // Leave a placeholder that the inst comes from the other IR. auto& parent_scope = context.name_scopes().Get(parent_scope_id); - auto insert = parent_scope.name_map.Insert(name_id, [&] { + auto [inserted, entry_id] = parent_scope.LookupOrAdd( + name_id, + // This InstId is temporary and would be overridden if used. + SemIR::InstId::Invalid, SemIR::AccessKind::Public); + auto& entry = parent_scope.GetEntry(entry_id); + if (inserted) { auto entity_name_id = context.entity_names().Add( {.name_id = name_id, .parent_scope_id = parent_scope_id, .bind_index = SemIR::CompileTimeBindIndex::Invalid}); - int index = parent_scope.names.size(); - parent_scope.names.push_back( - {.name_id = name_id, - .inst_id = - AddImportRef(context, {.ir_id = ir_id, .inst_id = import_inst_id}, - entity_name_id), - .access_kind = SemIR::AccessKind::Public}); - return index; - }); - if (insert.is_inserted()) { + entry.inst_id = AddImportRef( + context, {.ir_id = ir_id, .inst_id = import_inst_id}, entity_name_id); return; } - auto inst_id = parent_scope.names[insert.value()].inst_id; + auto inst_id = entry.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); @@ -333,7 +332,7 @@ static auto ImportScopeFromApiFile(Context& context, const auto& api_scope = api_sem_ir.name_scopes().Get(api_scope_id); auto& impl_scope = context.name_scopes().Get(impl_scope_id); - for (const auto& api_entry : api_scope.names) { + for (const auto& api_entry : api_scope.entries()) { auto impl_name_id = CopyNameFromImportIR(context, api_sem_ir, api_entry.name_id); if (auto ns = @@ -342,7 +341,7 @@ static auto ImportScopeFromApiFile(Context& context, // ImportLibrariesFromOtherPackage. if (api_scope_id == SemIR::NameScopeId::Package) { const auto& ns_scope = api_sem_ir.name_scopes().Get(ns->name_scope_id); - if (!ns_scope.import_ir_scopes.empty()) { + if (!ns_scope.import_ir_scopes().empty()) { continue; } } @@ -427,8 +426,8 @@ auto ImportLibrariesFromCurrentPackage( // file, it transitively affects the current file too. if (import_ir.sem_ir->name_scopes() .Get(SemIR::NameScopeId::Package) - .has_error) { - context.name_scopes().Get(SemIR::NameScopeId::Package).has_error = true; + .has_error()) { + context.name_scopes().Get(SemIR::NameScopeId::Package).set_has_error(); } } } @@ -450,15 +449,16 @@ auto ImportLibrariesFromOtherPackage(Context& context, auto namespace_const_id = context.constant_values().Get(result.inst_id); auto& scope = context.name_scopes().Get(result.name_scope_id); - scope.is_closed_import = !result.is_duplicate_of_namespace_in_current_package; + scope.set_is_closed_import( + !result.is_duplicate_of_namespace_in_current_package); for (auto import_ir : import_irs) { auto ir_id = AddImportIR(context, import_ir); - scope.import_ir_scopes.push_back({ir_id, SemIR::NameScopeId::Package}); + scope.AddImportIRScope({ir_id, SemIR::NameScopeId::Package}); context.import_ir_constant_values()[ir_id.index].Set( SemIR::Namespace::PackageInstId, namespace_const_id); } if (has_load_error) { - scope.has_error = has_load_error; + scope.set_has_error(); } } @@ -483,13 +483,15 @@ static auto LookupNameInImport(const SemIR::File& import_ir, // Look up the name in the import scope. const auto& import_scope = import_ir.name_scopes().Get(import_scope_id); - auto lookup = import_scope.name_map.Lookup(import_name_id); - if (!lookup) { + auto import_scope_entry_id = import_scope.Lookup(import_name_id); + if (!import_scope_entry_id) { // Name doesn't exist in the import scope. return nullptr; } - const auto& import_scope_entry = import_scope.names[lookup.value()]; + const auto& import_scope_entry = + import_scope.GetEntry(*import_scope_entry_id); + if (import_scope_entry.access_kind != SemIR::AccessKind::Public) { // Ignore cross-package non-public names. return nullptr; @@ -512,8 +514,9 @@ static auto AddNamespaceFromOtherPackage(Context& context, context, namespace_type_id, /*copied_namespaces=*/nullptr, import_ir_id, import_inst_id, import_ns.name_scope_id, parent_scope_id, name_id); auto& scope = context.name_scopes().Get(result.name_scope_id); - scope.is_closed_import = !result.is_duplicate_of_namespace_in_current_package; - scope.import_ir_scopes.push_back({import_ir_id, import_ns.name_scope_id}); + scope.set_is_closed_import( + !result.is_duplicate_of_namespace_in_current_package); + scope.AddImportIRScope({import_ir_id, import_ns.name_scope_id}); return result.inst_id; } @@ -584,8 +587,7 @@ auto ImportNameFromOtherPackage( if (auto import_ns = import_inst.TryAs()) { if (auto ns = context.insts().TryGetAs(result_id)) { auto& name_scope = context.name_scopes().Get(ns->name_scope_id); - name_scope.import_ir_scopes.push_back( - {import_ir_id, import_ns->name_scope_id}); + name_scope.AddImportIRScope({import_ir_id, import_ns->name_scope_id}); continue; } } diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index ae0ec1cb769c4..bbf8b12a6e3bc 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -1204,14 +1204,14 @@ static auto GetIncompleteLocalEntityBase( static auto AddNameScopeImportRefs(ImportContext& context, const SemIR::NameScope& import_scope, SemIR::NameScope& new_scope) -> void { - for (auto entry : import_scope.names) { + for (auto entry : import_scope.entries()) { auto ref_id = AddImportRef(context, entry.inst_id); new_scope.AddRequired({.name_id = GetLocalNameId(context, entry.name_id), .inst_id = ref_id, .access_kind = entry.access_kind}); } - for (auto scope_inst_id : import_scope.extended_scopes) { - new_scope.extended_scopes.push_back(AddImportRef(context, scope_inst_id)); + for (auto scope_inst_id : import_scope.extended_scopes()) { + new_scope.AddExtendedScope(AddImportRef(context, scope_inst_id)); } } @@ -2020,7 +2020,7 @@ static auto AddInterfaceDefinition(ImportContext& context, context.local_context().inst_block_stack().Pop(); new_interface.self_param_id = self_param_id; - CARBON_CHECK(import_scope.extended_scopes.empty(), + CARBON_CHECK(import_scope.extended_scopes().empty(), "Interfaces don't currently have extended scopes to support."); } diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index 1d2a38307f46a..b19eb0cc87ddd 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -169,8 +169,9 @@ auto ReplacePrevInstForMerge(Context& context, SemIR::NameScopeId scope_id, SemIR::NameId name_id, SemIR::InstId new_inst_id) -> void { auto& scope = context.name_scopes().Get(scope_id); - if (auto lookup = scope.name_map.Lookup(name_id)) { - scope.names[lookup.value()].inst_id = new_inst_id; + auto entry_id = scope.Lookup(name_id); + if (entry_id) { + scope.GetEntry(*entry_id).inst_id = new_inst_id; } } diff --git a/toolchain/lower/mangler.cpp b/toolchain/lower/mangler.cpp index fb910e1355336..16ceb05d72574 100644 --- a/toolchain/lower/mangler.cpp +++ b/toolchain/lower/mangler.cpp @@ -39,7 +39,7 @@ auto Mangler::MangleInverseQualifiedNameScope(llvm::raw_ostream& os, continue; } const auto& name_scope = sem_ir().name_scopes().Get(name_scope_id); - CARBON_KIND_SWITCH(sem_ir().insts().Get(name_scope.inst_id)) { + CARBON_KIND_SWITCH(sem_ir().insts().Get(name_scope.inst_id())) { case CARBON_KIND(SemIR::ImplDecl impl_decl): { const auto& impl = sem_ir().impls().Get(impl_decl.impl_id); @@ -110,7 +110,7 @@ auto Mangler::MangleInverseQualifiedNameScope(llvm::raw_ostream& os, break; } case SemIR::Namespace::Kind: { - os << names().GetAsStringIfIdentifier(name_scope.name_id); + os << names().GetAsStringIfIdentifier(name_scope.name_id()); break; } default: @@ -119,7 +119,7 @@ auto Mangler::MangleInverseQualifiedNameScope(llvm::raw_ostream& os, } if (!name_scope.is_imported_package()) { names_to_render.push_back( - {.name_scope_id = name_scope.parent_scope_id, .prefix = '.'}); + {.name_scope_id = name_scope.parent_scope_id(), .prefix = '.'}); } } } diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index 99ef32bc095c0..0d102074dcd89 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -75,6 +75,7 @@ cc_library( "generic.cpp", "impl.cpp", "name.cpp", + "name_scope.cpp", "type.cpp", "type_info.cpp", ], @@ -181,6 +182,17 @@ cc_library( ], ) +cc_test( + name = "name_scope_test", + size = "small", + srcs = ["name_scope_test.cpp"], + deps = [ + ":file", + "//testing/base:gtest_main", + "@googletest//:gtest", + ], +) + cc_test( name = "typed_insts_test", size = "small", diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 49f1fd5d052f0..ff70a25b3e9d3 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -614,8 +614,8 @@ class FormatterImpl { auto FormatNameScope(NameScopeId id, llvm::StringRef label = "") -> void { const auto& scope = sem_ir_->name_scopes().Get(id); - if (scope.names.empty() && scope.extended_scopes.empty() && - scope.import_ir_scopes.empty() && !scope.has_error) { + if (scope.entries().empty() && scope.extended_scopes().empty() && + scope.import_ir_scopes().empty() && !scope.has_error()) { // Name scope is empty. return; } @@ -625,7 +625,7 @@ class FormatterImpl { out_ << label; } - for (auto [name_id, inst_id, access_kind] : scope.names) { + for (auto [name_id, inst_id, access_kind] : scope.entries()) { Indent(); out_ << "."; FormatName(name_id); @@ -644,7 +644,7 @@ class FormatterImpl { out_ << "\n"; } - for (auto extended_scope_id : scope.extended_scopes) { + for (auto extended_scope_id : scope.extended_scopes()) { Indent(); out_ << "extend "; FormatName(extended_scope_id); @@ -656,7 +656,7 @@ class FormatterImpl { // add or remove an unused prelude file, but is intended to still show the // existence of indirect imports. bool has_prelude_components = false; - for (auto [import_ir_id, unused] : scope.import_ir_scopes) { + for (auto [import_ir_id, unused] : scope.import_ir_scopes()) { auto label = GetImportIRLabel(import_ir_id); if (label.starts_with("Core//prelude/")) { if (has_prelude_components) { @@ -671,7 +671,7 @@ class FormatterImpl { out_ << "import " << label << "\n"; } - if (scope.has_error) { + if (scope.has_error()) { Indent(); out_ << "has_error\n"; } diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index e643298bca381..4fb961c5e2cf5 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -17,6 +17,7 @@ namespace Carbon::SemIR { // Forward declare indexed types, for integration with ValueStore. class File; class Inst; +class NameScope; struct EntityName; struct Class; struct FacetTypeInfo; @@ -27,7 +28,6 @@ struct ImportIR; struct ImportIRInst; struct Impl; struct Interface; -struct NameScope; struct StructTypeField; struct TypeInfo; diff --git a/toolchain/sem_ir/inst_namer.cpp b/toolchain/sem_ir/inst_namer.cpp index 83501f0eebeb8..c2c1fdcb23af0 100644 --- a/toolchain/sem_ir/inst_namer.cpp +++ b/toolchain/sem_ir/inst_namer.cpp @@ -675,7 +675,7 @@ auto InstNamer::CollectNamesInBlock(ScopeId scope_id, // The namespace is specified here due to the name conflict. case CARBON_KIND(SemIR::Namespace inst): { add_inst_name_id( - sem_ir_->name_scopes().Get(inst.name_scope_id).name_id); + sem_ir_->name_scopes().Get(inst.name_scope_id).name_id()); continue; } case OutParam::Kind: diff --git a/toolchain/sem_ir/name_scope.cpp b/toolchain/sem_ir/name_scope.cpp new file mode 100644 index 0000000000000..d87e9d2e96626 --- /dev/null +++ b/toolchain/sem_ir/name_scope.cpp @@ -0,0 +1,54 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/sem_ir/name_scope.h" + +namespace Carbon::SemIR { + +auto NameScope::Print(llvm::raw_ostream& out) const -> void { + out << "{inst: " << inst_id_ << ", parent_scope: " << parent_scope_id_ + << ", has_error: " << (has_error_ ? "true" : "false"); + + out << ", extended_scopes: ["; + llvm::ListSeparator scope_sep; + for (auto id : extended_scopes_) { + out << scope_sep << id; + } + out << "]"; + + out << ", names: {"; + llvm::ListSeparator sep; + for (auto entry : names_) { + out << sep << entry.name_id << ": " << entry.inst_id; + } + out << "}"; + + out << "}"; +} + +auto NameScope::AddRequired(Entry name_entry) -> void { + auto add_name = [&] { + EntryId index(names_.size()); + names_.push_back(name_entry); + return index; + }; + auto result = name_map_.Insert(name_entry.name_id, add_name); + CARBON_CHECK(result.is_inserted(), "Failed to add required name: {0}", + name_entry.name_id); +} + +auto NameScope::LookupOrAdd(SemIR::NameId name_id, InstId inst_id, + AccessKind access_kind) + -> std::pair { + auto insert_result = name_map_.Insert(name_id, EntryId(names_.size())); + if (!insert_result.is_inserted()) { + return {false, EntryId(insert_result.value())}; + } + + names_.push_back( + {.name_id = name_id, .inst_id = inst_id, .access_kind = access_kind}); + return {true, EntryId(names_.size() - 1)}; +} + +} // namespace Carbon::SemIR diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index fe985ab7c996a..e7b38278220bb 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -18,52 +18,104 @@ enum class AccessKind : int8_t { Private, }; -struct NameScope : Printable { +class NameScope : public Printable { + public: struct Entry { NameId name_id; InstId inst_id; AccessKind access_kind; }; - auto Print(llvm::raw_ostream& out) const -> void { - out << "{inst: " << inst_id << ", parent_scope: " << parent_scope_id - << ", has_error: " << (has_error ? "true" : "false"); + struct EntryId : public IdBase { + static constexpr llvm::StringLiteral Label = "name_scope_entry"; + using IdBase::IdBase; + }; - out << ", extended_scopes: ["; - llvm::ListSeparator scope_sep; - for (auto id : extended_scopes) { - out << scope_sep << id; - } - out << "]"; + explicit NameScope(InstId inst_id, NameId name_id, + NameScopeId parent_scope_id) + : inst_id_(inst_id), + name_id_(name_id), + parent_scope_id_(parent_scope_id) {} - out << ", names: {"; - llvm::ListSeparator sep; - for (auto entry : names) { - out << sep << entry.name_id << ": " << entry.inst_id; + auto Print(llvm::raw_ostream& out) const -> void; + + auto entries() const -> llvm::ArrayRef { return names_; } + + // Get a specific Name entry based on an EntryId that return from a lookup. + // The Entry could become invalidated if the scope object is invalidated or if + // a name is added. + auto GetEntry(EntryId entry_id) const -> const Entry& { + return names_[entry_id.index]; + } + auto GetEntry(EntryId entry_id) -> Entry& { return names_[entry_id.index]; } + + // Searches for the given name and returns an EntryId if found or nullopt if + // not. + auto Lookup(NameId name_id) const -> std::optional { + auto lookup = name_map_.Lookup(name_id); + if (!lookup) { + return std::nullopt; } - out << "}"; + return lookup.value(); + } + + // Adds a new name known to not exist. + auto AddRequired(Entry name_entry) -> void; + + // If the given name already exists, return true and an EntryId. + // If not, adds the name using inst_id and access_kind and returns false and + // an EntryId. + auto LookupOrAdd(SemIR::NameId name_id, InstId inst_id, + AccessKind access_kind) -> std::pair; - out << "}"; + auto extended_scopes() const -> llvm::ArrayRef { + return extended_scopes_; } - // Adds a name to the scope that must not already exist. - auto AddRequired(Entry name_entry) -> void { - auto add_name = [&] { - int index = names.size(); - names.push_back(name_entry); - return index; - }; - auto result = name_map.Insert(name_entry.name_id, add_name); - CARBON_CHECK(result.is_inserted(), "Failed to add required name: {0}", - name_entry.name_id); + auto AddExtendedScope(SemIR::InstId extended_scope) -> void { + extended_scopes_.push_back(extended_scope); + } + + auto inst_id() const -> InstId { return inst_id_; } + + auto name_id() const -> NameId { return name_id_; } + + auto parent_scope_id() const -> NameScopeId { return parent_scope_id_; } + + auto has_error() const -> bool { return has_error_; } + + // Mark that we have diagnosed an error in a construct that would have added + // names to this scope. + auto set_has_error() -> void { has_error_ = true; } + + auto is_closed_import() const -> bool { return is_closed_import_; } + + auto set_is_closed_import(bool is_closed_import) -> void { + is_closed_import_ = is_closed_import; } // Returns true if this name scope describes an imported package. auto is_imported_package() const -> bool { - return is_closed_import && parent_scope_id == NameScopeId::Package; + return is_closed_import() && parent_scope_id() == NameScopeId::Package; + } + + auto import_ir_scopes() const + -> llvm::ArrayRef> { + return import_ir_scopes_; + } + + auto AddImportIRScope( + const std::pair& import_ir_scope) + -> void { + return import_ir_scopes_.push_back(import_ir_scope); } - // Names in the scope. We store both an insertion-ordered vector for iterating + private: + // Names in the scope. + // Entries could become invalidated if the scope object is invalidated or if a + // name is added. + // + // We store both an insertion-ordered vector for iterating // and a map from `NameId` to the index of that vector for name lookup. // // Optimization notes: this is somewhat memory inefficient. If this ends up @@ -72,8 +124,8 @@ struct NameScope : Printable { // vector so that these can pack densely. If this ends up both cold and memory // intensive, we can also switch the lookup to a set of indices into the // vector rather than a map from `NameId` to index. - llvm::SmallVector names; - Map name_map; + llvm::SmallVector names_; + Map name_map_; // Instructions returning values that are extended by this scope. // @@ -81,31 +133,31 @@ struct NameScope : Printable { // than a single extended scope. // TODO: Revisit this once we have more kinds of extended scope and data. // TODO: Consider using something like `TinyPtrVector` for this. - llvm::SmallVector extended_scopes; + llvm::SmallVector extended_scopes_; // The instruction which owns the scope. - InstId inst_id; + InstId inst_id_; // When the scope is a namespace, the name. Otherwise, invalid. - NameId name_id; + NameId name_id_; // The parent scope. - NameScopeId parent_scope_id; + NameScopeId parent_scope_id_; // Whether we have diagnosed an error in a construct that would have added // names to this scope. For example, this can happen if an `import` failed or - // an `extend` declaration was ill-formed. If true, the `names` map is assumed - // to be missing names as a result of the error, and no further errors are - // produced for lookup failures in this scope. - bool has_error = false; + // an `extend` declaration was ill-formed. If true, names are assumed to be + // missing as a result of the error, and no further errors are produced for + // lookup failures in this scope. + bool has_error_ = false; // True if this is a closed namespace created by importing a package. - bool is_closed_import = false; + bool is_closed_import_ = false; // Imported IR scopes that compose this namespace. This will be empty for // scopes that correspond to the current package. llvm::SmallVector, 0> - import_ir_scopes; + import_ir_scopes_; }; // Provides a ValueStore wrapper for an API specific to name scopes. @@ -116,9 +168,7 @@ class NameScopeStore { // Adds a name scope, returning an ID to reference it. auto Add(InstId inst_id, NameId name_id, NameScopeId parent_scope_id) -> NameScopeId { - return values_.Add({.inst_id = inst_id, - .name_id = name_id, - .parent_scope_id = parent_scope_id}); + return values_.Add(NameScope(inst_id, name_id, parent_scope_id)); } // Adds a name that is required to exist in a name scope, such as `Self`. @@ -145,7 +195,7 @@ class NameScopeStore { if (!scope_id.is_valid()) { return {InstId::Invalid, std::nullopt}; } - auto inst_id = Get(scope_id).inst_id; + auto inst_id = Get(scope_id).inst_id(); if (!inst_id.is_valid()) { return {InstId::Invalid, std::nullopt}; } diff --git a/toolchain/sem_ir/name_scope_test.cpp b/toolchain/sem_ir/name_scope_test.cpp new file mode 100644 index 0000000000000..c34aa1d8f010e --- /dev/null +++ b/toolchain/sem_ir/name_scope_test.cpp @@ -0,0 +1,261 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/sem_ir/name_scope.h" + +#include +#include + +namespace Carbon::SemIR { +namespace { + +using ::testing::ElementsAre; +using ::testing::Field; +using ::testing::Pair; + +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(NameScope, Empty) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id(++id); + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_THAT(name_scope.entries(), ElementsAre()); + EXPECT_THAT(name_scope.extended_scopes(), ElementsAre()); + EXPECT_EQ(name_scope.inst_id(), scope_inst_id); + EXPECT_EQ(name_scope.name_id(), scope_name_id); + EXPECT_EQ(name_scope.parent_scope_id(), parent_scope_id); + EXPECT_FALSE(name_scope.has_error()); + EXPECT_FALSE(name_scope.is_closed_import()); + EXPECT_FALSE(name_scope.is_imported_package()); + EXPECT_THAT(name_scope.import_ir_scopes(), ElementsAre()); +} + +TEST(NameScope, Lookup) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + 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}; + name_scope.AddRequired(entry1); + + NameScope::Entry entry2 = {.name_id = NameId(++id), + .inst_id = InstId(++id), + .access_kind = AccessKind::Protected}; + name_scope.AddRequired(entry2); + + NameScope::Entry entry3 = {.name_id = NameId(++id), + .inst_id = InstId(++id), + .access_kind = 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)); + + lookup = name_scope.Lookup(entry2.name_id); + ASSERT_NE(lookup, std::nullopt); + EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry2)); + + lookup = name_scope.Lookup(entry3.name_id); + ASSERT_NE(lookup, std::nullopt); + EXPECT_THAT(name_scope.GetEntry(*lookup), NameScopeEntryEquals(entry3)); + + NameId unknown_name_id(++id); + lookup = name_scope.Lookup(unknown_name_id); + EXPECT_EQ(lookup, std::nullopt); +} + +TEST(NameScope, LookupOrAdd) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + 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}; + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry1.name_id, entry1.inst_id, entry1.access_kind); + EXPECT_TRUE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry1)); + } + + NameScope::Entry entry2 = {.name_id = NameId(++id), + .inst_id = InstId(++id), + .access_kind = AccessKind::Protected}; + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry2.name_id, entry2.inst_id, entry2.access_kind); + EXPECT_TRUE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry2)); + } + + NameScope::Entry entry3 = {.name_id = NameId(++id), + .inst_id = InstId(++id), + .access_kind = AccessKind::Private}; + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry3.name_id, entry3.inst_id, entry3.access_kind); + EXPECT_TRUE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry3)); + } + + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry1.name_id, entry1.inst_id, entry1.access_kind); + EXPECT_FALSE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry1)); + } + + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry2.name_id, entry2.inst_id, entry2.access_kind); + EXPECT_FALSE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry2)); + } + + { + auto [added, entry_id] = name_scope.LookupOrAdd( + entry3.name_id, entry3.inst_id, entry3.access_kind); + EXPECT_FALSE(added); + EXPECT_THAT(name_scope.GetEntry(entry_id), NameScopeEntryEquals(entry3)); + } +} + +TEST(NameScope, ExtendedScopes) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id = NameScopeId::Package; + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_THAT(name_scope.extended_scopes(), ElementsAre()); + + InstId extended_scope1(++id); + name_scope.AddExtendedScope(extended_scope1); + EXPECT_THAT(name_scope.extended_scopes(), ElementsAre(extended_scope1)); + + InstId extended_scope2(++id); + name_scope.AddExtendedScope(extended_scope2); + EXPECT_THAT(name_scope.extended_scopes(), + ElementsAre(extended_scope1, extended_scope2)); +} + +TEST(NameScope, HasError) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id(++id); + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_FALSE(name_scope.has_error()); + + name_scope.set_has_error(); + EXPECT_TRUE(name_scope.has_error()); + + name_scope.set_has_error(); + EXPECT_TRUE(name_scope.has_error()); +} + +TEST(NameScope, IsClosedImport) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id(++id); + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_FALSE(name_scope.is_closed_import()); + + name_scope.set_is_closed_import(true); + EXPECT_TRUE(name_scope.is_closed_import()); + + name_scope.set_is_closed_import(false); + EXPECT_FALSE(name_scope.is_closed_import()); +} + +TEST(NameScope, IsImportedPackageParentNonPackageScope) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id(++id); + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_FALSE(name_scope.is_imported_package()); + + name_scope.set_is_closed_import(true); + EXPECT_FALSE(name_scope.is_imported_package()); + + name_scope.set_is_closed_import(false); + EXPECT_FALSE(name_scope.is_imported_package()); +} + +TEST(NameScope, IsImportedPackageParentPackageScope) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id = NameScopeId::Package; + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_FALSE(name_scope.is_imported_package()); + + name_scope.set_is_closed_import(true); + EXPECT_TRUE(name_scope.is_imported_package()); + + name_scope.set_is_closed_import(false); + EXPECT_FALSE(name_scope.is_imported_package()); +} + +TEST(NameScope, ImportIRScopes) { + int id = 0; + + InstId scope_inst_id(++id); + NameId scope_name_id(++id); + NameScopeId parent_scope_id = NameScopeId::Package; + NameScope name_scope(scope_inst_id, scope_name_id, parent_scope_id); + + EXPECT_THAT(name_scope.import_ir_scopes(), ElementsAre()); + + ImportIRId import_ir_id1(++id); + NameScopeId import_name_scope_id1(++id); + name_scope.AddImportIRScope({import_ir_id1, import_name_scope_id1}); + EXPECT_THAT(name_scope.import_ir_scopes(), + ElementsAre(Pair(import_ir_id1, import_name_scope_id1))); + + ImportIRId import_ir_id2(++id); + NameScopeId import_name_scope_id2(++id); + name_scope.AddImportIRScope({import_ir_id2, import_name_scope_id2}); + EXPECT_THAT(name_scope.import_ir_scopes(), + ElementsAre(Pair(import_ir_id1, import_name_scope_id1), + Pair(import_ir_id2, import_name_scope_id2))); +} + +} // namespace +} // namespace Carbon::SemIR