diff --git a/toolchain/check/context.h b/toolchain/check/context.h index b7e67a13152f..c6197cc4c88b 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -123,6 +123,18 @@ class Context { return current_scope().scope_id; } + // Returns true if currently at file scope. + auto at_file_scope() const -> bool { return scope_stack_.size() == 1; } + + // Returns the instruction kind associated with the current scope, if any. + auto current_scope_kind() const -> std::optional { + auto current_scope_inst_id = current_scope().scope_inst_id; + if (!current_scope_inst_id.is_valid()) { + return std::nullopt; + } + return sem_ir_->insts().Get(current_scope_inst_id).kind(); + } + // Returns the current scope, if it is of the specified kind. Otherwise, // returns nullopt. template diff --git a/toolchain/check/decl_state.h b/toolchain/check/decl_state.h index b437a6f29cea..9d14df8b7572 100644 --- a/toolchain/check/decl_state.h +++ b/toolchain/check/decl_state.h @@ -84,13 +84,6 @@ class DeclStateStack { // declaration currently being processed. auto innermost() -> DeclState& { return stack_.back(); } - // Gets the state for the declaration containing the innermost declaration. - // Requires that the innermost declaration is not `FileScope`. - auto containing() const -> const DeclState& { - CARBON_CHECK(stack_.size() >= 2); - return stack_[stack_.size() - 2]; - } - // Exits a declaration of kind `k`. auto Pop(DeclState::DeclKind k) -> void { CARBON_CHECK(stack_.back().kind == k); diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index b862b2a9ccf8..415d60e73a95 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -17,23 +17,22 @@ static auto DiagnoseModifiers(Context& context) -> KeywordModifierSet { KeywordModifierSet::Interface, decl_kind); // Rules for abstract, virtual, and impl, which are only allowed in classes. - auto containing_kind = context.decl_state_stack().containing().kind; - if (containing_kind != DeclState::Class) { - ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind, - " outside of a class"); - } else { - auto containing_decl_modifiers = - context.decl_state_stack().containing().modifier_set; - if (!(containing_decl_modifiers & KeywordModifierSet::Class)) { + if (auto class_decl = context.GetCurrentScopeAs()) { + 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.decl_state_stack().containing().first_node); + class_decl->parse_node); } - if (!(containing_decl_modifiers & KeywordModifierSet::Abstract)) { + if (inheritance_kind != SemIR::Class::Abstract) { ForbidModifiersOnDecl(context, KeywordModifierSet::Abstract, decl_kind, " in a non-abstract `class` definition", - context.decl_state_stack().containing().first_node); + class_decl->parse_node); } + } else { + ForbidModifiersOnDecl(context, KeywordModifierSet::Method, decl_kind, + " outside of a class"); } RequireDefaultFinalOnlyInInterfaces(context, decl_kind); diff --git a/toolchain/check/modifiers.cpp b/toolchain/check/modifiers.cpp index 837ea5927511..e4bdc101b9d0 100644 --- a/toolchain/check/modifiers.cpp +++ b/toolchain/check/modifiers.cpp @@ -64,34 +64,33 @@ auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden, auto CheckAccessModifiersOnDecl(Context& context, Lex::TokenKind decl_kind) -> void { - switch (context.decl_state_stack().containing().kind) { - case DeclState::FileScope: - ForbidModifiersOnDecl( - context, KeywordModifierSet::Protected, decl_kind, - " at file scope, `protected` is only allowed on class members"); - break; - - case DeclState::Class: - // Both `private` and `protected` allowed in a class definition. - break; - - default: - // Otherwise neither `private` nor `protected` allowed. - ForbidModifiersOnDecl(context, KeywordModifierSet::Protected, decl_kind, - ", `protected` is only allowed on class members"); - ForbidModifiersOnDecl( - context, KeywordModifierSet::Private, decl_kind, - ", `private` is only allowed on class members and at file scope"); - break; + if (context.at_file_scope()) { + ForbidModifiersOnDecl( + context, KeywordModifierSet::Protected, decl_kind, + " at file scope, `protected` is only allowed on class members"); + return; } + + if (auto kind = context.current_scope_kind()) { + if (*kind == SemIR::ClassDecl::Kind) { + // Both `private` and `protected` allowed in a class definition. + return; + } + } + + // Otherwise neither `private` nor `protected` allowed. + ForbidModifiersOnDecl(context, KeywordModifierSet::Protected, decl_kind, + ", `protected` is only allowed on class members"); + ForbidModifiersOnDecl( + context, KeywordModifierSet::Private, decl_kind, + ", `private` is only allowed on class members and at file scope"); } auto RequireDefaultFinalOnlyInInterfaces(Context& context, Lex::TokenKind decl_kind) -> void { - if (context.decl_state_stack().containing().kind != DeclState::Interface) { - ForbidModifiersOnDecl(context, KeywordModifierSet::Interface, decl_kind, - " outside of an interface"); - } + // TODO: Skip this if *context.current_scope_kind() == SemIR::InterfaceDecl + ForbidModifiersOnDecl(context, KeywordModifierSet::Interface, decl_kind, + " outside of an interface"); } } // namespace Carbon::Check