Просмотр исходного кода

Add an EnumMaskBase type (#6053)

This is a bit of an experiment to see if there's a reasonable way to
write a shared enum type, rather than writing per-case wrappers for
things like `HasTypeQualifiers` or the printing. I think it's a bit
borderline complexity right now, but I'm not sure I can reduce it much
further.

This changes from things like `Internal::EnumClassName##RawEnum` to
`Internal::EnumClassName##Data::RawEnum` so that the enum entries can
have back references to bit shifts without needing to know the
containing type name. Because I'm trying to reduce duplication between
mask and non-mask enums, I did this to non-mask enums too.

This was motivated by #6035 adding another enum mask (which will grow
more entries, and is intended to switch if this is accepted), but I'm
not using that PR as a base here because I didn't want the merge
dependency.
Jon Ross-Perkins 7 месяцев назад
Родитель
Сommit
6cc5d7ed2a

+ 6 - 1
.clang-format

@@ -14,5 +14,10 @@ InsertBraces: 'true'
 PointerAlignment: Left
 # We abuse control macros for formatting other kinds of macros.
 SpaceBeforeParens: ControlStatementsExceptControlMacros
-IfMacros: ['CARBON_DEFINE_RAW_ENUM_CLASS', 'CARBON_KIND_SWITCH']
+IfMacros:
+  [
+    'CARBON_DEFINE_RAW_ENUM_CLASS',
+    'CARBON_DEFINE_RAW_ENUM_MASK',
+    'CARBON_KIND_SWITCH',
+  ]
 StatementMacros: ['ABSTRACT']

+ 9 - 0
common/BUILD

@@ -159,6 +159,15 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "enum_mask_base",
+    hdrs = ["enum_mask_base.h"],
+    deps = [
+        ":enum_base",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
 cc_library(
     name = "error",
     hdrs = ["error.h"],

+ 16 - 9
common/enum_base.h

@@ -165,6 +165,10 @@ class EnumBase : public Printable<DerivedT> {
   }
 
  private:
+  template <typename MaskDerivedT, typename MaskEnumT,
+            const llvm::StringLiteral MaskNames[]>
+  friend class EnumMaskBase;
+
   RawEnumType value_;
 };
 
@@ -175,21 +179,24 @@ class EnumBase : public Printable<DerivedT> {
 // raw enum class.
 #define CARBON_DEFINE_RAW_ENUM_CLASS(EnumClassName, UnderlyingType) \
   namespace Internal {                                              \
-  extern const llvm::StringLiteral EnumClassName##Names[];          \
-  enum class EnumClassName##RawEnum : UnderlyingType;               \
+  struct EnumClassName##Data {                                      \
+    static const llvm::StringLiteral Names[];                       \
+    enum class RawEnum : UnderlyingType;                            \
+  };                                                                \
   }                                                                 \
-  enum class Internal::EnumClassName##RawEnum : UnderlyingType
+  enum class Internal::EnumClassName##Data::RawEnum : UnderlyingType
 
-// In CARBON_DEFINE_RAW_ENUM_CLASS block, use this to generate each enumerator.
+// In the `CARBON_DEFINE_RAW_ENUM_CLASS` block, use this to generate each
+// enumerator.
 #define CARBON_RAW_ENUM_ENUMERATOR(Name) Name,
 
 // Use this to compute the `Internal::EnumBase` specialization for a Carbon enum
 // class. It both computes the name of the raw enum and ensures all the
 // namespaces are correct.
-#define CARBON_ENUM_BASE(EnumClassName)                          \
-  ::Carbon::Internal::EnumBase<EnumClassName,                    \
-                               Internal::EnumClassName##RawEnum, \
-                               Internal::EnumClassName##Names>
+#define CARBON_ENUM_BASE(EnumClassName)                                \
+  ::Carbon::Internal::EnumBase<EnumClassName,                          \
+                               Internal::EnumClassName##Data::RawEnum, \
+                               Internal::EnumClassName##Data::Names>
 
 // Use this within the Carbon enum class body to generate named constant
 // declarations for each value.
@@ -208,7 +215,7 @@ class EnumBase : public Printable<DerivedT> {
 // `clang-format` has a bug with spacing around `->` returns in macros. See
 // https://bugs.llvm.org/show_bug.cgi?id=48320 for details.
 #define CARBON_DEFINE_ENUM_CLASS_NAMES(EnumClassName) \
-  constexpr llvm::StringLiteral Internal::EnumClassName##Names[]
+  constexpr llvm::StringLiteral Internal::EnumClassName##Data::Names[]
 
 // Use this within the names array initializer to generate a string for each
 // name.

+ 158 - 0
common/enum_mask_base.h

@@ -0,0 +1,158 @@
+// 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_COMMON_ENUM_MASK_BASE_H_
+#define CARBON_COMMON_ENUM_MASK_BASE_H_
+
+#include <bit>
+
+#include "common/enum_base.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace Carbon::Internal {
+
+// CRTP-style base class similar to `EnumBase`, but supporting mask enums.
+//
+// Users must be in the `Carbon` namespace and should look like the following.
+//
+// In `my_kind.h`:
+//   ```
+//   #define CARBON_MY_KIND(X) \
+//       X(Enumerator1)        \
+//       X(Enumerator2)        \
+//       X(Enumerator3)        \
+//       ...
+//
+//   CARBON_DEFINE_RAW_ENUM_MASK(MyKind) {
+//     CARBON_MY_KIND(CARBON_RAW_ENUM_MASK_ENUMERATOR)
+//   };
+//
+//   class MyKind : public CARBON_ENUM_MASK_BASE(MyKind) {
+//    public:
+//     CARBON_MY_KIND(CARBON_ENUM_MASK_CONSTANT_DECL)
+//
+//     // Plus, anything else you wish to include.
+//   };
+//
+//   #define CARBON_MY_KIND_WITH_TYPE(X) \
+//     CARBON_ENUM_MASK_CONSTANT_DEFINITION(MyKind, X)
+//   CARBON_MY_KIND(CARBON_MY_KIND_WITH_TYPE)
+//   #undef CARBON_MY_KIND_WITH_TYPE
+//   ```
+//
+// In `my_kind.cpp`:
+//   ```
+//   CARBON_DEFINE_ENUM_MASK_NAMES(MyKind) = {
+//       CARBON_MY_KIND(CARBON_ENUM_MASK_NAME_STRING)};
+//   ```
+template <typename DerivedT, typename EnumT, const llvm::StringLiteral Names[]>
+class EnumMaskBase : public EnumBase<DerivedT, EnumT, Names> {
+ public:
+  // Provide a standard `None`.
+  //
+  // This uses a `&` to trigger slightly different instantiation behaviors in
+  // Clang. For context on why this is needed, see http://wg21.link/CWG2800.
+  // NOLINTNEXTLINE(readability-identifier-naming)
+  static const DerivedT& None;
+
+  // Returns true if there's a non-empty set intersection.
+  constexpr auto HasAnyOf(DerivedT other) const -> bool {
+    return !(*this & other).empty();
+  }
+
+  // Adds entries to the mask.
+  auto Add(DerivedT other) -> void { *this = *this | other; }
+
+  // Removes entries from the mask.
+  auto Remove(DerivedT other) -> void { *this = *this & ~other; }
+
+  constexpr auto operator|(DerivedT other) const -> DerivedT {
+    return DerivedT::FromInt(this->AsInt() | other.AsInt());
+  }
+
+  constexpr auto operator&(DerivedT other) const -> DerivedT {
+    return DerivedT::FromInt(this->AsInt() & other.AsInt());
+  }
+
+  constexpr auto operator~() const -> DerivedT {
+    return DerivedT::FromInt(~this->AsInt());
+  }
+
+  constexpr auto empty() const -> bool { return this->AsInt() == 0; }
+
+  // Returns the name of this value. Requires it to be a single value, not
+  // combined.
+  //
+  // This method will be automatically defined using the static `names` string
+  // table in the base class, which is in turn will be populated for each
+  // derived type using the macro helpers in this file.
+  //
+  // This shadows EnumBase::name.
+  auto name() const -> llvm::StringRef {
+    CARBON_CHECK(std::has_single_bit(this->AsInt()), "Not a single bit: {0}",
+                 this->AsInt());
+    return Names[std::bit_width(this->AsInt())];
+  }
+
+  // Prints this value as a `|`-separated list of mask entries, or `None`.
+  //
+  // This shadows EnumBase::Print.
+  auto Print(llvm::raw_ostream& out) const -> void {
+    int value = this->AsInt();
+    if (value == 0) {
+      out << "None";
+      return;
+    }
+    llvm::ListSeparator sep("|");
+    for (int bit = 0; value != 0; value >>= 1, ++bit) {
+      if (value & 1) {
+        out << sep << Names[bit];
+      }
+    }
+  }
+};
+
+template <typename DerivedT, typename EnumT, const llvm::StringLiteral Names[]>
+constexpr const DerivedT& EnumMaskBase<DerivedT, EnumT, Names>::None =
+    DerivedT::FromInt(0);
+
+}  // namespace Carbon::Internal
+
+// Use this before defining a class that derives from `EnumBase` to begin the
+// definition of the raw `enum class`. It should be followed by the body of that
+// raw enum class.
+#define CARBON_DEFINE_RAW_ENUM_MASK(EnumMaskName, UnderlyingType)              \
+  namespace Internal {                                                         \
+  struct EnumMaskName##Data {                                                  \
+    static const llvm::StringLiteral Names[];                                  \
+    /* For bit shifts, track the initial counter value. This will increment on \
+     * each enum entry. */                                                     \
+    static constexpr uint64_t BitShiftCounter = __COUNTER__ + 1;               \
+    enum class RawEnum : UnderlyingType;                                       \
+  };                                                                           \
+  }                                                                            \
+  enum class Internal::EnumMaskName##Data::RawEnum : UnderlyingType
+
+// In the `CARBON_DEFINE_RAW_ENUM_MASK` block, use this to generate each
+// enumerator.
+#define CARBON_RAW_ENUM_MASK_ENUMERATOR(Name) \
+  Name = 1 << (__COUNTER__ - BitShiftCounter),
+
+// Use this to compute the `Internal::EnumMaskBase` specialization for a Carbon
+// enum mask. It both computes the name of the raw enum and ensures all the
+// namespaces are correct.
+#define CARBON_ENUM_MASK_BASE(EnumMaskName)                               \
+  ::Carbon::Internal::EnumMaskBase<EnumMaskName,                          \
+                                   Internal::EnumMaskName##Data::RawEnum, \
+                                   Internal::EnumMaskName##Data::Names>
+
+// Constants and names are declared equivalently as to `EnumBase`.
+#define CARBON_ENUM_MASK_CONSTANT_DECL(Name) CARBON_ENUM_CONSTANT_DECL(Name)
+#define CARBON_ENUM_MASK_CONSTANT_DEFINITION(EnumMaskName, Name) \
+  CARBON_ENUM_CONSTANT_DEFINITION(EnumMaskName, Name)
+#define CARBON_DEFINE_ENUM_MASK_NAMES(EnumMaskName) \
+  CARBON_DEFINE_ENUM_CLASS_NAMES(EnumMaskName)
+#define CARBON_ENUM_MASK_NAME_STRING(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
+
+#endif  // CARBON_COMMON_ENUM_MASK_BASE_H_

+ 2 - 0
toolchain/check/BUILD

@@ -42,6 +42,7 @@ cc_library(
         "inst.cpp",
         "inst_block_stack.cpp",
         "interface.cpp",
+        "keyword_modifier_set.cpp",
         "literal.cpp",
         "member_access.cpp",
         "merge.cpp",
@@ -118,6 +119,7 @@ cc_library(
         "//common:check",
         "//common:concepts",
         "//common:emplace_by_calling",
+        "//common:enum_mask_base",
         "//common:find",
         "//common:growing_range",
         "//common:map",

+ 4 - 4
toolchain/check/convert.cpp

@@ -773,7 +773,7 @@ static auto CanUseValueOfInitializer(const SemIR::File& sem_ir,
 // of an expression of the given category.
 static auto CanAddQualifiers(SemIR::TypeQualifiers quals,
                              SemIR::ExprCategory cat) -> bool {
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::MaybeUnformed) &&
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::MaybeUnformed) &&
       !SemIR::IsRefCategory(cat)) {
     // `MaybeUnformed(T)` may have a different value representation or
     // initializing representation from `T`, so only allow it to be added for a
@@ -794,13 +794,13 @@ static auto CanAddQualifiers(SemIR::TypeQualifiers quals,
 static auto CanRemoveQualifiers(SemIR::TypeQualifiers quals,
                                 SemIR::ExprCategory cat, bool allow_unsafe)
     -> bool {
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::Const) && !allow_unsafe &&
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::Const) && !allow_unsafe &&
       SemIR::IsRefCategory(cat)) {
     // Removing `const` is an unsafe conversion for a reference expression.
     return false;
   }
 
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::Partial) &&
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::Partial) &&
       (!allow_unsafe || cat == SemIR::ExprCategory::Initializing)) {
     // TODO: Allow removing `partial` for initializing expressions as a safe
     // conversion. `PerformBuiltinConversion` will need to initialize the vptr
@@ -808,7 +808,7 @@ static auto CanRemoveQualifiers(SemIR::TypeQualifiers quals,
     return false;
   }
 
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::MaybeUnformed) &&
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::MaybeUnformed) &&
       (!allow_unsafe || cat == SemIR::ExprCategory::Initializing)) {
     // As an unsafe conversion, `MaybeUnformed` can be removed from a value or
     // reference expression.

+ 7 - 4
toolchain/check/generic.cpp

@@ -23,6 +23,9 @@
 
 namespace Carbon::Check {
 
+CARBON_DEFINE_ENUM_MASK_NAMES(DependentInstKind) = {
+    CARBON_DEPENDENT_INST_KIND(CARBON_ENUM_MASK_NAME_STRING)};
+
 static auto MakeSelfSpecificId(Context& context, SemIR::GenericId generic_id)
     -> SemIR::SpecificId;
 
@@ -302,14 +305,14 @@ auto AttachDependentInstToCurrentGeneric(Context& context,
   if (context.generic_region_stack().Empty()) {
     // This should only happen for `*Decl` instructions, never for template
     // actions.
-    CARBON_CHECK((dep_kind & DependentInst::Template) == DependentInst::None);
+    CARBON_CHECK(!dep_kind.HasAnyOf(DependentInstKind::Template));
     return;
   }
 
   context.generic_region_stack().AddDependentInst(dependent_inst.inst_id);
 
   // If the type is symbolic, replace it with a type specific to this generic.
-  if ((dep_kind & DependentInst::SymbolicType) != DependentInst::None) {
+  if (dep_kind.HasAnyOf(DependentInstKind::SymbolicType)) {
     auto inst = context.insts().Get(inst_id);
     auto type_id = AddGenericTypeToEvalBlock(context, SemIR::LocId(inst_id),
                                              inst.type_id());
@@ -327,7 +330,7 @@ auto AttachDependentInstToCurrentGeneric(Context& context,
   // we'll need to evaluate this instruction when forming the specific. Update
   // the constant value of the instruction to refer to the result of that
   // eventual evaluation.
-  if ((dep_kind & DependentInst::SymbolicConstant) != DependentInst::None) {
+  if (dep_kind.HasAnyOf(DependentInstKind::SymbolicConstant)) {
     // Update the constant value to refer to this generic.
     context.constant_values().Set(
         inst_id, AddGenericConstantToEvalBlock(context, inst_id));
@@ -335,7 +338,7 @@ auto AttachDependentInstToCurrentGeneric(Context& context,
 
   // If the instruction is a template action, add it directly to this position
   // in the eval block.
-  if ((dep_kind & DependentInst::Template) != DependentInst::None) {
+  if (dep_kind.HasAnyOf(DependentInstKind::Template)) {
     AddTemplateActionToEvalBlock(context, inst_id);
   }
 }

+ 25 - 16
toolchain/check/generic.h

@@ -5,15 +5,13 @@
 #ifndef CARBON_TOOLCHAIN_CHECK_GENERIC_H_
 #define CARBON_TOOLCHAIN_CHECK_GENERIC_H_
 
-#include "llvm/ADT/BitmaskEnum.h"
+#include "common/enum_mask_base.h"
 #include "toolchain/check/context.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
 
-LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
-
 // Start processing a declaration or definition that might be a generic entity.
 auto StartGenericDecl(Context& context) -> void;
 
@@ -21,22 +19,33 @@ auto StartGenericDecl(Context& context) -> void;
 auto StartGenericDefinition(Context& context, SemIR::GenericId generic_id)
     -> void;
 
+#define CARBON_DEPENDENT_INST_KIND(X)                                       \
+  /* The type of the instruction depends on a checked generic parameter. */ \
+  X(SymbolicType)                                                           \
+  /* The constant value of the instruction depends on a checked generic     \
+   * parameter. */                                                          \
+  X(SymbolicConstant)                                                       \
+  X(Template)
+
+CARBON_DEFINE_RAW_ENUM_MASK(DependentInstKind, uint8_t) {
+  CARBON_DEPENDENT_INST_KIND(CARBON_RAW_ENUM_MASK_ENUMERATOR)
+};
+
+// Represents a set of keyword modifiers, using a separate bit per modifier.
+class DependentInstKind : public CARBON_ENUM_MASK_BASE(DependentInstKind) {
+ public:
+  CARBON_DEPENDENT_INST_KIND(CARBON_ENUM_MASK_CONSTANT_DECL)
+};
+
+#define CARBON_DEPENDENT_INST_KIND_WITH_TYPE(X) \
+  CARBON_ENUM_MASK_CONSTANT_DEFINITION(DependentInstKind, X)
+CARBON_DEPENDENT_INST_KIND(CARBON_DEPENDENT_INST_KIND_WITH_TYPE)
+#undef CARBON_DEPENDENT_INST_KIND_WITH_TYPE
+
 // An instruction that depends on a generic parameter in some way.
 struct DependentInst {
-  // Ways in which an instruction can depend on a generic parameter.
-  enum Kind : int8_t {
-    None = 0x0,
-    // The type of the instruction depends on a checked generic parameter.
-    SymbolicType = 0x1,
-    // The constant value of the instruction depends on a checked generic
-    // parameter.
-    SymbolicConstant = 0x2,
-    Template = 0x4,
-    LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Template)
-  };
-
   SemIR::InstId inst_id;
-  Kind kind;
+  DependentInstKind kind;
 };
 
 // Attach a dependent instruction to the current generic, updating its type and

+ 5 - 5
toolchain/check/inst.cpp

@@ -18,13 +18,13 @@ namespace Carbon::Check {
 // any applicable instruction lists.
 static auto FinishInst(Context& context, SemIR::InstId inst_id,
                        SemIR::Inst inst) -> void {
-  DependentInst::Kind dep_kind = DependentInst::None;
+  DependentInstKind dep_kind = DependentInstKind::None;
 
   // If the instruction has a symbolic constant type, track that we need to
   // substitute into it.
   if (context.constant_values().DependsOnGenericParameter(
           context.types().GetConstantId(inst.type_id()))) {
-    dep_kind |= DependentInst::SymbolicType;
+    dep_kind.Add(DependentInstKind::SymbolicType);
   }
 
   // If the instruction has a constant value, compute it.
@@ -37,7 +37,7 @@ static auto FinishInst(Context& context, SemIR::InstId inst_id,
     // If the constant value is symbolic, track that we need to substitute into
     // it.
     if (context.constant_values().DependsOnGenericParameter(const_id)) {
-      dep_kind |= DependentInst::SymbolicConstant;
+      dep_kind.Add(DependentInstKind::SymbolicConstant);
     }
   }
 
@@ -48,7 +48,7 @@ static auto FinishInst(Context& context, SemIR::InstId inst_id,
       "Use AddDependentActionInst to add an action instruction");
 
   // Keep track of dependent instructions.
-  if (dep_kind != DependentInst::None) {
+  if (!dep_kind.empty()) {
     AttachDependentInstToCurrentGeneric(context,
                                         {.inst_id = inst_id, .kind = dep_kind});
   }
@@ -86,7 +86,7 @@ auto AddDependentActionInst(Context& context,
 
   // Register the instruction to be added to the eval block.
   AttachDependentInstToCurrentGeneric(
-      context, {.inst_id = inst_id, .kind = DependentInst::Template});
+      context, {.inst_id = inst_id, .kind = DependentInstKind::Template});
   return inst_id;
 }
 

+ 12 - 0
toolchain/check/keyword_modifier_set.cpp

@@ -0,0 +1,12 @@
+// 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/check/keyword_modifier_set.h"
+
+namespace Carbon::Check {
+
+CARBON_DEFINE_ENUM_MASK_NAMES(KeywordModifierSet) = {
+    CARBON_KEYWORD_MODIFIER_SET(CARBON_ENUM_MASK_NAME_STRING)};
+
+}  // namespace Carbon::Check

+ 72 - 87
toolchain/check/keyword_modifier_set.h

@@ -7,75 +7,57 @@
 
 #include <optional>
 
-#include "llvm/ADT/BitmaskEnum.h"
+#include "common/enum_mask_base.h"
 #include "toolchain/sem_ir/name_scope.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, Extend, Decl, Last = Decl };
 
+// A single X-macro to cover modifier groups. These are split out to make groups
+// clearer.
+#define CARBON_KEYWORD_MODIFIER_SET(X)                                       \
+  /* At most one of these access modifiers allowed for a given declaration,  \
+   * and if present it must be first. */                                     \
+  X(Private)                                                                 \
+  X(Protected)                                                               \
+                                                                             \
+  /* Extern is standalone. */                                                \
+  X(Extern)                                                                  \
+                                                                             \
+  /* Extend can be combined with Final, but no others in the group below. */ \
+  X(Extend)                                                                  \
+                                                                             \
+  /* At most one of these declaration modifiers allowed for a given          \
+   * declaration. */                                                         \
+  X(Abstract)                                                                \
+  X(Base)                                                                    \
+  X(Default)                                                                 \
+  X(Export)                                                                  \
+  X(Final)                                                                   \
+  X(Impl)                                                                    \
+  X(Returned)                                                                \
+  X(Virtual)
+
+// We expect this to grow, so are using a bigger size than needed.
+CARBON_DEFINE_RAW_ENUM_MASK(KeywordModifierSet, uint32_t) {
+  CARBON_KEYWORD_MODIFIER_SET(CARBON_RAW_ENUM_MASK_ENUMERATOR)
+};
+
 // Represents a set of keyword modifiers, using a separate bit per modifier.
-class KeywordModifierSet {
+class KeywordModifierSet : public CARBON_ENUM_MASK_BASE(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.
-  enum RawEnumType : 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,
-
-    // Extend can be combined with Final, but no others in the group below.
-    Extend = 1 << 3,
-
-    // At most one of these declaration modifiers allowed for a given
-    // declaration:
-    Abstract = 1 << 4,
-    Base = 1 << 5,
-    Default = 1 << 6,
-    Export = 1 << 7,
-    Final = 1 << 8,
-    Impl = 1 << 9,
-    Virtual = 1 << 10,
-    Returned = 1 << 11,
-
-    // Sets of modifiers:
-    Access = Private | Protected,
-    Class = Abstract | Base,
-    Method = Abstract | Impl | Virtual,
-    ImplDecl = Extend | Final,
-    Interface = Default | Final,
-    Decl = Class | Method | Interface | Export | Returned,
-    None = 0,
-
-    LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Returned)
-  };
-
-  // Default construct to empty.
-  explicit KeywordModifierSet() : set_(None) {}
-
-  // Support implicit conversion so that the difference with the member enum is
-  // opaque.
-  explicit(false) constexpr KeywordModifierSet(RawEnumType 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) const -> bool {
-    return set_ & other.set_;
-  }
+  CARBON_KEYWORD_MODIFIER_SET(CARBON_ENUM_MASK_CONSTANT_DECL)
+
+  // Sets of modifiers.
+  static const KeywordModifierSet Access;
+  static const KeywordModifierSet Class;
+  static const KeywordModifierSet Method;
+  static const KeywordModifierSet ImplDecl;
+  static const KeywordModifierSet Interface;
+  static const KeywordModifierSet Decl;
 
   // Return a builder that returns the new enumeration type once a series of
   // mapping `Case`s and a final `Default` are provided. For example:
@@ -91,8 +73,8 @@ class KeywordModifierSet {
      public:
       explicit Converter(const KeywordModifierSet& set) : set_(set) {}
 
-      auto Case(RawEnumType raw_enumerator, T result) -> Converter& {
-        if (set_.HasAnyOf(raw_enumerator)) {
+      auto Case(KeywordModifierSet other, T result) -> Converter& {
+        if (set_.HasAnyOf(other)) {
           result_ = result;
         }
         return *this;
@@ -122,34 +104,37 @@ class KeywordModifierSet {
     }
     return SemIR::AccessKind::Public;
   }
-
-  // Returns true if empty.
-  constexpr auto empty() const -> bool { return !set_; }
-
-  // Returns the set intersection.
-  constexpr auto operator&(KeywordModifierSet other) const
-      -> KeywordModifierSet {
-    return set_ & other.set_;
-  }
-
-  // Returns the set inverse.
-  auto operator~() const -> KeywordModifierSet { return ~set_; }
-
- private:
-  RawEnumType set_;
 };
 
-static_assert(!KeywordModifierSet(KeywordModifierSet::Access)
-                      .HasAnyOf(KeywordModifierSet::Extern) &&
-                  !KeywordModifierSet(KeywordModifierSet::Access |
-                                      KeywordModifierSet::Extern |
-                                      KeywordModifierSet::Extend)
-                       .HasAnyOf(KeywordModifierSet::Decl),
-              "Order-related sets must not overlap");
-static_assert(~KeywordModifierSet::None ==
-                  (KeywordModifierSet::Access | KeywordModifierSet::Extern |
-                   KeywordModifierSet::Extend | KeywordModifierSet::Decl),
-              "Modifier missing from all modifier sets");
+#define CARBON_KEYWORD_MODIFIER_SET_WITH_TYPE(X) \
+  CARBON_ENUM_MASK_CONSTANT_DEFINITION(KeywordModifierSet, X)
+CARBON_KEYWORD_MODIFIER_SET(CARBON_KEYWORD_MODIFIER_SET_WITH_TYPE)
+#undef CARBON_KEYWORD_MODIFIER_SET_WITH_TYPE
+
+constexpr KeywordModifierSet KeywordModifierSet::Access(Private | Protected);
+constexpr KeywordModifierSet KeywordModifierSet::Class(Abstract | Base);
+constexpr KeywordModifierSet KeywordModifierSet::Method(Abstract | Impl |
+                                                        Virtual);
+constexpr KeywordModifierSet KeywordModifierSet::ImplDecl(Extend | Final);
+constexpr KeywordModifierSet KeywordModifierSet::Interface(Default | Final);
+constexpr KeywordModifierSet KeywordModifierSet::Decl(Class | Method |
+                                                      Interface | Export |
+                                                      Returned);
+
+static_assert(
+    !KeywordModifierSet::Access.HasAnyOf(KeywordModifierSet::Extern) &&
+        !(KeywordModifierSet::Access | KeywordModifierSet::Extern |
+          KeywordModifierSet::Extend)
+             .HasAnyOf(KeywordModifierSet::Decl),
+    "Order-related sets must not overlap");
+
+#define CARBON_KEYWORD_MODIFIER_SET_IN_GROUP(Modifier)                     \
+  static_assert((KeywordModifierSet::Access | KeywordModifierSet::Extern | \
+                 KeywordModifierSet::Extend | KeywordModifierSet::Decl)    \
+                    .HasAnyOf(KeywordModifierSet::Modifier),               \
+                "Modifier missing from all modifier sets: " #Modifier);
+CARBON_KEYWORD_MODIFIER_SET(CARBON_KEYWORD_MODIFIER_SET_IN_GROUP)
+#undef CARBON_KEYWORD_MODIFIER_SET_IN_GROUP
 
 }  // namespace Carbon::Check
 

+ 6 - 6
toolchain/check/type.cpp

@@ -138,19 +138,19 @@ auto GetConstType(Context& context, SemIR::TypeInstId inner_type_id)
 
 auto GetQualifiedType(Context& context, SemIR::TypeId type_id,
                       SemIR::TypeQualifiers quals) -> SemIR::TypeId {
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::Const)) {
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::Const)) {
     type_id = GetConstType(context, context.types().GetInstId(type_id));
-    quals &= ~SemIR::TypeQualifiers::Const;
+    quals.Remove(SemIR::TypeQualifiers::Const);
   }
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::MaybeUnformed)) {
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::MaybeUnformed)) {
     type_id = GetTypeImpl<SemIR::MaybeUnformedType>(
         context, context.types().GetInstId(type_id));
-    quals &= ~SemIR::TypeQualifiers::MaybeUnformed;
+    quals.Remove(SemIR::TypeQualifiers::MaybeUnformed);
   }
-  if (HasTypeQualifier(quals, SemIR::TypeQualifiers::Partial)) {
+  if (quals.HasAnyOf(SemIR::TypeQualifiers::Partial)) {
     type_id = GetTypeImpl<SemIR::PartialType>(
         context, context.types().GetInstId(type_id));
-    quals &= ~SemIR::TypeQualifiers::Partial;
+    quals.Remove(SemIR::TypeQualifiers::Partial);
   }
   CARBON_CHECK(quals == SemIR::TypeQualifiers::None);
   return type_id;

+ 1 - 0
toolchain/parse/BUILD

@@ -23,6 +23,7 @@ cc_library(
     srcs = ["node_category.cpp"],
     hdrs = ["node_category.h"],
     deps = [
+        "//common:enum_mask_base",
         "//common:ostream",
         "@llvm-project//llvm:Support",
     ],

+ 2 - 25
toolchain/parse/node_category.cpp

@@ -8,30 +8,7 @@
 
 namespace Carbon::Parse {
 
-// Returns a string form of the node category for printing.
-static auto NodeCategoryToString(NodeCategory::RawEnumType category)
-    -> llvm::StringLiteral {
-#define CARBON_NODE_CATEGORY_TO_STRING(Name) \
-  case NodeCategory::Name:                   \
-    return #Name;
-
-  switch (category) {
-    CARBON_NODE_CATEGORY(CARBON_NODE_CATEGORY_TO_STRING)
-    CARBON_NODE_CATEGORY_TO_STRING(None)
-  }
-
-#undef CARBON_NODE_CATEGORY_TO_STRING
-}
-
-auto NodeCategory::Print(llvm::raw_ostream& out) const -> void {
-  llvm::ListSeparator sep("|");
-  auto value = value_;
-  do {
-    // The lowest set bit in the value, or 0 (`None`) if no bits are set.
-    auto lowest_bit = static_cast<RawEnumType>(value & -value);
-    out << sep << NodeCategoryToString(lowest_bit);
-    value &= ~lowest_bit;
-  } while (value);
-}
+CARBON_DEFINE_ENUM_MASK_NAMES(NodeCategory) = {
+    CARBON_NODE_CATEGORY(CARBON_ENUM_MASK_NAME_STRING)};
 
 }  // namespace Carbon::Parse

+ 17 - 50
toolchain/parse/node_category.h

@@ -5,13 +5,11 @@
 #ifndef CARBON_TOOLCHAIN_PARSE_NODE_CATEGORY_H_
 #define CARBON_TOOLCHAIN_PARSE_NODE_CATEGORY_H_
 
+#include "common/enum_mask_base.h"
 #include "common/ostream.h"
-#include "llvm/ADT/BitmaskEnum.h"
 
 namespace Carbon::Parse {
 
-LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
-
 // An X-macro for node categories. Uses should look like:
 //
 //   #define CARBON_NODE_CATEGORY_FOR_XYZ(Name) ...
@@ -31,59 +29,28 @@ LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
   X(Requirement)                \
   X(Statement)
 
-// Represents a set of keyword modifiers, using a separate bit per modifier.
-class NodeCategory : public Printable<NodeCategory> {
- private:
-  // Use an enum to get incremental bit shifts.
-  enum class BitShift : uint8_t {
-#define CARBON_NODE_CATEGORY_FOR_BIT_SHIFT(Name) Name,
-    CARBON_NODE_CATEGORY(CARBON_NODE_CATEGORY_FOR_BIT_SHIFT)
-#undef CARBON_NODE_CATEGORY_FOR_BIT_SHIFT
-
-    // For `LLVM_MARK_AS_BITMASK_ENUM`.
-    LargestValueMarker,
-  };
+// We expect this to grow, so are using a bigger size than needed.
+CARBON_DEFINE_RAW_ENUM_MASK(NodeCategory, uint32_t) {
+  CARBON_NODE_CATEGORY(CARBON_RAW_ENUM_MASK_ENUMERATOR)
+};
 
+// Represents a set of keyword modifiers, using a separate bit per modifier.
+class NodeCategory : public CARBON_ENUM_MASK_BASE(NodeCategory) {
  public:
-  // Provide values as an enum. This doesn't expose these as NodeCategory
-  // instances just due to the duplication of declarations that would cause.
-  //
-  // We expect this to grow, so are using a bigger size than needed.
-  enum RawEnumType : uint32_t {
-#define CARBON_NODE_CATEGORY_FOR_BIT_MASK(Name) \
-  Name = 1 << static_cast<uint8_t>(BitShift::Name),
-    CARBON_NODE_CATEGORY(CARBON_NODE_CATEGORY_FOR_BIT_MASK)
-#undef CARBON_NODE_CATEGORY_FOR_BIT_MASK
-    // If you add a new category here, also add it to the Print function.
-    None = 0,
-
-    LLVM_MARK_AS_BITMASK_ENUM(
-        /*LargestValue=*/1
-        << (static_cast<uint8_t>(BitShift::LargestValueMarker) - 1))
-  };
-
-  // Support implicit conversion so that the difference with the member enum is
-  // opaque.
-  explicit(false) constexpr NodeCategory(RawEnumType value) : value_(value) {}
+  CARBON_NODE_CATEGORY(CARBON_ENUM_MASK_CONSTANT_DECL)
 
-  // Returns true if there's a non-empty set intersection.
-  constexpr auto HasAnyOf(NodeCategory other) const -> bool {
-    return value_ & other.value_;
+  using EnumMaskBase::EnumMaskBase;
+  // Provide implicit conversion because the raw enum is used in templates.
+  explicit(false) constexpr NodeCategory(RawEnumType value) {
+    *this = EnumBase::Make(value);
   }
-
-  // Returns the set inverse.
-  constexpr auto operator~() const -> NodeCategory { return ~value_; }
-
-  friend auto operator==(NodeCategory lhs, NodeCategory rhs) -> bool {
-    return lhs.value_ == rhs.value_;
-  }
-
-  auto Print(llvm::raw_ostream& out) const -> void;
-
- private:
-  RawEnumType value_;
 };
 
+#define CARBON_NODE_CATEGORY_WITH_TYPE(X) \
+  CARBON_ENUM_MASK_CONSTANT_DEFINITION(NodeCategory, X)
+CARBON_NODE_CATEGORY(CARBON_NODE_CATEGORY_WITH_TYPE)
+#undef CARBON_NODE_CATEGORY_WITH_TYPE
+
 }  // namespace Carbon::Parse
 
 #endif  // CARBON_TOOLCHAIN_PARSE_NODE_CATEGORY_H_

+ 7 - 4
toolchain/sem_ir/type.cpp

@@ -10,6 +10,9 @@
 
 namespace Carbon::SemIR {
 
+CARBON_DEFINE_ENUM_MASK_NAMES(TypeQualifiers) = {
+    CARBON_TYPE_QUALIFIERS(CARBON_ENUM_MASK_NAME_STRING)};
+
 // Verify that the constant value's type is `TypeType` (or an error).
 static void CheckTypeOfConstantIsTypeType(File& file, ConstantId constant_id) {
   CARBON_CHECK(constant_id.is_constant(),
@@ -101,13 +104,13 @@ auto TypeStore::GetUnqualifiedTypeAndQualifiers(TypeId type_id) const
       type_id = file_->types().GetTypeIdForTypeInstId(qualified_type->inner_id);
       switch (qualified_type->kind) {
         case ConstType::Kind:
-          quals |= TypeQualifiers::Const;
+          quals.Add(TypeQualifiers::Const);
           break;
         case MaybeUnformedType::Kind:
-          quals |= TypeQualifiers::MaybeUnformed;
+          quals.Add(TypeQualifiers::MaybeUnformed);
           break;
         case PartialType::Kind:
-          quals |= TypeQualifiers::Partial;
+          quals.Add(TypeQualifiers::Partial);
           break;
         default:
           CARBON_FATAL("Unknown type qualifier {0}", qualified_type->kind);
@@ -129,7 +132,7 @@ auto TypeStore::GetTransitiveUnqualifiedAdaptedType(TypeId type_id) const
       return {type_id, quals};
     }
     type_id = unqual_type_id;
-    quals |= inner_quals;
+    quals.Add(inner_quals);
   }
 }
 

+ 15 - 14
toolchain/sem_ir/type.h

@@ -5,7 +5,6 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_TYPE_H_
 #define CARBON_TOOLCHAIN_SEM_IR_TYPE_H_
 
-#include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/STLExtras.h"
 #include "toolchain/base/shared_value_stores.h"
 #include "toolchain/sem_ir/constant.h"
@@ -15,23 +14,25 @@
 
 namespace Carbon::SemIR {
 
-LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+#define CARBON_TYPE_QUALIFIERS(X) \
+  X(Const)                        \
+  X(MaybeUnformed)                \
+  X(Partial)
 
-// A bitmask of type qualifiers.
-enum class TypeQualifiers {
-  None = 0,
-  Const = 1 << 0,
-  MaybeUnformed = 1 << 1,
-  Partial = 1 << 2,
+CARBON_DEFINE_RAW_ENUM_MASK(TypeQualifiers, uint8_t) {
+  CARBON_TYPE_QUALIFIERS(CARBON_RAW_ENUM_MASK_ENUMERATOR)
+};
 
-  LLVM_MARK_AS_BITMASK_ENUM(Partial)
+// Represents a set of keyword modifiers, using a separate bit per modifier.
+class TypeQualifiers : public CARBON_ENUM_MASK_BASE(TypeQualifiers) {
+ public:
+  CARBON_TYPE_QUALIFIERS(CARBON_ENUM_MASK_CONSTANT_DECL)
 };
 
-// Returns whether the type qualifier set `quals` contains `qual`.
-inline auto HasTypeQualifier(TypeQualifiers quals, TypeQualifiers qual)
-    -> bool {
-  return (quals & qual) != TypeQualifiers::None;
-}
+#define CARBON_TYPE_QUALIFIERS_WITH_TYPE(X) \
+  CARBON_ENUM_MASK_CONSTANT_DEFINITION(TypeQualifiers, X)
+CARBON_TYPE_QUALIFIERS(CARBON_TYPE_QUALIFIERS_WITH_TYPE)
+#undef CARBON_TYPE_QUALIFIERS_WITH_TYPE
 
 // Provides a ValueStore wrapper with an API specific to types.
 class TypeStore : public Yaml::Printable<TypeStore> {