From 0ffa5bf6599947c9758cef6f4d75fe02eea4c448 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 5 Jun 2024 09:23:36 -0700 Subject: [PATCH] For modifiers, get the TokenKind from DeclKind instead of argument. (#4028) --- toolchain/check/decl_introducer_state.h | 3 +- toolchain/check/handle_alias.cpp | 3 +- toolchain/check/handle_class.cpp | 16 ++--- toolchain/check/handle_export.cpp | 3 +- toolchain/check/handle_function.cpp | 13 ++-- toolchain/check/handle_impl.cpp | 3 +- toolchain/check/handle_import_and_package.cpp | 23 +++---- toolchain/check/handle_interface.cpp | 6 +- toolchain/check/handle_let.cpp | 9 +-- toolchain/check/handle_namespace.cpp | 3 +- toolchain/check/handle_variable.cpp | 6 +- toolchain/check/modifiers.cpp | 63 ++++++++++++++----- toolchain/check/modifiers.h | 10 +-- 13 files changed, 83 insertions(+), 78 deletions(-) diff --git a/toolchain/check/decl_introducer_state.h b/toolchain/check/decl_introducer_state.h index 0816db2ca13d..15fb5630e741 100644 --- a/toolchain/check/decl_introducer_state.h +++ b/toolchain/check/decl_introducer_state.h @@ -27,8 +27,9 @@ struct DeclIntroducerState { Import, Interface, Let, + Library, Namespace, - PackageOrLibrary, + Package, Var }; diff --git a/toolchain/check/handle_alias.cpp b/toolchain/check/handle_alias.cpp index 32e27dae312b..2055bd8ab793 100644 --- a/toolchain/check/handle_alias.cpp +++ b/toolchain/check/handle_alias.cpp @@ -32,8 +32,7 @@ auto HandleAlias(Context& context, Parse::AliasId /*node_id*/) -> bool { auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Alias); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access, - Lex::TokenKind::Alias); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access); if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(introducer.modifier_node_id(ModifierOrder::Access), "access modifier"); diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index bd9c50c09977..7b92449b9eeb 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -189,14 +189,12 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id, context.name_scopes().GetInstIfValid(name_context.parent_scope_id); auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Class); - CheckAccessModifiersOnDecl(context, introducer, Lex::TokenKind::Class, - parent_scope_inst); + CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst); LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Class | KeywordModifierSet::Access | - KeywordModifierSet::Extern, - Lex::TokenKind::Class); - RestrictExternModifierOnDecl(context, introducer, Lex::TokenKind::Class, - parent_scope_inst, is_definition); + KeywordModifierSet::Extern); + RestrictExternModifierOnDecl(context, introducer, parent_scope_inst, + is_definition); if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(introducer.modifier_node_id(ModifierOrder::Access), @@ -368,8 +366,7 @@ auto HandleAdaptDecl(Context& context, Parse::AdaptDeclId node_id) -> bool { // Process modifiers. `extend` is permitted, no others are allowed. auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Adapt); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Extend, - Lex::TokenKind::Adapt); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Extend); auto parent_class_decl = GetCurrentScopeAsClassOrDiagnose(context, node_id, Lex::TokenKind::Adapt); @@ -499,8 +496,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId node_id) -> bool { // Process modifiers. `extend` is required, no others are allowed. auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Base); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Extend, - Lex::TokenKind::Base); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Extend); if (!introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) { CARBON_DIAGNOSTIC(BaseMissingExtend, Error, "Missing `extend` before `base` declaration in class."); diff --git a/toolchain/check/handle_export.cpp b/toolchain/check/handle_export.cpp index 0d79ab28b8ef..d3c482de11e2 100644 --- a/toolchain/check/handle_export.cpp +++ b/toolchain/check/handle_export.cpp @@ -28,8 +28,7 @@ auto HandleExportDecl(Context& context, Parse::ExportDeclId node_id) -> bool { auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Export); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::None, - Lex::TokenKind::Export); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::None); if (name_context.state == DeclNameStack::NameContext::State::Error) { // Should already be diagnosed. diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 05437ffe86d2..814ebe2ab06a 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -50,19 +50,16 @@ static auto DiagnoseModifiers(Context& context, DeclIntroducerState& introducer, SemIR::InstId parent_scope_inst_id, std::optional parent_scope_inst) -> void { - CheckAccessModifiersOnDecl(context, introducer, Lex::TokenKind::Fn, - parent_scope_inst); + CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst); LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access | KeywordModifierSet::Extern | KeywordModifierSet::Method | - KeywordModifierSet::Interface, - Lex::TokenKind::Fn); - RestrictExternModifierOnDecl(context, introducer, Lex::TokenKind::Fn, - parent_scope_inst, is_definition); + KeywordModifierSet::Interface); + RestrictExternModifierOnDecl(context, introducer, parent_scope_inst, + is_definition); CheckMethodModifiersOnFunction(context, introducer, parent_scope_inst_id, parent_scope_inst); - RequireDefaultFinalOnlyInInterfaces(context, introducer, Lex::TokenKind::Fn, - parent_scope_inst); + RequireDefaultFinalOnlyInInterfaces(context, introducer, parent_scope_inst); } // Returns the return slot usage for a function given the computed usage for two diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 59d8cd9fe308..bd0a6117c241 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -194,8 +194,7 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id) // TODO: Handle `final` modifier. auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Impl); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::ImplDecl, - Lex::TokenKind::Impl); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::ImplDecl); // Finish processing the name, which should be empty, but might have // parameters. diff --git a/toolchain/check/handle_import_and_package.cpp b/toolchain/check/handle_import_and_package.cpp index bf3618d4722d..3734c0469f6f 100644 --- a/toolchain/check/handle_import_and_package.cpp +++ b/toolchain/check/handle_import_and_package.cpp @@ -22,40 +22,35 @@ auto HandleImportDecl(Context& context, Parse::ImportDeclId /*node_id*/) -> bool { auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Import); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Export, - Lex::TokenKind::Import); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Export); return true; } auto HandleLibraryIntroducer(Context& context, Parse::LibraryIntroducerId /*node_id*/) -> bool { - context.decl_introducer_state_stack().Push( - DeclIntroducerState::PackageOrLibrary); + context.decl_introducer_state_stack().Push(DeclIntroducerState::Library); return true; } auto HandleLibraryDecl(Context& context, Parse::LibraryDeclId /*node_id*/) -> bool { - auto introducer = context.decl_introducer_state_stack().Pop( - DeclIntroducerState::PackageOrLibrary); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Impl, - Lex::TokenKind::Library); + auto introducer = + context.decl_introducer_state_stack().Pop(DeclIntroducerState::Library); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Impl); return true; } auto HandlePackageIntroducer(Context& context, Parse::PackageIntroducerId /*node_id*/) -> bool { - context.decl_introducer_state_stack().Push( - DeclIntroducerState::PackageOrLibrary); + context.decl_introducer_state_stack().Push(DeclIntroducerState::Package); return true; } auto HandlePackageDecl(Context& context, Parse::PackageDeclId /*node_id*/) -> bool { - auto introducer = context.decl_introducer_state_stack().Pop( - DeclIntroducerState::PackageOrLibrary); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Impl, - Lex::TokenKind::Package); + auto introducer = + context.decl_introducer_state_stack().Pop(DeclIntroducerState::Package); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Impl); return true; } diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index e4dafdcbd9f0..03d8f20cfefe 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -41,10 +41,8 @@ static auto BuildInterfaceDecl(Context& context, context.name_scopes().GetInstIfValid(name_context.parent_scope_id); auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Interface); - CheckAccessModifiersOnDecl(context, introducer, Lex::TokenKind::Interface, - parent_scope_inst); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access, - Lex::TokenKind::Interface); + CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access); if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(introducer.modifier_node_id(ModifierOrder::Access), diff --git a/toolchain/check/handle_let.cpp b/toolchain/check/handle_let.cpp index 54bca1a57dfa..d740000a7799 100644 --- a/toolchain/check/handle_let.cpp +++ b/toolchain/check/handle_let.cpp @@ -84,14 +84,11 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { context.scope_stack().PeekNameScopeId()); auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Let); - CheckAccessModifiersOnDecl(context, introducer, Lex::TokenKind::Let, - parent_scope_inst); - RequireDefaultFinalOnlyInInterfaces(context, introducer, Lex::TokenKind::Let, - parent_scope_inst); + CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst); + RequireDefaultFinalOnlyInInterfaces(context, introducer, parent_scope_inst); LimitModifiersOnDecl( context, introducer, - KeywordModifierSet::Access | KeywordModifierSet::Interface, - Lex::TokenKind::Let); + KeywordModifierSet::Access | KeywordModifierSet::Interface); if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(introducer.modifier_node_id(ModifierOrder::Access), diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index f86ce26fd534..9d5efc9b924d 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/toolchain/check/handle_namespace.cpp @@ -26,8 +26,7 @@ auto HandleNamespace(Context& context, Parse::NamespaceId node_id) -> bool { auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Namespace); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::None, - Lex::TokenKind::Namespace); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::None); auto namespace_inst = SemIR::Namespace{ context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType), diff --git a/toolchain/check/handle_variable.cpp b/toolchain/check/handle_variable.cpp index ba47c93d1cae..81932c316d99 100644 --- a/toolchain/check/handle_variable.cpp +++ b/toolchain/check/handle_variable.cpp @@ -103,10 +103,8 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId node_id) context.scope_stack().PeekNameScopeId()); auto introducer = context.decl_introducer_state_stack().Pop(DeclIntroducerState::Var); - CheckAccessModifiersOnDecl(context, introducer, Lex::TokenKind::Var, - parent_scope_inst); - LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access, - Lex::TokenKind::Var); + CheckAccessModifiersOnDecl(context, introducer, parent_scope_inst); + LimitModifiersOnDecl(context, introducer, KeywordModifierSet::Access); if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(introducer.modifier_node_id(ModifierOrder::Access), "access modifier"); diff --git a/toolchain/check/modifiers.cpp b/toolchain/check/modifiers.cpp index 8d8d486508b7..ae9563199ad5 100644 --- a/toolchain/check/modifiers.cpp +++ b/toolchain/check/modifiers.cpp @@ -8,6 +8,43 @@ namespace Carbon::Check { +// Returns the TokenKind for a DeclKind. +static auto DeclKindToken(DeclIntroducerState::DeclKind decl_kind) + -> Lex::TokenKind { + switch (decl_kind) { + case DeclIntroducerState::Adapt: + return Lex::TokenKind::Adapt; + case DeclIntroducerState::Alias: + return Lex::TokenKind::Alias; + case DeclIntroducerState::Base: + return Lex::TokenKind::Base; + case DeclIntroducerState::Class: + return Lex::TokenKind::Class; + case DeclIntroducerState::Constraint: + return Lex::TokenKind::Constraint; + case DeclIntroducerState::Export: + return Lex::TokenKind::Export; + case DeclIntroducerState::Fn: + return Lex::TokenKind::Fn; + case DeclIntroducerState::Import: + return Lex::TokenKind::Import; + case DeclIntroducerState::Impl: + return Lex::TokenKind::Impl; + case DeclIntroducerState::Interface: + return Lex::TokenKind::Interface; + case DeclIntroducerState::Let: + return Lex::TokenKind::Let; + case DeclIntroducerState::Library: + return Lex::TokenKind::Library; + case DeclIntroducerState::Namespace: + return Lex::TokenKind::Namespace; + case DeclIntroducerState::Package: + return Lex::TokenKind::Package; + case DeclIntroducerState::Var: + return Lex::TokenKind::Var; + } +} + static auto DiagnoseNotAllowed(Context& context, Parse::NodeId modifier_node, Lex::TokenKind decl_kind, llvm::StringRef context_string, @@ -40,7 +77,6 @@ static auto ModifierOrderAsSet(ModifierOrder order) -> KeywordModifierSet { auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, KeywordModifierSet forbidden, - Lex::TokenKind decl_kind, llvm::StringRef context_string, SemIR::LocId context_loc_id) -> void { auto not_allowed = introducer.modifier_set & forbidden; @@ -52,8 +88,9 @@ auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, order_index <= static_cast(ModifierOrder::Last); ++order_index) { auto order = static_cast(order_index); if (not_allowed.HasAnyOf(ModifierOrderAsSet(order))) { - DiagnoseNotAllowed(context, introducer.modifier_node_id(order), decl_kind, - context_string, context_loc_id); + DiagnoseNotAllowed(context, introducer.modifier_node_id(order), + DeclKindToken(introducer.kind), context_string, + context_loc_id); introducer.set_modifier_node_id(order, Parse::NodeId::Invalid); } } @@ -63,7 +100,6 @@ auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, auto CheckAccessModifiersOnDecl(Context& context, DeclIntroducerState& introducer, - Lex::TokenKind decl_kind, std::optional parent_scope_inst) -> void { if (parent_scope_inst) { @@ -73,7 +109,7 @@ auto CheckAccessModifiersOnDecl(Context& context, // the parents of the target scope to determine whether we're at file // scope. ForbidModifiersOnDecl( - context, introducer, KeywordModifierSet::Protected, decl_kind, + context, introducer, KeywordModifierSet::Protected, " at file scope, `protected` is only allowed on class members"); return; } @@ -86,10 +122,9 @@ auto CheckAccessModifiersOnDecl(Context& context, // Otherwise neither `private` nor `protected` allowed. ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Protected, - decl_kind, ", `protected` is only allowed on class members"); ForbidModifiersOnDecl( - context, introducer, KeywordModifierSet::Private, decl_kind, + context, introducer, KeywordModifierSet::Private, ", `private` is only allowed on class members and at file scope"); } @@ -97,20 +132,17 @@ auto CheckMethodModifiersOnFunction( Context& context, DeclIntroducerState& introducer, SemIR::InstId parent_scope_inst_id, std::optional parent_scope_inst) -> void { - const Lex::TokenKind decl_kind = Lex::TokenKind::Fn; if (parent_scope_inst) { if (auto class_decl = parent_scope_inst->TryAs()) { auto inheritance_kind = context.classes().Get(class_decl->class_id).inheritance_kind; if (inheritance_kind == SemIR::Class::Final) { ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Virtual, - decl_kind, " in a non-abstract non-base `class` definition", context.insts().GetLocId(parent_scope_inst_id)); } if (inheritance_kind != SemIR::Class::Abstract) { ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Abstract, - decl_kind, " in a non-abstract `class` definition", context.insts().GetLocId(parent_scope_inst_id)); } @@ -119,33 +151,32 @@ auto CheckMethodModifiersOnFunction( } ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Method, - decl_kind, " outside of a class"); + " outside of a class"); } auto RestrictExternModifierOnDecl(Context& context, DeclIntroducerState& introducer, - Lex::TokenKind decl_kind, std::optional parent_scope_inst, bool is_definition) -> void { if (is_definition) { ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Extern, - decl_kind, " that provides a definition"); + " that provides a definition"); } if (parent_scope_inst && !parent_scope_inst->Is()) { ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Extern, - decl_kind, " that is a member"); + " that is a member"); } } auto RequireDefaultFinalOnlyInInterfaces( - Context& context, DeclIntroducerState& introducer, Lex::TokenKind decl_kind, + Context& context, DeclIntroducerState& introducer, std::optional parent_scope_inst) -> void { if (parent_scope_inst && parent_scope_inst->Is()) { // Both `default` and `final` allowed in an interface definition. return; } ForbidModifiersOnDecl(context, introducer, KeywordModifierSet::Interface, - decl_kind, " outside of an interface"); + " outside of an interface"); } } // namespace Carbon::Check diff --git a/toolchain/check/modifiers.h b/toolchain/check/modifiers.h index 6a99de9bfefc..002b5f12959b 100644 --- a/toolchain/check/modifiers.h +++ b/toolchain/check/modifiers.h @@ -15,7 +15,6 @@ namespace Carbon::Check { // `parent_scope_inst` may be nullopt for a declaration in a block scope. auto CheckAccessModifiersOnDecl(Context& context, DeclIntroducerState& introducer, - Lex::TokenKind decl_kind, std::optional parent_scope_inst) -> void; @@ -35,7 +34,6 @@ auto CheckMethodModifiersOnFunction( // TODO: Take another look at diagnostic phrasing for callers. auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, KeywordModifierSet forbidden, - Lex::TokenKind decl_kind, llvm::StringRef context_string, SemIR::LocId context_loc_id = SemIR::LocId::Invalid) -> void; @@ -44,9 +42,8 @@ auto ForbidModifiersOnDecl(Context& context, DeclIntroducerState& introducer, // not in `allowed`. Updates `introducer`. inline auto LimitModifiersOnDecl(Context& context, DeclIntroducerState& introducer, - KeywordModifierSet allowed, - Lex::TokenKind decl_kind) -> void { - ForbidModifiersOnDecl(context, introducer, ~allowed, decl_kind, ""); + KeywordModifierSet allowed) -> void { + ForbidModifiersOnDecl(context, introducer, ~allowed, ""); } // Restricts the `extern` modifier to only be used on namespace-scoped @@ -57,7 +54,6 @@ inline auto LimitModifiersOnDecl(Context& context, // `parent_scope_inst` may be nullopt for a declaration in a block scope. auto RestrictExternModifierOnDecl(Context& context, DeclIntroducerState& introducer, - Lex::TokenKind decl_kind, std::optional parent_scope_inst, bool is_definition) -> void; @@ -67,7 +63,7 @@ auto RestrictExternModifierOnDecl(Context& context, // // `parent_scope_inst` may be nullopt for a declaration in a block scope. auto RequireDefaultFinalOnlyInInterfaces( - Context& context, DeclIntroducerState& introducer, Lex::TokenKind decl_kind, + Context& context, DeclIntroducerState& introducer, std::optional parent_scope_inst) -> void; } // namespace Carbon::Check