diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 9c273280c14f..7b50435c9280 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -72,6 +72,7 @@ cc_library( "function.h", "import_ref.h", "inst_block_stack.h", + "keyword_modifier_set.h", "merge.h", "modifiers.h", "param_and_arg_refs_stack.h", diff --git a/toolchain/check/decl_state.h b/toolchain/check/decl_state.h index da4c24af3b00..ec64086b1ded 100644 --- a/toolchain/check/decl_state.h +++ b/toolchain/check/decl_state.h @@ -5,66 +5,11 @@ #ifndef CARBON_TOOLCHAIN_CHECK_DECL_STATE_H_ #define CARBON_TOOLCHAIN_CHECK_DECL_STATE_H_ -#include "llvm/ADT/BitmaskEnum.h" +#include "toolchain/check/keyword_modifier_set.h" #include "toolchain/parse/node_ids.h" namespace Carbon::Check { -LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); - -// Represents a set of keyword modifiers, using a separate bit per modifier. -// -// We expect this to grow, so are using a bigger size than needed. -// NOLINTNEXTLINE(performance-enum-size) -enum class KeywordModifierSet : uint32_t { - // At most one of these access modifiers allowed for a given declaration, - // and if present it must be first: - Private = 1 << 0, - Protected = 1 << 1, - - // Extern is standalone. - Extern = 1 << 2, - - // At most one of these declaration modifiers allowed for a given - // declaration: - Abstract = 1 << 3, - Base = 1 << 4, - Default = 1 << 5, - Export = 1 << 6, - Extend = 1 << 7, - Final = 1 << 8, - Impl = 1 << 9, - Virtual = 1 << 10, - - // Sets of modifiers: - Access = Private | Protected, - Class = Abstract | Base, - Method = Abstract | Impl | Virtual, - ImplDecl = Extend | Final, - Interface = Default | Final, - Decl = Class | Method | ImplDecl | Interface | Export, - None = 0, - - LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Virtual) -}; - -inline constexpr auto operator!(KeywordModifierSet k) -> bool { - return !static_cast(k); -} - -// The order of modifiers. Each of these corresponds to a group on -// KeywordModifierSet, and can be used as an array index. -enum class ModifierOrder : int8_t { Access, Extern, Decl, Last = Decl }; - -static_assert(!(KeywordModifierSet::Access & KeywordModifierSet::Extern) && - !((KeywordModifierSet::Access | KeywordModifierSet::Extern) & - KeywordModifierSet::Decl), - "Order-related sets must not overlap"); -static_assert(~KeywordModifierSet::None == - (KeywordModifierSet::Access | KeywordModifierSet::Extern | - KeywordModifierSet::Decl), - "Modifier missing from all modifier sets"); - // State stored for each declaration we are currently in: the kind of // declaration and the keyword modifiers that apply to that declaration. struct DeclState { @@ -107,7 +52,7 @@ struct DeclState { Parse::NodeId::Invalid}; // Invariant: contains just the modifiers represented by `saw_*_modifier`. - KeywordModifierSet modifier_set = KeywordModifierSet::None; + KeywordModifierSet modifier_set; }; // Stack of `DeclState` values, representing all the declarations we are diff --git a/toolchain/check/handle_alias.cpp b/toolchain/check/handle_alias.cpp index a2a10e3b9379..864dc3135a6b 100644 --- a/toolchain/check/handle_alias.cpp +++ b/toolchain/check/handle_alias.cpp @@ -30,7 +30,7 @@ auto HandleAlias(Context& context, Parse::AliasId /*node_id*/) -> bool { LimitModifiersOnDecl(context, KeywordModifierSet::Access, Lex::TokenKind::Alias); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index 870f4dc9fe9e..9aaa3b623065 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -198,17 +198,17 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id, name_context.enclosing_scope_id, is_definition); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); } - bool is_extern = !!(modifiers & KeywordModifierSet::Extern); + bool is_extern = modifiers.HasAnyOf(KeywordModifierSet::Extern); auto inheritance_kind = - !!(modifiers & KeywordModifierSet::Abstract) ? SemIR::Class::Abstract - : !!(modifiers & KeywordModifierSet::Base) ? SemIR::Class::Base - : SemIR::Class::Final; + modifiers.HasAnyOf(KeywordModifierSet::Abstract) ? SemIR::Class::Abstract + : modifiers.HasAnyOf(KeywordModifierSet::Base) ? SemIR::Class::Base + : SemIR::Class::Final; context.decl_state_stack().Pop(DeclState::Class); auto decl_block_id = context.inst_block_stack().Pop(); @@ -395,7 +395,7 @@ auto HandleAdaptDecl(Context& context, Parse::AdaptDeclId node_id) -> bool { context.AddInst({node_id, SemIR::AdaptDecl{adapted_type_id}}); // Extend the class scope with the adapted type's scope if requested. - if (!!(modifiers & KeywordModifierSet::Extend)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Extend)) { auto extended_scope_id = SemIR::NameScopeId::Invalid; if (adapted_type_id == SemIR::TypeId::Error) { // Recover by not extending any scope. We instead set has_error to true @@ -497,7 +497,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId node_id) -> bool { LimitModifiersOnDecl(context, KeywordModifierSet::Extend, Lex::TokenKind::Base); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!(modifiers & KeywordModifierSet::Extend)) { + if (!modifiers.HasAnyOf(KeywordModifierSet::Extend)) { CARBON_DIAGNOSTIC(BaseMissingExtend, Error, "Missing `extend` before `base` declaration in class."); context.emitter().Emit(node_id, BaseMissingExtend); @@ -544,7 +544,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId node_id) -> bool { class_info.base_id); // Extend the class scope with the base class. - if (!!(modifiers & KeywordModifierSet::Extend)) { + if (modifiers.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.scope_id); diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 60bfaa370269..2012929457be 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -229,18 +229,18 @@ static auto BuildFunctionDecl(Context& context, // Process modifiers. auto modifiers = DiagnoseModifiers(context, is_definition, name_context.enclosing_scope_id); - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); } - bool is_extern = !!(modifiers & KeywordModifierSet::Extern); - if (!!(modifiers & KeywordModifierSet::Method)) { + bool is_extern = modifiers.HasAnyOf(KeywordModifierSet::Extern); + if (modifiers.HasAnyOf(KeywordModifierSet::Method)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Decl), "method modifier"); } - if (!!(modifiers & KeywordModifierSet::Interface)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Interface)) { // TODO: Once we are saving the modifiers for a function, add check that // the function may only be defined if it is marked `default` or `final`. context.TODO(context.decl_state_stack().innermost().modifier_node_id( diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index f1cd1cf79d58..ddb4d452379f 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -212,8 +212,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id) auto impl_decl_id = context.AddInst({node_id, impl_decl}); // For an `extend impl` declaration, mark the impl as extending this `impl`. - if (!!(context.decl_state_stack().innermost().modifier_set & - KeywordModifierSet::Extend)) { + if (context.decl_state_stack().innermost().modifier_set.HasAnyOf( + KeywordModifierSet::Extend)) { auto extend_node = context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Decl); ExtendImpl(context, extend_node, node_id, self_type_node, self_type_id, diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 2f9a9b32b1c6..034106b92fb7 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -43,7 +43,7 @@ static auto BuildInterfaceDecl(Context& context, Lex::TokenKind::Interface); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); diff --git a/toolchain/check/handle_let.cpp b/toolchain/check/handle_let.cpp index 2db3c4c60afe..83f47c0c266a 100644 --- a/toolchain/check/handle_let.cpp +++ b/toolchain/check/handle_let.cpp @@ -86,12 +86,12 @@ auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { Lex::TokenKind::Let); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); } - if (!!(modifiers & KeywordModifierSet::Interface)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Interface)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Decl), "interface modifier"); diff --git a/toolchain/check/handle_modifier.cpp b/toolchain/check/handle_modifier.cpp index bb81423aa35f..ca256fbae73e 100644 --- a/toolchain/check/handle_modifier.cpp +++ b/toolchain/check/handle_modifier.cpp @@ -39,10 +39,10 @@ static auto HandleModifier(Context& context, Parse::NodeId node_id, ModifierOrder order; KeywordModifierSet later_modifiers; - if (!!(keyword & KeywordModifierSet::Access)) { + if (keyword.HasAnyOf(KeywordModifierSet::Access)) { order = ModifierOrder::Access; later_modifiers = KeywordModifierSet::Extern | KeywordModifierSet::Decl; - } else if (keyword == KeywordModifierSet::Extern) { + } else if (keyword.HasAnyOf(KeywordModifierSet::Extern)) { order = ModifierOrder::Extern; later_modifiers = KeywordModifierSet::Decl; } else { @@ -51,12 +51,12 @@ static auto HandleModifier(Context& context, Parse::NodeId node_id, } auto current_modifier_node_id = s.modifier_node_id(order); - if (!!(s.modifier_set & keyword)) { + if (s.modifier_set.HasAnyOf(keyword)) { DiagnoseRepeated(context, current_modifier_node_id, node_id); } else if (current_modifier_node_id.is_valid()) { DiagnoseNotAllowedWith(context, current_modifier_node_id, node_id); } else if (auto later_modifier_set = s.modifier_set & later_modifiers; - !!later_modifier_set) { + !later_modifier_set.empty()) { // At least one later modifier is present. Diagnose using the closest. Parse::NodeId closest_later_modifier = Parse::NodeId::Invalid; for (auto later_order = static_cast(order) + 1; @@ -79,7 +79,7 @@ static auto HandleModifier(Context& context, Parse::NodeId node_id, context.token_kind(closest_later_modifier)) .Emit(); } else { - s.modifier_set |= keyword; + s.modifier_set.Add(keyword); s.set_modifier_node_id(order, node_id); } return true; diff --git a/toolchain/check/handle_variable.cpp b/toolchain/check/handle_variable.cpp index 6afe8a0769f4..f3a496696753 100644 --- a/toolchain/check/handle_variable.cpp +++ b/toolchain/check/handle_variable.cpp @@ -102,7 +102,7 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId node_id) LimitModifiersOnDecl(context, KeywordModifierSet::Access, Lex::TokenKind::Var); auto modifiers = context.decl_state_stack().innermost().modifier_set; - if (!!(modifiers & KeywordModifierSet::Access)) { + if (modifiers.HasAnyOf(KeywordModifierSet::Access)) { context.TODO(context.decl_state_stack().innermost().modifier_node_id( ModifierOrder::Access), "access modifier"); diff --git a/toolchain/check/keyword_modifier_set.h b/toolchain/check/keyword_modifier_set.h new file mode 100644 index 000000000000..8800d9cc741e --- /dev/null +++ b/toolchain/check/keyword_modifier_set.h @@ -0,0 +1,104 @@ +// 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 + +#ifndef CARBON_TOOLCHAIN_CHECK_KEYWORD_MODIFIER_SET_H_ +#define CARBON_TOOLCHAIN_CHECK_KEYWORD_MODIFIER_SET_H_ + +#include "llvm/ADT/BitmaskEnum.h" + +namespace Carbon::Check { + +LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE(); + +// The order of modifiers. Each of these corresponds to a group on +// KeywordModifierSet, and can be used as an array index. +enum class ModifierOrder : int8_t { Access, Extern, Decl, Last = Decl }; + +// Represents a set of keyword modifiers, using a separate bit per modifier. +class KeywordModifierSet { + public: + // Provide values as an enum. This doesn't expose these as KeywordModifierSet + // instances just due to the duplication of declarations that would cause. + // + // We expect this to grow, so are using a bigger size than needed. + // NOLINTNEXTLINE(performance-enum-size) + enum Enum : uint32_t { + // At most one of these access modifiers allowed for a given declaration, + // and if present it must be first: + Private = 1 << 0, + Protected = 1 << 1, + + // Extern is standalone. + Extern = 1 << 2, + + // At most one of these declaration modifiers allowed for a given + // declaration: + Abstract = 1 << 3, + Base = 1 << 4, + Default = 1 << 5, + Export = 1 << 6, + Extend = 1 << 7, + Final = 1 << 8, + Impl = 1 << 9, + Virtual = 1 << 10, + + // Sets of modifiers: + Access = Private | Protected, + Class = Abstract | Base, + Method = Abstract | Impl | Virtual, + ImplDecl = Extend | Final, + Interface = Default | Final, + Decl = Class | Method | ImplDecl | Interface | Export, + None = 0, + + LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Virtual) + }; + + // Default construct to empty. + explicit KeywordModifierSet() : set_(None) {} + + // Support implicit conversion so that the difference with the member enum is + // opaque. + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr KeywordModifierSet(Enum set) : set_(set) {} + + // Adds entries to the set. + auto Add(KeywordModifierSet set) -> void { set_ |= set.set_; } + // Removes entries from the set. + auto Remove(KeywordModifierSet set) -> void { set_ &= ~set.set_; } + + // Returns true if there's a non-empty set intersection. + constexpr auto HasAnyOf(KeywordModifierSet other) -> bool { + return !(*this & other).empty(); + } + + // Returns true if empty. + constexpr auto empty() -> bool { return set_ == Enum::None; } + + // Returns the set intersection. + constexpr auto operator&(KeywordModifierSet other) -> KeywordModifierSet { + return set_ & other.set_; + } + + // Returns the set inverse. + auto operator~() -> KeywordModifierSet { return ~set_; } + + private: + Enum set_; +}; + +static_assert(!KeywordModifierSet(KeywordModifierSet::Access) + .HasAnyOf(KeywordModifierSet::Extern) && + !KeywordModifierSet(KeywordModifierSet::Access | + KeywordModifierSet::Extern) + .HasAnyOf(KeywordModifierSet::Decl), + "Order-related sets must not overlap"); +static_assert(~KeywordModifierSet::None == + (KeywordModifierSet::Access | KeywordModifierSet::Extern | + KeywordModifierSet::Decl), + "Modifier missing from all modifier sets"); + +} // namespace Carbon::Check + +#endif // CARBON_TOOLCHAIN_CHECK_KEYWORD_MODIFIER_SET_H_ diff --git a/toolchain/check/modifiers.cpp b/toolchain/check/modifiers.cpp index b1efdbe8f914..280d7e2b29bf 100644 --- a/toolchain/check/modifiers.cpp +++ b/toolchain/check/modifiers.cpp @@ -44,21 +44,21 @@ auto ForbidModifiersOnDecl(Context& context, KeywordModifierSet forbidden, SemIR::LocId context_loc_id) -> void { auto& s = context.decl_state_stack().innermost(); auto not_allowed = s.modifier_set & forbidden; - if (!not_allowed) { + if (not_allowed.empty()) { return; } for (auto order_index = 0; order_index <= static_cast(ModifierOrder::Last); ++order_index) { auto order = static_cast(order_index); - if (!!(not_allowed & ModifierOrderAsSet(order))) { + if (not_allowed.HasAnyOf(ModifierOrderAsSet(order))) { DiagnoseNotAllowed(context, s.modifier_node_id(order), decl_kind, context_string, context_loc_id); s.set_modifier_node_id(order, Parse::NodeId::Invalid); } } - s.modifier_set &= ~forbidden; + s.modifier_set.Remove(forbidden); } // Returns the instruction that owns the given scope, or Invalid if the scope is