diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index 74afd4d3ba52..62e573d69379 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -183,14 +183,16 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id, .PopAndDiscardSoloNodeId(); // Process modifiers. + auto [_, enclosing_scope_inst] = + context.name_scopes().GetInstIfValid(name_context.enclosing_scope_id); CheckAccessModifiersOnDecl(context, Lex::TokenKind::Class, - name_context.enclosing_scope_id); + enclosing_scope_inst); LimitModifiersOnDecl(context, KeywordModifierSet::Class | KeywordModifierSet::Access | KeywordModifierSet::Extern, Lex::TokenKind::Class); RestrictExternModifierOnDecl(context, Lex::TokenKind::Class, - name_context.enclosing_scope_id, is_definition); + enclosing_scope_inst, is_definition); auto modifiers = context.decl_state_stack().innermost().modifier_set; if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 1c8521397cab..cc621d65fe9a 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -45,19 +45,21 @@ auto HandleReturnType(Context& context, Parse::ReturnTypeId node_id) -> bool { } static auto DiagnoseModifiers(Context& context, bool is_definition, - SemIR::NameScopeId enclosing_scope_id) + SemIR::InstId enclosing_scope_inst_id, + std::optional enclosing_scope_inst) -> KeywordModifierSet { - const Lex::TokenKind decl_kind = Lex::TokenKind::Fn; - CheckAccessModifiersOnDecl(context, decl_kind, enclosing_scope_id); + CheckAccessModifiersOnDecl(context, Lex::TokenKind::Fn, enclosing_scope_inst); LimitModifiersOnDecl(context, KeywordModifierSet::Access | KeywordModifierSet::Extern | KeywordModifierSet::Method | KeywordModifierSet::Interface, - decl_kind); - RestrictExternModifierOnDecl(context, decl_kind, enclosing_scope_id, - is_definition); - CheckMethodModifiersOnFunction(context, enclosing_scope_id); - RequireDefaultFinalOnlyInInterfaces(context, decl_kind, enclosing_scope_id); + Lex::TokenKind::Fn); + RestrictExternModifierOnDecl(context, Lex::TokenKind::Fn, + enclosing_scope_inst, is_definition); + CheckMethodModifiersOnFunction(context, enclosing_scope_inst_id, + enclosing_scope_inst); + RequireDefaultFinalOnlyInInterfaces(context, Lex::TokenKind::Fn, + enclosing_scope_inst); return context.decl_state_stack().innermost().modifier_set; } @@ -222,8 +224,10 @@ static auto BuildFunctionDecl(Context& context, .PopAndDiscardSoloNodeId(); // Process modifiers. - auto modifiers = DiagnoseModifiers(context, is_definition, - name_context.enclosing_scope_id); + auto [enclosing_scope_inst_id, enclosing_scope_inst] = + context.name_scopes().GetInstIfValid(name_context.enclosing_scope_id); + auto modifiers = DiagnoseModifiers( + context, is_definition, enclosing_scope_inst_id, enclosing_scope_inst); if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), @@ -279,13 +283,9 @@ static auto BuildFunctionDecl(Context& context, // At interface scope, a function declaration introduces an associated // function. auto lookup_result_id = function_info.decl_id; - if (name_context.enclosing_scope_id_for_new_inst().is_valid() && - !name_context.has_qualifiers) { - auto scope_inst_id = context.name_scopes().GetInstIdIfValid( - name_context.enclosing_scope_id_for_new_inst()); + if (enclosing_scope_inst && !name_context.has_qualifiers) { if (auto interface_scope = - context.insts().TryGetAsIfValid( - scope_inst_id)) { + enclosing_scope_inst->TryAs()) { lookup_result_id = BuildAssociatedEntity( context, interface_scope->interface_id, function_info.decl_id); } diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 39b780786b62..61aa1036ad5d 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -36,8 +36,10 @@ static auto BuildInterfaceDecl(Context& context, .PopAndDiscardSoloNodeId(); // Process modifiers. + auto [_, enclosing_scope_inst] = + context.name_scopes().GetInstIfValid(name_context.enclosing_scope_id); CheckAccessModifiersOnDecl(context, Lex::TokenKind::Interface, - name_context.enclosing_scope_id); + enclosing_scope_inst); LimitModifiersOnDecl(context, KeywordModifierSet::Access, Lex::TokenKind::Interface); diff --git a/toolchain/check/handle_let.cpp b/toolchain/check/handle_let.cpp index 83f47c0c266a..7c4355d7a2e6 100644 --- a/toolchain/check/handle_let.cpp +++ b/toolchain/check/handle_let.cpp @@ -77,10 +77,13 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { // Process declaration modifiers. // TODO: For a qualified `let` declaration, this should use the target scope // of the name introduced in the declaration. See #2590. + auto [enclosing_scope_inst_id, enclosing_scope_inst] = + context.name_scopes().GetInstIfValid( + context.scope_stack().PeekNameScopeId()); CheckAccessModifiersOnDecl(context, Lex::TokenKind::Let, - context.scope_stack().PeekNameScopeId()); + enclosing_scope_inst); RequireDefaultFinalOnlyInInterfaces(context, Lex::TokenKind::Let, - context.scope_stack().PeekNameScopeId()); + enclosing_scope_inst); LimitModifiersOnDecl( context, KeywordModifierSet::Access | KeywordModifierSet::Interface, Lex::TokenKind::Let); @@ -136,7 +139,7 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { // Add the name of the binding to the current scope. auto name_id = context.bind_names().Get(bind_name.bind_name_id).name_id; context.AddNameToLookup(name_id, pattern_id); - if (context.scope_stack().PeekNameScopeId() == SemIR::NameScopeId::Package) { + if (enclosing_scope_inst_id == SemIR::InstId::PackageNamespace) { context.AddExport(pattern_id); } return true; diff --git a/toolchain/check/handle_variable.cpp b/toolchain/check/handle_variable.cpp index f3a496696753..25188508b624 100644 --- a/toolchain/check/handle_variable.cpp +++ b/toolchain/check/handle_variable.cpp @@ -97,8 +97,10 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId node_id) // Process declaration modifiers. // TODO: For a qualified `var` declaration, this should use the target scope // of the name introduced in the declaration. See #2590. + auto [_, enclosing_scope_inst] = context.name_scopes().GetInstIfValid( + context.scope_stack().PeekNameScopeId()); CheckAccessModifiersOnDecl(context, Lex::TokenKind::Var, - context.scope_stack().PeekNameScopeId()); + enclosing_scope_inst); LimitModifiersOnDecl(context, KeywordModifierSet::Access, Lex::TokenKind::Var); auto modifiers = context.decl_state_stack().innermost().modifier_set; diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 3ceedf6aeb9b..308408cb5b97 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -495,14 +495,15 @@ class ImportRefResolver { // unresolved constants to the work stack. auto GetLocalNameScopeId(SemIR::NameScopeId name_scope_id) -> SemIR::NameScopeId { - auto inst_id = import_ir_.name_scopes().GetInstIdIfValid(name_scope_id); - if (!inst_id.is_valid()) { + auto [inst_id, inst] = + import_ir_.name_scopes().GetInstIfValid(name_scope_id); + if (!inst) { // Map scopes that aren't associated with an instruction to invalid // scopes. For now, such scopes aren't used, and we don't have a good way // to remap them. return SemIR::NameScopeId::Invalid; } - if (import_ir_.insts().Is(inst_id)) { + if (inst->Is()) { // TODO: Import the scope for an `impl` definition. return SemIR::NameScopeId::Invalid; } diff --git a/toolchain/check/member_access.cpp b/toolchain/check/member_access.cpp index dec4f451fdae..02718a92f841 100644 --- a/toolchain/check/member_access.cpp +++ b/toolchain/check/member_access.cpp @@ -97,18 +97,17 @@ static auto IsInstanceMethod(const SemIR::File& sem_ir, // performed if we find an associated entity. static auto ScopeNeedsImplLookup(Context& context, SemIR::NameScopeId name_scope_id) -> bool { - auto inst_id = context.name_scopes().GetInstIdIfValid(name_scope_id); - if (!inst_id.is_valid()) { + auto [_, inst] = context.name_scopes().GetInstIfValid(name_scope_id); + if (!inst) { return false; } - auto inst = context.insts().Get(inst_id); - if (inst.Is()) { + if (inst->Is()) { // Don't perform impl lookup if an associated entity is named as a member of // a facet type. return false; } - if (inst.Is()) { + if (inst->Is()) { // Don't perform impl lookup if an associated entity is named as a namespace // member. // TODO: This case is not yet listed in the design. diff --git a/toolchain/check/modifiers.cpp b/toolchain/check/modifiers.cpp index 280d7e2b29bf..1af512b2b7ff 100644 --- a/toolchain/check/modifiers.cpp +++ b/toolchain/check/modifiers.cpp @@ -61,43 +61,25 @@ auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden, s.modifier_set.Remove(forbidden); } -// Returns the instruction that owns the given scope, or Invalid if the scope is -// not associated with an instruction. -static auto GetScopeInstId(Context& context, SemIR::NameScopeId scope_id) - -> SemIR::InstId { - if (!scope_id.is_valid()) { - return SemIR::InstId::Invalid; - } - return context.name_scopes().Get(scope_id).inst_id; -} - -// Returns the instruction that owns the given scope, or Invalid if the scope is -// not associated with an instruction. -static auto GetScopeInst(Context& context, SemIR::NameScopeId scope_id) - -> std::optional { - auto inst_id = GetScopeInstId(context, scope_id); - if (!inst_id.is_valid()) { - return std::nullopt; - } - return context.insts().Get(inst_id); -} - auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id) -> void { - auto target = GetScopeInst(context, enclosing_scope_id); - if (target && target->Is()) { - // TODO: This assumes that namespaces can only be declared at file scope. If - // we add support for non-file-scope namespaces, we will need to check the - // parents of the target scope to determine whether we're at file scope. - ForbidModifiersOnDecl( - context, KeywordModifierSet::Protected, decl_kind, - " at file scope, `protected` is only allowed on class members"); - return; - } + std::optional enclosing_scope_inst) + -> void { + if (enclosing_scope_inst) { + if (enclosing_scope_inst->Is()) { + // TODO: This assumes that namespaces can only be declared at file scope. + // If we add support for non-file-scope namespaces, we will need to check + // the parents of the target scope to determine whether we're at file + // scope. + ForbidModifiersOnDecl( + context, KeywordModifierSet::Protected, decl_kind, + " at file scope, `protected` is only allowed on class members"); + return; + } - if (target && target->Is()) { - // Both `private` and `protected` allowed in a class definition. - return; + if (enclosing_scope_inst->Is()) { + // Both `private` and `protected` allowed in a class definition. + return; + } } // Otherwise neither `private` nor `protected` allowed. @@ -108,25 +90,25 @@ auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind, ", `private` is only allowed on class members and at file scope"); } -auto CheckMethodModifiersOnFunction(Context& context, - SemIR::NameScopeId enclosing_scope_id) - -> void { +auto CheckMethodModifiersOnFunction( + Context& context, SemIR::InstId enclosing_scope_inst_id, + std::optional enclosing_scope_inst) -> void { const Lex::TokenKind decl_kind = Lex::TokenKind::Fn; - auto target_id = GetScopeInstId(context, enclosing_scope_id); - if (target_id.is_valid()) { - if (auto class_decl = - context.insts().TryGetAs(target_id)) { + if (enclosing_scope_inst) { + if (auto class_decl = enclosing_scope_inst->TryAs()) { auto inheritance_kind = context.classes().Get(class_decl->class_id).inheritance_kind; if (inheritance_kind == SemIR::Class::Final) { - ForbidModifiersOnDecl(context, KeywordModifierSet::Virtual, decl_kind, - " in a non-abstract non-base `class` definition", - context.insts().GetLocId(target_id)); + ForbidModifiersOnDecl( + context, KeywordModifierSet::Virtual, decl_kind, + " in a non-abstract non-base `class` definition", + context.insts().GetLocId(enclosing_scope_inst_id)); } if (inheritance_kind != SemIR::Class::Abstract) { - ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind, - " in a non-abstract `class` definition", - context.insts().GetLocId(target_id)); + ForbidModifiersOnDecl( + context, KeywordModifierSet::Abstract, decl_kind, + " in a non-abstract `class` definition", + context.insts().GetLocId(enclosing_scope_inst_id)); } return; } @@ -136,29 +118,25 @@ auto CheckMethodModifiersOnFunction(Context& context, " outside of a class"); } -auto RestrictExternModifierOnDecl(Context& context, Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id, - bool is_definition) -> void { +auto RestrictExternModifierOnDecl( + Context& context, Lex::TokenKind decl_kind, + std::optional enclosing_scope_inst, bool is_definition) + -> void { if (is_definition) { ForbidModifiersOnDecl(context, KeywordModifierSet::Extern, decl_kind, " that provides a definition"); } - if (enclosing_scope_id.is_valid()) { - auto target_id = context.name_scopes().Get(enclosing_scope_id).inst_id; - if (target_id.is_valid() && - !context.insts().Is(target_id)) { - ForbidModifiersOnDecl(context, KeywordModifierSet::Extern, decl_kind, - " that is a member"); - } + if (enclosing_scope_inst && !enclosing_scope_inst->Is()) { + ForbidModifiersOnDecl(context, KeywordModifierSet::Extern, decl_kind, + " that is a member"); } } -auto RequireDefaultFinalOnlyInInterfaces(Context& context, - Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id) - -> void { - auto target = GetScopeInst(context, enclosing_scope_id); - if (target && target->Is()) { +auto RequireDefaultFinalOnlyInInterfaces( + Context& context, Lex::TokenKind decl_kind, + std::optional enclosing_scope_inst) -> void { + if (enclosing_scope_inst && + enclosing_scope_inst->Is()) { // Both `default` and `final` allowed in an interface definition. return; } diff --git a/toolchain/check/modifiers.h b/toolchain/check/modifiers.h index 36f8144b9e07..7c9b8bbc3440 100644 --- a/toolchain/check/modifiers.h +++ b/toolchain/check/modifiers.h @@ -10,21 +10,22 @@ namespace Carbon::Check { // Reports a diagnostic if access control modifiers on this are not allowed for -// a declaration in `enclosing_scope_id`, and updates the declaration state in +// a declaration in `enclosing_scope_inst`, and updates the declaration state in // `context`. // -// `enclosing_scope_id` may be Invalid for a declaration in a block scope. +// `enclosing_scope_inst` may be nullopt for a declaration in a block scope. auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id) -> void; + std::optional enclosing_scope_inst) + -> void; // Reports a diagnostic if the method function modifiers `abstract`, `virtual`, // or `impl` are present but not permitted on a function declaration in -// `enclosing_scope_id`. +// `enclosing_scope_inst`. // -// `enclosing_scope_id` may be Invalid for a declaration in a block scope. -auto CheckMethodModifiersOnFunction(Context& context, - SemIR::NameScopeId enclosing_scope_id) - -> void; +// `enclosing_scope_inst` may be nullopt for a declaration in a block scope. +auto CheckMethodModifiersOnFunction( + Context& context, SemIR::InstId enclosing_scope_inst_id, + std::optional enclosing_scope_inst) -> void; // Like `LimitModifiersOnDecl`, except says which modifiers are forbidden, and a // `context_string` (and optional `context_loc_id`) specifying the context in @@ -48,19 +49,21 @@ inline auto LimitModifiersOnDecl(Context& context, KeywordModifierSet allowed, // declarations, diagnosing and removing it on: // - `extern` on a definition. // - `extern` on a scoped entity. -auto RestrictExternModifierOnDecl(Context& context, Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id, - bool is_definition) -> void; +// +// `enclosing_scope_inst` may be nullopt for a declaration in a block scope. +auto RestrictExternModifierOnDecl( + Context& context, Lex::TokenKind decl_kind, + std::optional enclosing_scope_inst, bool is_definition) + -> void; // Report a diagonostic if `default` and `final` modifiers are used on // declarations where they are not allowed. Right now they are only allowed // inside interfaces. // -// `enclosing_scope_id` may be Invalid for a declaration in a block scope. -auto RequireDefaultFinalOnlyInInterfaces(Context& context, - Lex::TokenKind decl_kind, - SemIR::NameScopeId enclosing_scope_id) - -> void; +// `enclosing_scope_inst` may be nullopt for a declaration in a block scope. +auto RequireDefaultFinalOnlyInInterfaces( + Context& context, Lex::TokenKind decl_kind, + std::optional enclosing_scope_inst) -> void; } // namespace Carbon::Check diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index 8115033f0743..337245798fde 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -68,6 +68,7 @@ File::File(CheckIRId check_ir_id, SharedValueStores& value_stores, value_stores_(&value_stores), filename_(std::move(filename)), type_blocks_(allocator_), + name_scopes_(&insts_), constant_values_(ConstantId::NotConstant), inst_blocks_(allocator_), constants_(*this, allocator_) { diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 2a57691f7ee5..f1bc96db2a7a 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -212,9 +212,6 @@ class File : public Printable { // that are import-related. ValueStore import_ir_insts_; - // Storage for name scopes. - NameScopeStore name_scopes_; - // Type blocks within the IR. These reference entries in types_. Storage for // the data is provided by allocator_. BlockValueStore type_blocks_; @@ -223,6 +220,9 @@ class File : public Printable { // indices matching BuiltinKind ordering. InstStore insts_; + // Storage for name scopes. + NameScopeStore name_scopes_; + // Constant values for instructions. ConstantValueStore constant_values_; diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index 0df120ff080e..267d55ef4a82 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_SEM_IR_NAME_SCOPE_H_ #include "toolchain/sem_ir/ids.h" +#include "toolchain/sem_ir/inst.h" namespace Carbon::SemIR { @@ -87,6 +88,8 @@ struct NameScope : Printable { // Provides a ValueStore wrapper for an API specific to name scopes. class NameScopeStore { public: + explicit NameScopeStore(InstStore* insts) : insts_(insts) {} + // Adds a name scope, returning an ID to reference it. auto Add(InstId inst_id, NameId name_id, NameScopeId enclosing_scope_id) -> NameScopeId { @@ -103,14 +106,18 @@ class NameScopeStore { return values_.Get(scope_id); } - // Returns the instruction owning the requested name scope, or an invalid - // instruction if the scope is either invalid or has no associated - // instruction. - auto GetInstIdIfValid(NameScopeId scope_id) const -> InstId { + // Returns the instruction owning the requested name scope, or Invalid with + // nullopt if the scope is either invalid or has no associated instruction. + auto GetInstIfValid(NameScopeId scope_id) const + -> std::pair> { if (!scope_id.is_valid()) { - return InstId::Invalid; + return {InstId::Invalid, std::nullopt}; } - return Get(scope_id).inst_id; + auto inst_id = Get(scope_id).inst_id; + if (!inst_id.is_valid()) { + return {InstId::Invalid, std::nullopt}; + } + return {inst_id, insts_->Get(inst_id)}; } auto OutputYaml() const -> Yaml::OutputMapping { @@ -118,6 +125,7 @@ class NameScopeStore { } private: + InstStore* insts_; ValueStore values_; };