Modify EnumBase to better support the namespacing of toolchain (#3156)

The different approach to Names avoids the issues with trying to define
a static member (or also member function) of the templated instance of
Carbon::Internal::EnumBase from a non-enclosing namespace such as
Carbon::SemIR.

Note, I'm trying to do this from the cpp file. An alternative might be
to do `inline constexpr llvm::StringLiteral Names[]` in the .h file, but
I think concerns had been raised about that needing deduplication.

.clang-format

@@ -13,5 +13,6 @@ FixNamespaceComments: 'true'
 PointerAlignment: Left
 # We abuse control macros for formatting other kinds of macros.
 SpaceBeforeParens: ControlStatementsExceptControlMacros
-IfMacros: ['CARBON_DEFINE_RAW_ENUM_CLASS']
+IfMacros:
+  ['CARBON_DEFINE_RAW_ENUM_CLASS', 'CARBON_DEFINE_RAW_ENUM_CLASS_NO_NAMES']
 StatementMacros: ['ABSTRACT']

common/enum_base.h

@@ -51,7 +51,7 @@ namespace Carbon::Internal {
 //   #include ".../my_kind.def"
 //   };
 //   ```
-template <typename DerivedT, typename EnumT>
+template <typename DerivedT, typename EnumT, const llvm::StringLiteral Names[]>
 class EnumBase {
  public:
   // An alias for the raw enum type. This is an implementation detail and
@@ -78,12 +78,10 @@ class EnumBase {
   // 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.
-  [[nodiscard]] auto name() const -> llvm::StringRef;
+  [[nodiscard]] auto name() const -> llvm::StringRef { return Names[AsInt()]; }
 
   // Prints this value using its name.
-  void Print(llvm::raw_ostream& out) const {
-    out << static_cast<const EnumType*>(this)->name();
-  }
+  auto Print(llvm::raw_ostream& out) const -> void { out << name(); }
 
  protected:
   // The default constructor is explicitly defaulted (and constexpr) as a
@@ -112,22 +110,27 @@ class EnumBase {
   }
 
  private:
-  static llvm::StringLiteral names[];
-
   RawEnumType value_;
 };
 
 }  // namespace Carbon::Internal
 
+// For use when multiple enums use the same list of names.
+#define CARBON_DEFINE_RAW_ENUM_CLASS_NO_NAMES(EnumClassName, UnderlyingType) \
+  namespace Internal {                                                       \
+  /* NOLINTNEXTLINE(bugprone-macro-parentheses) */                           \
+  enum class EnumClassName##RawEnum : UnderlyingType;                        \
+  }                                                                          \
+  enum class Internal::EnumClassName##RawEnum : UnderlyingType
+
 // 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_CLASS(EnumClassName, UnderlyingType) \
   namespace Internal {                                              \
-  /* NOLINTNEXTLINE(bugprone-macro-parentheses) */                  \
-  enum class EnumClassName##RawEnum : UnderlyingType;               \
+  extern const llvm::StringLiteral EnumClassName##Names[];          \
   }                                                                 \
-  enum class ::Carbon::Internal::EnumClassName##RawEnum : UnderlyingType
+  CARBON_DEFINE_RAW_ENUM_CLASS_NO_NAMES(EnumClassName, UnderlyingType)
 
 // In CARBON_DEFINE_RAW_ENUM_CLASS block, use this to generate each enumerator.
 #define CARBON_RAW_ENUM_ENUMERATOR(Name) Name,
@@ -136,12 +139,14 @@ class EnumBase {
 // class. It both computes the name of the raw enum and ensures all the
 // namespaces are correct.
 #define CARBON_ENUM_BASE(EnumClassName) \
-  CARBON_ENUM_BASE_CRTP(EnumClassName, EnumClassName)
+  CARBON_ENUM_BASE_CRTP(EnumClassName, EnumClassName, EnumClassName)
 // This variant handles the case where the external name for the Carbon enum is
 // not the same as the name by which we refer to it from this context.
-#define CARBON_ENUM_BASE_CRTP(EnumClassName, LocalTypeNameForEnumClass) \
+#define CARBON_ENUM_BASE_CRTP(EnumClassName, LocalTypeNameForEnumClass, \
+                              EnumClassNameForNames)                    \
   ::Carbon::Internal::EnumBase<LocalTypeNameForEnumClass,               \
-                               ::Carbon::Internal::EnumClassName##RawEnum>
+                               Internal::EnumClassName##RawEnum,        \
+                               Internal::EnumClassNameForNames##Names>
 
 // Use this within the Carbon enum class body to generate named constant
 // declarations for each value.
@@ -163,43 +168,14 @@ class EnumBase {
   static constexpr const typename Base::EnumType& Name = \
       Base::Create(Base::RawEnumType::Name);
 
-// Use this to define a custom `name()` function for an enum-like class. Usage:
-//
-//   CARBON_ENUM_NAME_FUNCTION(MyEnum) {
-//     // Return a StringRef based on the value of *this.
-//   }
-//
-// You should usually use CARBON_DEFINE_ENUM_CLASS_NAMES instead.
-#define CARBON_ENUM_NAME_FUNCTION(EnumClassName)                              \
-  template <>                                                                 \
-  auto                                                                        \
-  Internal::EnumBase<EnumClassName, Internal::EnumClassName##RawEnum>::name() \
-      const->llvm::StringRef
-
 // Use this in the `.cpp` file for an enum class to start the definition of the
 // constant names array for each enumerator. It is followed by the desired
 // constant initializer.
 //
 // `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)                         \
-  /* First declare an explicit specialization of the names array so we can    \
-   * reference it from an explicit function specialization. */                \
-  template <>                                                                 \
-  llvm::StringLiteral Internal::EnumBase<                                     \
-      EnumClassName, Internal::EnumClassName##RawEnum>::names[];              \
-                                                                              \
-  /* Now define an explicit function specialization for the `name` method, as \
-   * it can now reference our specialized array. */                           \
-  CARBON_ENUM_NAME_FUNCTION(EnumClassName) {                                  \
-    return names[static_cast<int>(value_)];                                   \
-  }                                                                           \
-                                                                              \
-  /* Finally, open up the definition of our specialized array for the user to \
-   * populate using the x-macro include. */                                   \
-  template <>                                                                 \
-  llvm::StringLiteral Internal::EnumBase<                                     \
-      EnumClassName, Internal::EnumClassName##RawEnum>::names[]
+#define CARBON_DEFINE_ENUM_CLASS_NAMES(EnumClassName) \
+  constexpr llvm::StringLiteral Internal::EnumClassName##Names[]
 
 // Use this within the names array initializer to generate a string for each
 // name.

explorer/ast/ast_rtti.cpp

@@ -15,6 +15,5 @@ CARBON_DEFINE_ENUM_CLASS_NAMES(AstRttiNodeKind) = {
   CARBON_ENUM_NAME_FUNCTION(C##Kind) {                                 \
     return AstRttiNodeKind(static_cast<const C##Kind&>(*this)).name(); \
   }
-CARBON_AST_FOR_EACH_ABSTRACT_CLASS(DEFINE_NAME_FUNCTION)
 
 }  // namespace Carbon

explorer/ast/ast_rtti.h

@@ -46,13 +46,14 @@ CARBON_AST_FOR_EACH_FINAL_CLASS(CONSTANT_DEFINITION)
 
 // Define Kind enumerations for all base classes.
 #define DEFINE_KIND_ENUM(C)                                                 \
-  CARBON_DEFINE_RAW_ENUM_CLASS(C##Kind, int) {                              \
+  CARBON_DEFINE_RAW_ENUM_CLASS_NO_NAMES(C##Kind, int) {                     \
     CARBON_AST_FOR_EACH_FINAL_CLASS_BELOW(C, DEFINE_ENUMERATOR)             \
   };                                                                        \
   template <typename Derived>                                               \
-  class C##KindTemplate : public CARBON_ENUM_BASE_CRTP(C##Kind, Derived) {  \
+  class C##KindTemplate                                                     \
+      : public CARBON_ENUM_BASE_CRTP(C##Kind, Derived, AstRttiNodeKind) {   \
    private:                                                                 \
-    using Base = CARBON_ENUM_BASE_CRTP(C##Kind, Derived);                   \
+    using Base = CARBON_ENUM_BASE_CRTP(C##Kind, Derived, AstRttiNodeKind);  \
     friend class AstRttiNodeKind;                                           \
                                                                             \
    public:                                                                  \

toolchain/semantics/semantics_builtin_kind.cpp

@@ -4,15 +4,15 @@
 
 #include "toolchain/semantics/semantics_builtin_kind.h"
 
-namespace Carbon {
+namespace Carbon::SemIR {
 
-CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsBuiltinKind) = {
+CARBON_DEFINE_ENUM_CLASS_NAMES(BuiltinKind) = {
 #define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \
   CARBON_ENUM_CLASS_NAME_STRING(Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
 };
 
-auto SemanticsBuiltinKind::label() -> llvm::StringRef {
+auto BuiltinKind::label() -> llvm::StringRef {
   static constexpr llvm::StringLiteral Labels[] = {
 #define CARBON_SEMANTICS_BUILTIN_KIND(Name, Label) Label,
 #include "toolchain/semantics/semantics_builtin_kind.def"
@@ -20,4 +20,4 @@ auto SemanticsBuiltinKind::label() -> llvm::StringRef {
   return Labels[AsInt()];
 }
 
-}  // namespace Carbon
+}  // namespace Carbon::SemIR

toolchain/semantics/semantics_builtin_kind.h

@@ -9,15 +9,15 @@
 
 #include "common/enum_base.h"
 
-namespace Carbon {
+namespace Carbon::SemIR {
 
-CARBON_DEFINE_RAW_ENUM_CLASS(SemanticsBuiltinKind, uint8_t) {
+CARBON_DEFINE_RAW_ENUM_CLASS(BuiltinKind, uint8_t) {
 #define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \
   CARBON_RAW_ENUM_ENUMERATOR(Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
 };
 
-class SemanticsBuiltinKind : public CARBON_ENUM_BASE(SemanticsBuiltinKind) {
+class BuiltinKind : public CARBON_ENUM_BASE(BuiltinKind) {
  public:
 #define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \
   CARBON_ENUM_CONSTANT_DECLARATION(Name)
@@ -37,26 +37,20 @@ class SemanticsBuiltinKind : public CARBON_ENUM_BASE(SemanticsBuiltinKind) {
 };
 
 #define CARBON_SEMANTICS_BUILTIN_KIND_NAME(Name) \
-  CARBON_ENUM_CONSTANT_DEFINITION(SemanticsBuiltinKind, Name)
+  CARBON_ENUM_CONSTANT_DEFINITION(BuiltinKind, Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
 
-constexpr uint8_t SemanticsBuiltinKind::ValidCount = Invalid.AsInt();
+constexpr uint8_t BuiltinKind::ValidCount = Invalid.AsInt();
 
 static_assert(
-    SemanticsBuiltinKind::ValidCount != 0,
+    BuiltinKind::ValidCount != 0,
     "The above `constexpr` definition of `ValidCount` makes it available in "
     "a `constexpr` context despite being declared as merely `const`. We use it "
     "in a static assert here to ensure that.");
 
 // We expect the builtin kind to fit compactly into 8 bits.
-static_assert(sizeof(SemanticsBuiltinKind) == 1,
-              "Kind objects include padding!");
+static_assert(sizeof(BuiltinKind) == 1, "Kind objects include padding!");
 
-// TODO: Refactor EnumBase to remove the need for this alias.
-namespace SemIR {
-using BuiltinKind = SemanticsBuiltinKind;
-}  // namespace SemIR
-
-}  // namespace Carbon
+}  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_BUILTIN_KIND_H_

toolchain/semantics/semantics_node.cpp

@@ -20,23 +20,21 @@ static auto PrintArgs(llvm::raw_ostream& out, std::pair<T0, T1> args) -> void {
   out << ", arg1: " << args.second;
 }
 
-auto operator<<(llvm::raw_ostream& out, const Node& node)
-    -> llvm::raw_ostream& {
-  out << "{kind: " << node.kind_;
+auto Node::Print(llvm::raw_ostream& out) const -> void {
+  out << "{kind: " << kind_;
   // clang warns on unhandled enum values; clang-tidy is incorrect here.
   // NOLINTNEXTLINE(bugprone-switch-missing-default-case)
-  switch (node.kind_) {
+  switch (kind_) {
 #define CARBON_SEMANTICS_NODE_KIND(Name) \
   case NodeKind::Name:                   \
-    PrintArgs(out, node.GetAs##Name());  \
+    PrintArgs(out, GetAs##Name());       \
     break;
 #include "toolchain/semantics/semantics_node_kind.def"
   }
-  if (node.type_id_.is_valid()) {
-    out << ", type: " << node.type_id_;
+  if (type_id_.is_valid()) {
+    out << ", type: " << type_id_;
   }
   out << "}";
-  return out;
 }
 
 }  // namespace Carbon::SemIR

toolchain/semantics/semantics_node.h

@@ -233,13 +233,10 @@ class Node {
   // Factory base classes are private, then used for public classes. This class
   // has two public and two private sections to prevent accidents.
  private:
-  // Factory templates need to use the raw enum instead of the class wrapper.
-  using KindTemplateEnum = Internal::SemanticsNodeKindRawEnum;
-
   // Provides Make and Get to support 0, 1, or 2 arguments for a Node.
   // These are protected so that child factories can opt in to what pieces they
   // want to use.
-  template <KindTemplateEnum Kind, typename... ArgTypes>
+  template <NodeKind::RawEnumType Kind, typename... ArgTypes>
   class FactoryBase {
    protected:
     static auto Make(ParseTree::Node parse_node, TypeId type_id,
@@ -273,7 +270,7 @@ class Node {
   };
 
   // Provide Get along with a Make that requires a type.
-  template <KindTemplateEnum Kind, typename... ArgTypes>
+  template <NodeKind::RawEnumType Kind, typename... ArgTypes>
   class Factory : public FactoryBase<Kind, ArgTypes...> {
    public:
     using FactoryBase<Kind, ArgTypes...>::Make;
@@ -282,7 +279,7 @@ class Node {
 
   // Provides Get along with a Make that assumes the node doesn't produce a
   // typed value.
-  template <KindTemplateEnum Kind, typename... ArgTypes>
+  template <NodeKind::RawEnumType Kind, typename... ArgTypes>
   class FactoryNoType : public FactoryBase<Kind, ArgTypes...> {
    public:
     static auto Make(ParseTree::Node parse_node, ArgTypes... args) {
@@ -436,9 +433,7 @@ class Node {
   // Gets the type of the value produced by evaluating this node.
   auto type_id() const -> TypeId { return type_id_; }
 
-  friend auto operator<<(llvm::raw_ostream& out, const Node& node)
-      -> llvm::raw_ostream&;
-  LLVM_DUMP_METHOD void Dump() const { llvm::errs() << *this; }
+  auto Print(llvm::raw_ostream& out) const -> void;
 
  private:
   // Builtins have peculiar construction, so they are a friend rather than using

toolchain/semantics/semantics_node_kind.cpp

@@ -4,15 +4,15 @@
 
 #include "toolchain/semantics/semantics_node_kind.h"
 
-namespace Carbon {
+namespace Carbon::SemIR {
 
-CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsNodeKind) = {
+CARBON_DEFINE_ENUM_CLASS_NAMES(NodeKind) = {
 #define CARBON_SEMANTICS_NODE_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
 #include "toolchain/semantics/semantics_node_kind.def"
 };
 
 // Returns the name to use for this node kind in Semantics IR.
-[[nodiscard]] auto SemanticsNodeKind::ir_name() const -> llvm::StringRef {
+[[nodiscard]] auto NodeKind::ir_name() const -> llvm::StringRef {
   static constexpr llvm::StringRef Table[] = {
 #define CARBON_SEMANTICS_NODE_KIND_WITH_IR_NAME(Name, IR_Name) IR_Name,
 #include "toolchain/semantics/semantics_node_kind.def"
@@ -20,22 +20,22 @@ CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsNodeKind) = {
   return Table[AsInt()];
 }
 
-auto SemanticsNodeKind::value_kind() const -> SemIR::NodeValueKind {
-  static constexpr SemIR::NodeValueKind Table[] = {
+auto NodeKind::value_kind() const -> NodeValueKind {
+  static constexpr NodeValueKind Table[] = {
 #define CARBON_SEMANTICS_NODE_KIND_WITH_VALUE_KIND(Name, Kind) \
-  SemIR::NodeValueKind::Kind,
+  NodeValueKind::Kind,
 #include "toolchain/semantics/semantics_node_kind.def"
   };
   return Table[AsInt()];
 }
 
-auto SemanticsNodeKind::terminator_kind() const -> SemIR::TerminatorKind {
-  static constexpr SemIR::TerminatorKind Table[] = {
+auto NodeKind::terminator_kind() const -> TerminatorKind {
+  static constexpr TerminatorKind Table[] = {
 #define CARBON_SEMANTICS_NODE_KIND_WITH_TERMINATOR_KIND(Name, Kind) \
-  SemIR::TerminatorKind::Kind,
+  TerminatorKind::Kind,
 #include "toolchain/semantics/semantics_node_kind.def"
   };
   return Table[AsInt()];
 }
 
-}  // namespace Carbon
+}  // namespace Carbon::SemIR

toolchain/semantics/semantics_node_kind.h

@@ -38,16 +38,12 @@ enum class TerminatorKind : int8_t {
   Terminator,
 };
 
-}  // namespace Carbon::SemIR
-
-namespace Carbon {
-
-CARBON_DEFINE_RAW_ENUM_CLASS(SemanticsNodeKind, uint8_t) {
+CARBON_DEFINE_RAW_ENUM_CLASS(NodeKind, uint8_t) {
 #define CARBON_SEMANTICS_NODE_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name)
 #include "toolchain/semantics/semantics_node_kind.def"
 };
 
-class SemanticsNodeKind : public CARBON_ENUM_BASE(SemanticsNodeKind) {
+class NodeKind : public CARBON_ENUM_BASE(NodeKind) {
  public:
 #define CARBON_SEMANTICS_NODE_KIND(Name) CARBON_ENUM_CONSTANT_DECLARATION(Name)
 #include "toolchain/semantics/semantics_node_kind.def"
@@ -58,14 +54,14 @@ class SemanticsNodeKind : public CARBON_ENUM_BASE(SemanticsNodeKind) {
   [[nodiscard]] auto ir_name() const -> llvm::StringRef;
 
   // Returns whether this kind of node is expected to produce a value.
-  [[nodiscard]] auto value_kind() const -> SemIR::NodeValueKind;
+  [[nodiscard]] auto value_kind() const -> NodeValueKind;
 
   // Returns whether this node kind is a code block terminator, such as an
   // unconditional branch instruction, or part of the termination sequence,
   // such as a conditional branch instruction. The termination sequence of a
   // code block appears after all other instructions, and ends with a
   // terminator instruction.
-  [[nodiscard]] auto terminator_kind() const -> SemIR::TerminatorKind;
+  [[nodiscard]] auto terminator_kind() const -> TerminatorKind;
 
   // Compute a fingerprint for this node kind, allowing its use as part of the
   // key in a `FoldingSet`.
@@ -73,17 +69,12 @@ class SemanticsNodeKind : public CARBON_ENUM_BASE(SemanticsNodeKind) {
 };
 
 #define CARBON_SEMANTICS_NODE_KIND(Name) \
-  CARBON_ENUM_CONSTANT_DEFINITION(SemanticsNodeKind, Name)
+  CARBON_ENUM_CONSTANT_DEFINITION(NodeKind, Name)
 #include "toolchain/semantics/semantics_node_kind.def"
 
 // We expect the node kind to fit compactly into 8 bits.
-static_assert(sizeof(SemanticsNodeKind) == 1, "Kind objects include padding!");
+static_assert(sizeof(NodeKind) == 1, "Kind objects include padding!");
 
-// TODO: Refactor EnumBase to remove the need for this alias.
-namespace SemIR {
-using NodeKind = SemanticsNodeKind;
-}  // namespace SemIR
-
-}  // namespace Carbon
+}  // namespace Carbon::SemIR
 
 #endif  // CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_NODE_KIND_H_