Эх сурвалжийг харах

Introduce helpers to build enum-wrapping classes. (#2504)

The goal here is to (significantly) reduce the boilerplate needed when defining classes that wrap enums, especially those managed with the `.def`-file style X-macros that are common in the toolchain.

This should also provide both better and more consistent functionality to those classes once ported over to it.

Initially, only `ParserState`, `SemanticsNodeKind`, and `SemanticsBuiltinKind` are ported as these were also the three that JonMeow ported in his original pull/2453 "option 5". This is heavily based on that version of the code.

Goals I was considering that influenced the design:

- Keep the individual enum-wrapping classes as simple and easy to read as possible. Especially important is keeping the `.def` files that are often filled with really important documentation clean and easy to maintain over time.

- Don't rely on computed `#include`s as that is an especially dark corner of the preprocessor and breaks some build systems.

- Have a really good API of the enum-wrapping class, including nice constant names for the values, easy printing, and even easy debugger-callable methods to get the name (as opposed to the integer value).

- Keep the API that users interact with in the base class as clean and easy to read as possible.

- Reduce the boiler plate for each instance of these as much as possible.

- Avoid excessive inline generated code or constants that would result in steady growth in object file sizes and linker effort doing deduplication.

These goals aren't always compatible, so we end up needing to pick a compromise between them when in tension. I think this version is a pretty good compromise.

The original version I started with already pull most of the API into a CRTP-style base class. This version pulls *all* of the common API. This is the main tool for getting consistency and avoiding duplication. However, connecting this base class to the individual enum wrappers is still difficult. Some specific changes here that try to do as much as possible there:

- Use a slightly fancier macro pattern to reduce the boilerplate of defining the raw `enum class` prior to the wrapper class.

- Use a macro to simplify naming the base class.

- Move the name table to a `.cpp` file to avoid every inclusion generating a complete copy of the strings (that the linker has to deduplicate). This is done with some care to sharply reduce the boilerplate needed in that `.cpp` file.

- Sink the name _API_ fully into the CRTP base class. This requires some significant complexity in the implementation, but all of that is hidden behind a single implementation detail macro, and the API itself is simple and readable. This also makes it much more reasonable to test the entire system a single time next to the base class.

This version also moves from constant factory functions to normal constants. This requires two batches -- first a declaration, and then a definition -- but the API result is significantly better and similar to the original option, the macro structure reduces the cost of these. Unfortunately that makes the adoption a bit noisy, but I think its worth the churn.

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Chandler Carruth 3 жил өмнө
parent
commit
a1ad39fa29

+ 25 - 0
toolchain/common/BUILD

@@ -4,6 +4,31 @@
 
 package(default_visibility = ["//visibility:public"])
 
+cc_library(
+    name = "enum_base",
+    hdrs = ["enum_base.h"],
+    deps = [
+        "//common:ostream",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_library(
+    name = "enum_base_test_def",
+    textual_hdrs = ["enum_base_test.def"],
+)
+
+cc_test(
+    name = "enum_base_test",
+    srcs = ["enum_base_test.cpp"],
+    deps = [
+        ":enum_base",
+        ":enum_base_test_def",
+        "//common:gtest_main",
+        "@com_google_googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "index_base",
     hdrs = ["index_base.h"],

+ 185 - 0
toolchain/common/enum_base.h

@@ -0,0 +1,185 @@
+// 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_COMMON_ENUM_BASE_H_
+#define CARBON_TOOLCHAIN_COMMON_ENUM_BASE_H_
+
+#include <cstdint>
+#include <type_traits>
+
+#include "common/ostream.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace Carbon::Internal {
+
+// CRTP-style base class used to define the common pattern of Carbon enum-like
+// classes. The result is a class with named constants similar to enumerators,
+// but that are normal classes, can contain other methods, and support a `name`
+// method and printing the enums. These even work in switch statements and
+// support `case MyEnum::Name:`.
+//
+// It is specifically designed to compose with X-MACRO style `.def` files that
+// stamp out all the enumerators.
+//
+// It also supports some opt-in APIs that classes can enable by `using` the
+// names to make them public: `AsInt` and `FromInt` allow converting to and from
+// the underlying type of the enumerator.
+//
+// Users must be in the `Carbon` namespace and should look like the following.
+//
+// In `my_kind.h`:
+//   ```
+//   CARBON_DEFINE_RAW_ENUM_CLASS(MyKind, uint8_t){
+//   #define CARBON_MY_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name)
+//   #include "toolchain/.../my_kind.def"
+//   };
+//
+//   class MyKind : public CARBON_ENUM_BASE(MyKind) {
+//    public:
+//   #define CARBON_MY_KIND(Name) CARBON_ENUM_CONSTANT_DECLARATION(Name)
+//   #include "toolchain/.../my_kind.def"
+//   };
+//
+//   #define CARBON_MY_KIND(Name) CARBON_ENUM_CONSTANT_DEFINITION(MyKind, Name)
+//   #include "toolchain/.../my_kind.def"
+//   ```
+//
+// In `my_kind.cpp`:
+//   ```
+//   CARBON_DEFINE_ENUM_CLASS_NAMES(MyKind) = {
+//   #define CARBON_MY_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
+//   #include "toolchain/.../my_kind.def"
+//   };
+//   ```
+template <typename DerivedT, typename EnumT>
+class EnumBase {
+ protected:
+  // An alias for the raw enum type. This is an implementation detail and
+  // shouldn't be used, but we need it for a signature so it is declared early.
+  using RawEnumType = EnumT;
+
+ public:
+  using EnumType = DerivedT;
+  using UnderlyingType = std::underlying_type_t<RawEnumType>;
+
+  // Enable conversion to the raw enum type, including in a `constexpr` context,
+  // to enable comparisons and usage in `switch` and `case`. The enum type
+  // remains an implementation detail and nothing else should be using this
+  // function.
+  //
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  constexpr operator RawEnumType() const { return value_; }
+
+  // Conversion to bool is deleted to prevent direct use in an `if` condition
+  // instead of comparing with another value.
+  explicit operator bool() const = delete;
+
+  // Returns the name of this value.
+  //
+  // 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;
+
+  // Prints this value using its name.
+  void Print(llvm::raw_ostream& out) const {
+    out << reinterpret_cast<const EnumType*>(this)->name();
+  }
+
+ protected:
+  // The default constructor is explicitly defaulted (and constexpr) as a
+  // protected constructor to allow derived classes to be constructed but not
+  // the base itself. This should only be used in the `Create` function below.
+  constexpr EnumBase() = default;
+
+  // Create an instance from the raw enumerator, for internal use.
+  static constexpr auto Create(RawEnumType value) -> EnumType {
+    EnumType result;
+    result.value_ = value;
+    return result;
+  }
+
+  // Convert to the underlying integer type. Derived types can choose to expose
+  // this as part of their API.
+  constexpr auto AsInt() const -> UnderlyingType {
+    return static_cast<UnderlyingType>(value_);
+  }
+
+  // Convert from the underlying integer type. Derived types can choose to
+  // expose this as part of their API.
+  static constexpr auto FromInt(UnderlyingType value) -> EnumType {
+    return Create(static_cast<RawEnumType>(value));
+  }
+
+ private:
+  static llvm::StringLiteral names[];
+
+  RawEnumType value_;
+};
+
+}  // 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_CLASS(EnumClassName, UnderlyingType) \
+  namespace Internal {                                              \
+  /* NOLINTNEXTLINE(bugprone-macro-parentheses) */                  \
+  enum class EnumClassName##RawEnum : UnderlyingType;               \
+  }                                                                 \
+  enum class ::Carbon::Internal::EnumClassName##RawEnum : UnderlyingType
+
+// In 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, \
+                               ::Carbon::Internal::EnumClassName##RawEnum>
+
+// Use this within the Carbon enum class body to generate named constant
+// declarations for each value.
+#define CARBON_ENUM_CONSTANT_DECLARATION(Name) static const EnumType Name;
+
+// Use this immediately after the Carbon enum class body to define each named
+// constant.
+#define CARBON_ENUM_CONSTANT_DEFINITION(EnumClassName, Name) \
+  constexpr EnumClassName EnumClassName::Name =              \
+      EnumClassName::Create(RawEnumType::Name);
+
+// 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. */                           \
+  template <>                                                                 \
+  auto                                                                        \
+  Internal::EnumBase<EnumClassName, Internal::EnumClassName##RawEnum>::name() \
+      const->llvm::StringRef {                                                \
+    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[]
+
+// Use this within the names array initializer to generate a string for each
+// name.
+#define CARBON_ENUM_CLASS_NAME_STRING(Name) #Name,
+
+#endif  // CARBON_TOOLCHAIN_COMMON_ENUM_BASE_H_

+ 108 - 0
toolchain/common/enum_base_test.cpp

@@ -0,0 +1,108 @@
+// 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/common/enum_base.h"
+
+#include <gtest/gtest.h>
+
+namespace Carbon {
+
+CARBON_DEFINE_RAW_ENUM_CLASS(TestKind, uint8_t){
+#define CARBON_ENUM_BASE_TEST_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name)
+#include "toolchain/common/enum_base_test.def"
+};
+
+class TestKind : public CARBON_ENUM_BASE(TestKind) {
+ public:
+#define CARBON_ENUM_BASE_TEST_KIND(Name) CARBON_ENUM_CONSTANT_DECLARATION(Name)
+#include "toolchain/common/enum_base_test.def"
+
+  using EnumBase::AsInt;
+  using EnumBase::FromInt;
+};
+
+#define CARBON_ENUM_BASE_TEST_KIND(Name) \
+  CARBON_ENUM_CONSTANT_DEFINITION(TestKind, Name)
+#include "toolchain/common/enum_base_test.def"
+
+CARBON_DEFINE_ENUM_CLASS_NAMES(TestKind) = {
+#define CARBON_ENUM_BASE_TEST_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
+#include "toolchain/common/enum_base_test.def"
+};
+
+namespace {
+
+TEST(EnumBaseTest, NamesAndConstants) {
+  EXPECT_EQ("Beep", TestKind::Beep.name());
+  EXPECT_EQ("Boop", TestKind::Boop.name());
+  EXPECT_EQ("Burr", TestKind::Burr.name());
+}
+
+TEST(EnumBaseTest, Printing) {
+  std::string s;
+  llvm::raw_string_ostream stream(s);
+
+  TestKind kind = TestKind::Beep;
+  stream << kind << " " << TestKind::Beep;
+  kind = TestKind::Boop;
+  stream << " " << kind;
+
+  // Check the streamed results and also make sure we can stream into GoogleTest
+  // assertions.
+  EXPECT_EQ("Beep Beep Boop", s) << "Final kind: " << kind;
+}
+
+TEST(EnumBaseTest, Switch) {
+  TestKind kind = TestKind::Boop;
+
+  switch (kind) {
+    case TestKind::Beep: {
+      FAIL() << "Beep case selected!";
+      break;
+    }
+    case TestKind::Boop: {
+      EXPECT_EQ("Boop", kind.name());
+      break;
+    }
+    case TestKind::Burr: {
+      FAIL() << "Burr case selected!";
+      break;
+    }
+  }
+}
+
+TEST(EnumBaseTest, Comparison) {
+  TestKind kind = TestKind::Beep;
+
+  // Make sure all the different comparisons work, and also to work with
+  // GoogleTest expectations.
+  EXPECT_EQ(TestKind::Beep, kind);
+  EXPECT_NE(TestKind::Boop, kind);
+  EXPECT_LT(kind, TestKind::Boop);
+  EXPECT_GT(TestKind::Burr, kind);
+  EXPECT_LE(kind, TestKind::Beep);
+  EXPECT_GE(TestKind::Beep, kind);
+
+  // These should also all be constexpr.
+  constexpr TestKind Kind2 = TestKind::Beep;
+  static_assert(Kind2 == TestKind::Beep);
+  static_assert(Kind2 != TestKind::Boop);
+  static_assert(Kind2 < TestKind::Boop);
+  static_assert(!(Kind2 > TestKind::Burr));
+  static_assert(Kind2 <= TestKind::Beep);
+  static_assert(!(Kind2 >= TestKind::Burr));
+}
+
+TEST(EnumBaseTest, IntConversion) {
+  EXPECT_EQ(0, TestKind::Beep.AsInt());
+  EXPECT_EQ(1, TestKind::Boop.AsInt());
+  EXPECT_EQ(2, TestKind::Burr.AsInt());
+
+  EXPECT_EQ(TestKind::Beep, TestKind::FromInt(0));
+  EXPECT_EQ(TestKind::Boop, TestKind::FromInt(1));
+  EXPECT_EQ(TestKind::Burr, TestKind::FromInt(2));
+}
+
+}  // namespace
+}  // namespace Carbon

+ 17 - 0
toolchain/common/enum_base_test.def

@@ -0,0 +1,17 @@
+// 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
+//
+// Note that this is an X-macro header. It does not use `#include` guards, and
+// instead is designed to be `#include`ed after the x-macro is defined in order
+// for its inclusion to expand to the desired output.
+
+#ifndef CARBON_ENUM_BASE_TEST_KIND
+#error "Must define the x-macro to use this file."
+#endif
+
+CARBON_ENUM_BASE_TEST_KIND(Beep)
+CARBON_ENUM_BASE_TEST_KIND(Boop)
+CARBON_ENUM_BASE_TEST_KIND(Burr)
+
+#undef CARBON_ENUM_BASE_TEST_KIND

+ 1 - 16
toolchain/parser/BUILD

@@ -35,22 +35,7 @@ cc_library(
     srcs = ["parser_state.cpp"],
     hdrs = ["parser_state.h"],
     textual_hdrs = ["parser_state.def"],
-    deps = [
-        "//common:ostream",
-        "@llvm-project//llvm:Support",
-    ],
-)
-
-cc_test(
-    name = "parser_state_test",
-    size = "small",
-    srcs = ["parser_state_test.cpp"],
-    deps = [
-        ":parser_state",
-        "//common:gtest_main",
-        "@com_google_googletest//:gtest",
-        "@llvm-project//llvm:Support",
-    ],
+    deps = ["//toolchain/common:enum_base"],
 )
 
 cc_library(

+ 114 - 114
toolchain/parser/parser.cpp

@@ -268,10 +268,10 @@ auto Parser::SkipTo(TokenizedBuffer::Token t) -> void {
 auto Parser::HandleCodeBlockState() -> void {
   PopAndDiscardState();
 
-  PushState(ParserState::CodeBlockFinish());
+  PushState(ParserState::CodeBlockFinish);
   if (ConsumeAndAddLeafNodeIf(TokenKind::OpenCurlyBrace(),
                               ParseNodeKind::CodeBlockStart())) {
-    PushState(ParserState::StatementScopeLoop());
+    PushState(ParserState::StatementScopeLoop);
   } else {
     AddLeafNode(ParseNodeKind::CodeBlockStart(), *position_,
                 /*has_error=*/true);
@@ -280,7 +280,7 @@ auto Parser::HandleCodeBlockState() -> void {
     CARBON_DIAGNOSTIC(ExpectedCodeBlock, Error, "Expected braced code block.");
     emitter_->Emit(*position_, ExpectedCodeBlock);
 
-    PushState(ParserState::Statement());
+    PushState(ParserState::Statement);
   }
 }
 
@@ -437,11 +437,11 @@ auto Parser::Parse() -> void {
   // Traces state_stack_. This runs even in opt because it's low overhead.
   PrettyStackTraceParseState pretty_stack(this);
 
-  PushState(ParserState::DeclarationLoop());
+  PushState(ParserState::DeclarationLoop);
   while (!state_stack_.empty()) {
     switch (state_stack_.back().state) {
 #define CARBON_PARSER_STATE(Name) \
-  case ParserState::Name():       \
+  case ParserState::Name:         \
     Handle##Name##State();        \
     break;
 #include "toolchain/parser/parser_state.def"
@@ -454,14 +454,14 @@ auto Parser::Parse() -> void {
 auto Parser::HandleBraceExpressionState() -> void {
   auto state = PopState();
 
-  state.state = ParserState::BraceExpressionFinishAsUnknown();
+  state.state = ParserState::BraceExpressionFinishAsUnknown;
   PushState(state);
 
   CARBON_CHECK(ConsumeAndAddLeafNodeIf(
       TokenKind::OpenCurlyBrace(),
       ParseNodeKind::StructLiteralOrStructTypeLiteralStart()));
   if (!PositionIs(TokenKind::CloseCurlyBrace())) {
-    PushState(ParserState::BraceExpressionParameterAsUnknown());
+    PushState(ParserState::BraceExpressionParameterAsUnknown);
   }
 }
 
@@ -496,9 +496,9 @@ auto Parser::HandleBraceExpressionParameterError(StateStackEntry state,
                  can_be_value ? "`.field = value`" : "");
 
   state.state = BraceExpressionKindToParserState(
-      kind, ParserState::BraceExpressionParameterFinishAsType(),
-      ParserState::BraceExpressionParameterFinishAsValue(),
-      ParserState::BraceExpressionParameterFinishAsUnknown());
+      kind, ParserState::BraceExpressionParameterFinishAsType,
+      ParserState::BraceExpressionParameterFinishAsValue,
+      ParserState::BraceExpressionParameterFinishAsUnknown);
   state.has_error = true;
   PushState(state);
 }
@@ -512,11 +512,11 @@ auto Parser::HandleBraceExpressionParameter(BraceExpressionKind kind) -> void {
   }
 
   state.state = BraceExpressionKindToParserState(
-      kind, ParserState::BraceExpressionParameterAfterDesignatorAsType(),
-      ParserState::BraceExpressionParameterAfterDesignatorAsValue(),
-      ParserState::BraceExpressionParameterAfterDesignatorAsUnknown());
+      kind, ParserState::BraceExpressionParameterAfterDesignatorAsType,
+      ParserState::BraceExpressionParameterAfterDesignatorAsValue,
+      ParserState::BraceExpressionParameterAfterDesignatorAsUnknown);
   PushState(state);
-  PushState(ParserState::DesignatorAsStruct());
+  PushState(ParserState::DesignatorAsStruct);
 }
 
 auto Parser::HandleBraceExpressionParameterAsTypeState() -> void {
@@ -541,9 +541,9 @@ auto Parser::HandleBraceExpressionParameterAfterDesignator(
     if (!recovery_pos ||
         tokens_->GetKind(*recovery_pos) == TokenKind::Comma()) {
       state.state = BraceExpressionKindToParserState(
-          kind, ParserState::BraceExpressionParameterFinishAsType(),
-          ParserState::BraceExpressionParameterFinishAsValue(),
-          ParserState::BraceExpressionParameterFinishAsUnknown());
+          kind, ParserState::BraceExpressionParameterFinishAsType,
+          ParserState::BraceExpressionParameterFinishAsValue,
+          ParserState::BraceExpressionParameterFinishAsUnknown);
       PushState(state);
       return;
     }
@@ -569,25 +569,25 @@ auto Parser::HandleBraceExpressionParameterAfterDesignator(
     kind = elem_kind;
     auto finish_state = PopState();
     CARBON_CHECK(finish_state.state ==
-                 ParserState::BraceExpressionFinishAsUnknown());
+                 ParserState::BraceExpressionFinishAsUnknown);
     finish_state.state = BraceExpressionKindToParserState(
-        kind, ParserState::BraceExpressionFinishAsType(),
-        ParserState::BraceExpressionFinishAsValue(),
-        ParserState::BraceExpressionFinishAsUnknown());
+        kind, ParserState::BraceExpressionFinishAsType,
+        ParserState::BraceExpressionFinishAsValue,
+        ParserState::BraceExpressionFinishAsUnknown);
     PushState(finish_state);
   }
 
   state.state = BraceExpressionKindToParserState(
-      kind, ParserState::BraceExpressionParameterFinishAsType(),
-      ParserState::BraceExpressionParameterFinishAsValue(),
-      ParserState::BraceExpressionParameterFinishAsUnknown());
+      kind, ParserState::BraceExpressionParameterFinishAsType,
+      ParserState::BraceExpressionParameterFinishAsValue,
+      ParserState::BraceExpressionParameterFinishAsUnknown);
 
   state.token = Consume();
 
   // Struct type fields and value fields use the same grammar except
   // that one has a `:` separator and the other has an `=` separator.
   PushState(state);
-  PushState(ParserState::Expression());
+  PushState(ParserState::Expression);
 }
 
 auto Parser::HandleBraceExpressionParameterAfterDesignatorAsTypeState()
@@ -623,9 +623,9 @@ auto Parser::HandleBraceExpressionParameterFinish(BraceExpressionKind kind)
                        TokenKind::CloseCurlyBrace(),
                        state.has_error) == ListTokenKind::Comma) {
     PushState(BraceExpressionKindToParserState(
-        kind, ParserState::BraceExpressionParameterAsType(),
-        ParserState::BraceExpressionParameterAsValue(),
-        ParserState::BraceExpressionParameterAsUnknown()));
+        kind, ParserState::BraceExpressionParameterAsType,
+        ParserState::BraceExpressionParameterAsValue,
+        ParserState::BraceExpressionParameterAsUnknown));
   }
 }
 
@@ -664,14 +664,14 @@ auto Parser::HandleBraceExpressionFinishAsUnknownState() -> void {
 auto Parser::HandleCallExpressionState() -> void {
   auto state = PopState();
 
-  state.state = ParserState::CallExpressionFinish();
+  state.state = ParserState::CallExpressionFinish;
   PushState(state);
 
   AddNode(ParseNodeKind::CallExpressionStart(), Consume(), state.subtree_start,
           state.has_error);
   if (!PositionIs(TokenKind::CloseParen())) {
-    PushState(ParserState::CallExpressionParameterFinish());
-    PushState(ParserState::Expression());
+    PushState(ParserState::CallExpressionParameterFinish);
+    PushState(ParserState::Expression);
   }
 }
 
@@ -685,8 +685,8 @@ auto Parser::HandleCallExpressionParameterFinishState() -> void {
   if (ConsumeListToken(ParseNodeKind::CallExpressionComma(),
                        TokenKind::CloseParen(),
                        state.has_error) == ListTokenKind::Comma) {
-    PushState(ParserState::CallExpressionParameterFinish());
-    PushState(ParserState::Expression());
+    PushState(ParserState::CallExpressionParameterFinish);
+    PushState(ParserState::Expression);
   }
 }
 
@@ -719,12 +719,12 @@ auto Parser::HandleDeclarationLoopState() -> void {
       break;
     }
     case TokenKind::Fn(): {
-      PushState(ParserState::FunctionIntroducer());
+      PushState(ParserState::FunctionIntroducer);
       AddLeafNode(ParseNodeKind::FunctionIntroducer(), Consume());
       break;
     }
     case TokenKind::Package(): {
-      PushState(ParserState::Package());
+      PushState(ParserState::Package);
       break;
     }
     case TokenKind::Semi(): {
@@ -732,11 +732,11 @@ auto Parser::HandleDeclarationLoopState() -> void {
       break;
     }
     case TokenKind::Var(): {
-      PushState(ParserState::VarAsSemicolon());
+      PushState(ParserState::VarAsSemicolon);
       break;
     }
     case TokenKind::Interface(): {
-      PushState(ParserState::InterfaceIntroducer());
+      PushState(ParserState::InterfaceIntroducer);
       ++position_;
       break;
     }
@@ -817,15 +817,15 @@ auto Parser::HandleExpressionState() -> void {
       DiagnoseOperatorFixity(OperatorFixity::Prefix);
     }
 
-    PushStateForExpressionLoop(ParserState::ExpressionLoopForPrefix(),
+    PushStateForExpressionLoop(ParserState::ExpressionLoopForPrefix,
                                state.ambient_precedence, *operator_precedence);
     ++position_;
     PushStateForExpression(*operator_precedence);
   } else {
-    PushStateForExpressionLoop(ParserState::ExpressionLoop(),
+    PushStateForExpressionLoop(ParserState::ExpressionLoop,
                                state.ambient_precedence,
                                PrecedenceGroup::ForPostfixExpression());
-    PushState(ParserState::ExpressionInPostfix());
+    PushState(ParserState::ExpressionInPostfix);
   }
 }
 
@@ -833,7 +833,7 @@ auto Parser::HandleExpressionInPostfixState() -> void {
   auto state = PopState();
 
   // Continue to the loop state.
-  state.state = ParserState::ExpressionInPostfixLoop();
+  state.state = ParserState::ExpressionInPostfixLoop;
 
   // Parses a primary expression, which is either a terminal portion of an
   // expression tree, such as an identifier or literal, or a parenthesized
@@ -856,12 +856,12 @@ auto Parser::HandleExpressionInPostfixState() -> void {
     }
     case TokenKind::OpenCurlyBrace(): {
       PushState(state);
-      PushState(ParserState::BraceExpression());
+      PushState(ParserState::BraceExpression);
       break;
     }
     case TokenKind::OpenParen(): {
       PushState(state);
-      PushState(ParserState::ParenExpression());
+      PushState(ParserState::ParenExpression);
       break;
     }
     case TokenKind::SelfType(): {
@@ -888,13 +888,13 @@ auto Parser::HandleExpressionInPostfixLoopState() -> void {
   switch (PositionKind()) {
     case TokenKind::Period(): {
       PushState(state);
-      state.state = ParserState::DesignatorAsExpression();
+      state.state = ParserState::DesignatorAsExpression;
       PushState(state);
       break;
     }
     case TokenKind::OpenParen(): {
       PushState(state);
-      state.state = ParserState::CallExpression();
+      state.state = ParserState::CallExpression;
       PushState(state);
       break;
     }
@@ -950,7 +950,7 @@ auto Parser::HandleExpressionLoopState() -> void {
   state.lhs_precedence = operator_precedence;
 
   if (is_binary) {
-    state.state = ParserState::ExpressionLoopForBinary();
+    state.state = ParserState::ExpressionLoopForBinary;
     PushState(state);
     PushStateForExpression(operator_precedence);
   } else {
@@ -966,7 +966,7 @@ auto Parser::HandleExpressionLoopForBinaryState() -> void {
 
   AddNode(ParseNodeKind::InfixOperator(), state.token, state.subtree_start,
           state.has_error);
-  state.state = ParserState::ExpressionLoop();
+  state.state = ParserState::ExpressionLoop;
   state.has_error = false;
   PushState(state);
 }
@@ -976,7 +976,7 @@ auto Parser::HandleExpressionLoopForPrefixState() -> void {
 
   AddNode(ParseNodeKind::PrefixOperator(), state.token, state.subtree_start,
           state.has_error);
-  state.state = ParserState::ExpressionLoop();
+  state.state = ParserState::ExpressionLoop;
   state.has_error = false;
   PushState(state);
 }
@@ -1032,12 +1032,12 @@ auto Parser::HandleFunctionIntroducerState() -> void {
     return;
   }
 
-  state.state = ParserState::FunctionAfterDeducedParameterList();
+  state.state = ParserState::FunctionAfterDeducedParameterList;
   PushState(state);
 
   // If there are deduced params handle them next.
   if (PositionIs(TokenKind::OpenSquareBracket())) {
-    PushState(ParserState::DeducedParameterListFinish());
+    PushState(ParserState::DeducedParameterListFinish);
     // This is for sure a `[`, we can safely create the corresponding node.
     AddLeafNode(ParseNodeKind::DeducedParameterListStart(), Consume());
 
@@ -1049,7 +1049,7 @@ auto Parser::HandleFunctionIntroducerState() -> void {
     // parameters need to be added, we will probably need to push a more
     // general state.
     // Push state to handle `self`'s pattern binding.
-    PushState(ParserState::SelfPattern());
+    PushState(ParserState::SelfPattern);
     return;
   }
 }
@@ -1067,21 +1067,21 @@ auto Parser::HandleFunctionAfterDeducedParameterListState() -> void {
 
   // Parse the parameter list as its own subtree; once that pops, resume
   // function parsing.
-  state.state = ParserState::FunctionAfterParameterList();
+  state.state = ParserState::FunctionAfterParameterList;
   PushState(state);
-  PushState(ParserState::FunctionParameterListFinish());
+  PushState(ParserState::FunctionParameterListFinish);
   AddLeafNode(ParseNodeKind::ParameterListStart(), Consume());
 
   if (!PositionIs(TokenKind::CloseParen())) {
-    PushState(ParserState::FunctionParameter());
+    PushState(ParserState::FunctionParameter);
   }
 }
 
 auto Parser::HandleFunctionParameterState() -> void {
   PopAndDiscardState();
 
-  PushState(ParserState::FunctionParameterFinish());
-  PushState(ParserState::PatternAsFunctionParameter());
+  PushState(ParserState::FunctionParameterFinish);
+  PushState(ParserState::PatternAsFunctionParameter);
 }
 
 auto Parser::HandleFunctionParameterFinishState() -> void {
@@ -1094,7 +1094,7 @@ auto Parser::HandleFunctionParameterFinishState() -> void {
   if (ConsumeListToken(ParseNodeKind::ParameterListComma(),
                        TokenKind::CloseParen(),
                        state.has_error) == ListTokenKind::Comma) {
-    PushState(ParserState::PatternAsFunctionParameter());
+    PushState(ParserState::PatternAsFunctionParameter);
   }
 }
 
@@ -1110,13 +1110,13 @@ auto Parser::HandleFunctionAfterParameterListState() -> void {
   auto state = PopState();
 
   // Regardless of whether there's a return type, we'll finish the signature.
-  state.state = ParserState::FunctionSignatureFinish();
+  state.state = ParserState::FunctionSignatureFinish;
   PushState(state);
 
   // If there is a return type, parse the expression before adding the return
   // type nod.e
   if (PositionIs(TokenKind::MinusGreater())) {
-    PushState(ParserState::FunctionReturnTypeFinish());
+    PushState(ParserState::FunctionReturnTypeFinish);
     ++position_;
     PushStateForExpression(PrecedenceGroup::ForType());
   }
@@ -1152,9 +1152,9 @@ auto Parser::HandleFunctionSignatureFinishState() -> void {
               state.subtree_start, state.has_error);
       // Any error is recorded on the FunctionDefinitionStart.
       state.has_error = false;
-      state.state = ParserState::FunctionDefinitionFinish();
+      state.state = ParserState::FunctionDefinitionFinish;
       PushState(state);
-      PushState(ParserState::StatementScopeLoop());
+      PushState(ParserState::StatementScopeLoop);
       break;
     }
     default: {
@@ -1209,11 +1209,11 @@ auto Parser::HandleInterfaceIntroducerState() -> void {
     parse_body = false;
   }
 
-  state.state = ParserState::InterfaceDefinitionFinish();
+  state.state = ParserState::InterfaceDefinitionFinish;
   PushState(state);
 
   if (parse_body) {
-    PushState(ParserState::InterfaceDefinitionLoop());
+    PushState(ParserState::InterfaceDefinitionLoop);
     AddLeafNode(ParseNodeKind::InterfaceBodyStart(), Consume());
   }
 }
@@ -1232,7 +1232,7 @@ auto Parser::HandleInterfaceDefinitionLoopState() -> void {
       break;
     }
     case TokenKind::Fn(): {
-      PushState(ParserState::FunctionIntroducer());
+      PushState(ParserState::FunctionIntroducer);
       AddLeafNode(ParseNodeKind::FunctionIntroducer(), Consume());
       break;
     }
@@ -1343,17 +1343,17 @@ auto Parser::HandleParenCondition(ParseNodeKind start_kind,
 
   state.state = finish_state;
   PushState(state);
-  PushState(ParserState::Expression());
+  PushState(ParserState::Expression);
 }
 
 auto Parser::HandleParenConditionAsIfState() -> void {
   HandleParenCondition(ParseNodeKind::IfConditionStart(),
-                       ParserState::ParenConditionFinishAsIf());
+                       ParserState::ParenConditionFinishAsIf);
 }
 
 auto Parser::HandleParenConditionAsWhileState() -> void {
   HandleParenCondition(ParseNodeKind::WhileConditionStart(),
-                       ParserState::ParenConditionFinishAsWhile());
+                       ParserState::ParenConditionFinishAsWhile);
 }
 
 auto Parser::HandleParenConditionFinishAsIfState() -> void {
@@ -1376,13 +1376,13 @@ auto Parser::HandleParenExpressionState() -> void {
               ConsumeChecked(TokenKind::OpenParen()));
 
   if (PositionIs(TokenKind::CloseParen())) {
-    state.state = ParserState::ParenExpressionFinishAsTuple();
+    state.state = ParserState::ParenExpressionFinishAsTuple;
     PushState(state);
   } else {
-    state.state = ParserState::ParenExpressionFinish();
+    state.state = ParserState::ParenExpressionFinish;
     PushState(state);
-    PushState(ParserState::ParenExpressionParameterFinishAsUnknown());
-    PushState(ParserState::Expression());
+    PushState(ParserState::ParenExpressionParameterFinishAsUnknown);
+    PushState(ParserState::Expression);
   }
 }
 
@@ -1400,19 +1400,19 @@ auto Parser::HandleParenExpressionParameterFinish(bool as_tuple) -> void {
   // Note this could be `(expr,)` so we may not reuse the current state, but
   // it's still necessary to switch the parent.
   if (!as_tuple) {
-    state.state = ParserState::ParenExpressionParameterFinishAsTuple();
+    state.state = ParserState::ParenExpressionParameterFinishAsTuple;
 
     auto finish_state = PopState();
-    CARBON_CHECK(finish_state.state == ParserState::ParenExpressionFinish())
+    CARBON_CHECK(finish_state.state == ParserState::ParenExpressionFinish)
         << "Unexpected parent state, found: " << finish_state.state;
-    finish_state.state = ParserState::ParenExpressionFinishAsTuple();
+    finish_state.state = ParserState::ParenExpressionFinishAsTuple;
     PushState(finish_state);
   }
 
   // On a comma, push another expression handler.
   if (list_token_kind == ListTokenKind::Comma) {
     PushState(state);
-    PushState(ParserState::Expression());
+    PushState(ParserState::Expression);
   }
 }
 
@@ -1442,7 +1442,7 @@ auto Parser::HandlePattern(PatternKind pattern_kind) -> void {
   auto state = PopState();
 
   // Ensure the finish state always follows.
-  state.state = ParserState::PatternFinish();
+  state.state = ParserState::PatternFinish;
 
   // Handle an invalid pattern introducer for parameters and variables.
   if (!PositionIs(TokenKind::Identifier()) ||
@@ -1522,7 +1522,7 @@ auto Parser::HandleSelfPatternState() -> void {
 
   if (possible_self_param) {
     // Ensure the finish state always follows.
-    state.state = ParserState::PatternFinish();
+    state.state = ParserState::PatternFinish;
 
     // Switch the context token to the colon, so that it'll be used for the root
     // node.
@@ -1542,11 +1542,11 @@ auto Parser::HandleSelfPatternState() -> void {
 
   if (possible_addr_self_param) {
     // Ensure the finish state always follows.
-    state.state = ParserState::PatternAddress();
+    state.state = ParserState::PatternAddress;
     state.token = Consume();
     PushState(state);
 
-    PushState(ParserState::PatternFinish());
+    PushState(ParserState::PatternFinish);
 
     PushStateForExpression(PrecedenceGroup::ForType());
     AddLeafNode(ParseNodeKind::SelfDeducedParameter(), *(position_ + 1));
@@ -1558,7 +1558,7 @@ auto Parser::HandleSelfPatternState() -> void {
                     "Deduced parameters must be of the form: `<name>: <Type>` "
                     "or `addr <name>: <Type>`.");
   emitter_->Emit(*position_, ExpectedDeducedParam);
-  state.state = ParserState::PatternFinish();
+  state.state = ParserState::PatternFinish;
   state.has_error = true;
 
   // Try to recover by skipping to the next `]`.
@@ -1576,40 +1576,40 @@ auto Parser::HandleStatementState() -> void {
 
   switch (PositionKind()) {
     case TokenKind::Break(): {
-      PushState(ParserState::StatementBreakFinish());
+      PushState(ParserState::StatementBreakFinish);
       AddLeafNode(ParseNodeKind::BreakStatementStart(), Consume());
       break;
     }
     case TokenKind::Continue(): {
-      PushState(ParserState::StatementContinueFinish());
+      PushState(ParserState::StatementContinueFinish);
       AddLeafNode(ParseNodeKind::ContinueStatementStart(), Consume());
       break;
     }
     case TokenKind::For(): {
-      PushState(ParserState::StatementForFinish());
-      PushState(ParserState::StatementForHeader());
+      PushState(ParserState::StatementForFinish);
+      PushState(ParserState::StatementForHeader);
       ++position_;
       break;
     }
     case TokenKind::If(): {
-      PushState(ParserState::StatementIf());
+      PushState(ParserState::StatementIf);
       break;
     }
     case TokenKind::Return(): {
-      PushState(ParserState::StatementReturn());
+      PushState(ParserState::StatementReturn);
       break;
     }
     case TokenKind::Var(): {
-      PushState(ParserState::VarAsSemicolon());
+      PushState(ParserState::VarAsSemicolon);
       break;
     }
     case TokenKind::While(): {
-      PushState(ParserState::StatementWhile());
+      PushState(ParserState::StatementWhile);
       break;
     }
     default: {
-      PushState(ParserState::ExpressionStatementFinish());
-      PushState(ParserState::Expression());
+      PushState(ParserState::ExpressionStatementFinish);
+      PushState(ParserState::Expression);
       break;
     }
   }
@@ -1628,11 +1628,11 @@ auto Parser::HandleStatementForHeaderState() -> void {
 
   ConsumeAndAddOpenParen(state.token, ParseNodeKind::ForHeaderStart());
 
-  state.state = ParserState::StatementForHeaderIn();
+  state.state = ParserState::StatementForHeaderIn;
 
   if (PositionIs(TokenKind::Var())) {
     PushState(state);
-    PushState(ParserState::VarAsFor());
+    PushState(ParserState::VarAsFor);
   } else {
     CARBON_DIAGNOSTIC(ExpectedVariableDeclaration, Error,
                       "Expected `var` declaration.");
@@ -1650,9 +1650,9 @@ auto Parser::HandleStatementForHeaderState() -> void {
 auto Parser::HandleStatementForHeaderInState() -> void {
   auto state = PopState();
 
-  state.state = ParserState::StatementForHeaderFinish();
+  state.state = ParserState::StatementForHeaderFinish;
   PushState(state);
-  PushState(ParserState::Expression());
+  PushState(ParserState::Expression);
 }
 
 auto Parser::HandleStatementForHeaderFinishState() -> void {
@@ -1660,7 +1660,7 @@ auto Parser::HandleStatementForHeaderFinishState() -> void {
 
   ConsumeAndAddCloseParen(state, ParseNodeKind::ForHeader());
 
-  PushState(ParserState::CodeBlock());
+  PushState(ParserState::CodeBlock);
 }
 
 auto Parser::HandleStatementForFinishState() -> void {
@@ -1673,17 +1673,17 @@ auto Parser::HandleStatementForFinishState() -> void {
 auto Parser::HandleStatementIfState() -> void {
   PopAndDiscardState();
 
-  PushState(ParserState::StatementIfConditionFinish());
-  PushState(ParserState::ParenConditionAsIf());
+  PushState(ParserState::StatementIfConditionFinish);
+  PushState(ParserState::ParenConditionAsIf);
   ++position_;
 }
 
 auto Parser::HandleStatementIfConditionFinishState() -> void {
   auto state = PopState();
 
-  state.state = ParserState::StatementIfThenBlockFinish();
+  state.state = ParserState::StatementIfThenBlockFinish;
   PushState(state);
-  PushState(ParserState::CodeBlock());
+  PushState(ParserState::CodeBlock);
 }
 
 auto Parser::HandleStatementIfThenBlockFinishState() -> void {
@@ -1691,11 +1691,11 @@ auto Parser::HandleStatementIfThenBlockFinishState() -> void {
 
   if (ConsumeAndAddLeafNodeIf(TokenKind::Else(),
                               ParseNodeKind::IfStatementElse())) {
-    state.state = ParserState::StatementIfElseBlockFinish();
+    state.state = ParserState::StatementIfElseBlockFinish;
     PushState(state);
     // `else if` is permitted as a special case.
-    PushState(PositionIs(TokenKind::If()) ? ParserState::StatementIf()
-                                          : ParserState::CodeBlock());
+    PushState(PositionIs(TokenKind::If()) ? ParserState::StatementIf
+                                          : ParserState::CodeBlock);
   } else {
     AddNode(ParseNodeKind::IfStatement(), state.token, state.subtree_start,
             state.has_error);
@@ -1730,12 +1730,12 @@ auto Parser::HandleStatementKeywordFinish(ParseNodeKind node_kind) -> void {
 
 auto Parser::HandleStatementReturnState() -> void {
   auto state = PopState();
-  state.state = ParserState::StatementReturnFinish();
+  state.state = ParserState::StatementReturnFinish;
   PushState(state);
 
   AddLeafNode(ParseNodeKind::ReturnStatementStart(), Consume());
   if (!PositionIs(TokenKind::Semi())) {
-    PushState(ParserState::Expression());
+    PushState(ParserState::Expression);
   }
 }
 
@@ -1753,24 +1753,24 @@ auto Parser::HandleStatementScopeLoopState() -> void {
       ReturnErrorOnState();
     }
   } else {
-    PushState(ParserState::Statement());
+    PushState(ParserState::Statement);
   }
 }
 
 auto Parser::HandleStatementWhileState() -> void {
   PopAndDiscardState();
 
-  PushState(ParserState::StatementWhileConditionFinish());
-  PushState(ParserState::ParenConditionAsWhile());
+  PushState(ParserState::StatementWhileConditionFinish);
+  PushState(ParserState::ParenConditionAsWhile);
   ++position_;
 }
 
 auto Parser::HandleStatementWhileConditionFinishState() -> void {
   auto state = PopState();
 
-  state.state = ParserState::StatementWhileBlockFinish();
+  state.state = ParserState::StatementWhileBlockFinish;
   PushState(state);
-  PushState(ParserState::CodeBlock());
+  PushState(ParserState::CodeBlock);
 }
 
 auto Parser::HandleStatementWhileBlockFinishState() -> void {
@@ -1785,20 +1785,20 @@ auto Parser::HandleVar(ParserState finish_state) -> void {
 
   // These will start at the `var`.
   PushState(finish_state);
-  PushState(ParserState::VarAfterPattern());
+  PushState(ParserState::VarAfterPattern);
 
   AddLeafNode(ParseNodeKind::VariableIntroducer(), Consume());
 
   // This will start at the pattern.
-  PushState(ParserState::PatternAsVariable());
+  PushState(ParserState::PatternAsVariable);
 }
 
 auto Parser::HandleVarAsSemicolonState() -> void {
-  HandleVar(ParserState::VarFinishAsSemicolon());
+  HandleVar(ParserState::VarFinishAsSemicolon);
 }
 
 auto Parser::HandleVarAsForState() -> void {
-  HandleVar(ParserState::VarFinishAsFor());
+  HandleVar(ParserState::VarFinishAsFor);
 }
 
 auto Parser::HandleVarAfterPatternState() -> void {
@@ -1813,7 +1813,7 @@ auto Parser::HandleVarAfterPatternState() -> void {
 
   if (auto equals = ConsumeIf(TokenKind::Equal())) {
     AddLeafNode(ParseNodeKind::VariableInitializer(), *equals);
-    PushState(ParserState::Expression());
+    PushState(ParserState::Expression);
   }
 }
 

+ 1 - 1
toolchain/parser/parser.h

@@ -234,7 +234,7 @@ class Parser {
 
   // Pushes a new expression state with specific precedence.
   auto PushStateForExpression(PrecedenceGroup ambient_precedence) -> void {
-    PushState(StateStackEntry(ParserState::Expression(), ambient_precedence,
+    PushState(StateStackEntry(ParserState::Expression, ambient_precedence,
                               PrecedenceGroup::ForTopLevelExpression(),
                               *position_, tree_->size()));
   }

+ 3 - 8
toolchain/parser/parser_state.cpp

@@ -4,16 +4,11 @@
 
 #include "toolchain/parser/parser_state.h"
 
-#include "llvm/ADT/StringRef.h"
-
 namespace Carbon {
 
-auto ParserState::name() const -> llvm::StringRef {
-  static constexpr llvm::StringLiteral Names[] = {
-#define CARBON_PARSER_STATE(Name) #Name,
+CARBON_DEFINE_ENUM_CLASS_NAMES(ParserState) = {
+#define CARBON_PARSER_STATE(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
 #include "toolchain/parser/parser_state.def"
-  };
-  return Names[static_cast<int>(state_)];
-}
+};
 
 }  // namespace Carbon

+ 12 - 40
toolchain/parser/parser_state.h

@@ -5,55 +5,27 @@
 #ifndef CARBON_TOOLCHAIN_PARSER_PARSER_STATE_H_
 #define CARBON_TOOLCHAIN_PARSER_PARSER_STATE_H_
 
-#include <cstdint>
-#include <iterator>
-
-#include "common/ostream.h"
-#include "llvm/ADT/StringRef.h"
+#include "toolchain/common/enum_base.h"
 
 namespace Carbon {
 
-class ParserState {
-  // Note that this must be declared earlier in the class so that its type can
-  // be used, for example in the conversion operator.
-  enum class StateEnum : uint8_t {
-#define CARBON_PARSER_STATE(Name) Name,
+CARBON_DEFINE_RAW_ENUM_CLASS(ParserState, uint8_t){
+#define CARBON_PARSER_STATE(Name) CARBON_RAW_ENUM_ENUMERATOR(Name)
 #include "toolchain/parser/parser_state.def"
-  };
+};
 
+class ParserState : public CARBON_ENUM_BASE(ParserState) {
  public:
-  // `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_PARSER_STATE(Name)             \
-  static constexpr auto Name()->ParserState { \
-    return ParserState(StateEnum::Name);      \
-  }
+#define CARBON_PARSER_STATE(Name) CARBON_ENUM_CONSTANT_DECLARATION(Name)
 #include "toolchain/parser/parser_state.def"
-
-  // The default constructor is deleted because objects of this type should
-  // always be constructed using the above factory functions for each unique
-  // kind.
-  ParserState() = delete;
-
-  // Gets a friendly name for the token for logging or debugging.
-  [[nodiscard]] auto name() const -> llvm::StringRef;
-
-  // Enable conversion to our private enum, including in a `constexpr` context,
-  // to enable usage in `switch` and `case`. The enum remains private and
-  // nothing else should be using this function.
-  // NOLINTNEXTLINE(google-explicit-constructor)
-  constexpr operator StateEnum() const { return state_; }
-
-  void Print(llvm::raw_ostream& out) const { out << name(); }
-
- private:
-  constexpr explicit ParserState(StateEnum k) : state_(k) {}
-
-  StateEnum state_;
 };
 
-// We expect the parse node kind to fit compactly into 8 bits.
-static_assert(sizeof(ParserState) == 1, "ParserState objects include padding!");
+#define CARBON_PARSER_STATE(Name) \
+  CARBON_ENUM_CONSTANT_DEFINITION(ParserState, Name)
+#include "toolchain/parser/parser_state.def"
+
+// We expect ParserState to fit compactly into 8 bits.
+static_assert(sizeof(ParserState) == 1, "ParserState includes padding!");
 
 }  // namespace Carbon
 

+ 0 - 23
toolchain/parser/parser_state_test.cpp

@@ -1,23 +0,0 @@
-// 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/parser/parser_state.h"
-
-#include <gtest/gtest.h>
-
-#include <cstring>
-
-#include "llvm/ADT/StringRef.h"
-
-namespace Carbon::Testing {
-namespace {
-
-// Not much to test here, so just verify that the API compiles and returns the
-// data in the `.def` file.
-#define CARBON_PARSER_STATE(Name) \
-  TEST(ParserState, Name) { EXPECT_EQ(#Name, ParserState::Name().name()); }
-#include "toolchain/parser/parser_state.def"
-
-}  // namespace
-}  // namespace Carbon::Testing

+ 2 - 8
toolchain/semantics/BUILD

@@ -9,10 +9,7 @@ cc_library(
     srcs = ["semantics_builtin_kind.cpp"],
     hdrs = ["semantics_builtin_kind.h"],
     textual_hdrs = ["semantics_builtin_kind.def"],
-    deps = [
-        "//common:ostream",
-        "@llvm-project//llvm:Support",
-    ],
+    deps = ["//toolchain/common:enum_base"],
 )
 
 cc_library(
@@ -20,10 +17,7 @@ cc_library(
     srcs = ["semantics_node_kind.cpp"],
     hdrs = ["semantics_node_kind.h"],
     textual_hdrs = ["semantics_node_kind.def"],
-    deps = [
-        "//common:ostream",
-        "@llvm-project//llvm:Support",
-    ],
+    deps = ["//toolchain/common:enum_base"],
 )
 
 cc_library(

+ 3 - 8
toolchain/semantics/semantics_builtin_kind.cpp

@@ -4,16 +4,11 @@
 
 #include "toolchain/semantics/semantics_builtin_kind.h"
 
-#include "llvm/ADT/StringRef.h"
-
 namespace Carbon {
 
-auto SemanticsBuiltinKind::name() const -> llvm::StringRef {
-  static constexpr llvm::StringLiteral Names[] = {
-#define CARBON_SEMANTICS_BUILTIN_KIND(Name) #Name,
+CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsBuiltinKind) = {
+#define CARBON_SEMANTICS_BUILTIN_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
-  };
-  return Names[static_cast<int>(kind_)];
-}
+};
 
 }  // namespace Carbon

+ 17 - 42
toolchain/semantics/semantics_builtin_kind.h

@@ -5,60 +5,35 @@
 #ifndef CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_BUILTIN_KIND_H_
 #define CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_BUILTIN_KIND_H_
 
-#include <cstdint>
-
-#include "common/ostream.h"
+#include "toolchain/common/enum_base.h"
 
 namespace Carbon {
 
-class SemanticsBuiltinKind {
- private:
-  // Note that this must be declared earlier in the class so that its type can
-  // be used, for example in the conversion operator.
-  enum class KindEnum : uint8_t {
-#define CARBON_SEMANTICS_BUILTIN_KIND(Name) Name,
+CARBON_DEFINE_RAW_ENUM_CLASS(SemanticsBuiltinKind, uint8_t){
+#define CARBON_SEMANTICS_BUILTIN_KIND(Name) CARBON_RAW_ENUM_ENUMERATOR(Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
-  };
+};
 
+class SemanticsBuiltinKind : public CARBON_ENUM_BASE(SemanticsBuiltinKind) {
  public:
-  // The count of enum values excluding Invalid.
-  static constexpr uint8_t ValidCount = static_cast<uint8_t>(KindEnum::Invalid);
-
-  // `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_SEMANTICS_BUILTIN_KIND(Name)            \
-  static constexpr auto Name()->SemanticsBuiltinKind { \
-    return SemanticsBuiltinKind(KindEnum::Name);       \
-  }
+#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \
+  CARBON_ENUM_CONSTANT_DECLARATION(Name)
 #include "toolchain/semantics/semantics_builtin_kind.def"
 
-  // The default constructor is deleted because objects of this type should
-  // always be constructed using the above factory functions for each unique
-  // kind.
-  SemanticsBuiltinKind() = delete;
-
-  // Gets a friendly name for the token for logging or debugging.
-  [[nodiscard]] auto name() const -> llvm::StringRef;
+  // The count of enum values excluding Invalid.
+  // NOLINTNEXTLINE(readability-identifier-naming)
+  static const uint8_t ValidCount;
 
   // Support conversion to and from an int32_t for SemanticNode storage.
-  auto AsInt() -> int32_t { return static_cast<int32_t>(kind_); }
-  static auto FromInt(int32_t val) -> SemanticsBuiltinKind {
-    return SemanticsBuiltinKind(static_cast<KindEnum>(val));
-  }
-
-  // Enable conversion to our private enum, including in a `constexpr`
-  // context, to enable usage in `switch` and `case`. The enum remains
-  // private and nothing else should be using this function.
-  // NOLINTNEXTLINE(google-explicit-constructor)
-  constexpr operator KindEnum() const { return kind_; }
-
-  void Print(llvm::raw_ostream& out) const { out << name(); }
+  using EnumBase::AsInt;
+  using EnumBase::FromInt;
+};
 
- private:
-  constexpr explicit SemanticsBuiltinKind(KindEnum k) : kind_(k) {}
+#define CARBON_SEMANTICS_BUILTIN_KIND(Name) \
+  CARBON_ENUM_CONSTANT_DEFINITION(SemanticsBuiltinKind, Name)
+#include "toolchain/semantics/semantics_builtin_kind.def"
 
-  KindEnum kind_;
-};
+constexpr uint8_t SemanticsBuiltinKind::ValidCount = Invalid.AsInt();
 
 // We expect the builtin kind to fit compactly into 8 bits.
 static_assert(sizeof(SemanticsBuiltinKind) == 1,

+ 5 - 5
toolchain/semantics/semantics_ir.cpp

@@ -19,24 +19,24 @@ auto SemanticsIR::MakeBuiltinIR() -> SemanticsIR {
 
   constexpr int32_t TypeOfTypeType = 0;
   auto type_type = semantics.AddNode(
-      block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::TypeType(),
+      block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::TypeType,
                                            SemanticsNodeId(TypeOfTypeType)));
   CARBON_CHECK(type_type.index == TypeOfTypeType)
       << "TypeType's type must be self-referential.";
 
   constexpr int32_t TypeOfInvalidType = 1;
   auto invalid_type = semantics.AddNode(
-      block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::InvalidType(),
+      block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::InvalidType,
                                            SemanticsNodeId(TypeOfInvalidType)));
   CARBON_CHECK(invalid_type.index == TypeOfInvalidType)
       << "InvalidType's type must be self-referential.";
 
   semantics.AddNode(
-      block_id, SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::IntegerType(),
-                                           type_type));
+      block_id,
+      SemanticsNode::MakeBuiltin(SemanticsBuiltinKind::IntegerType, type_type));
 
   semantics.AddNode(block_id, SemanticsNode::MakeBuiltin(
-                                  SemanticsBuiltinKind::RealType(), type_type));
+                                  SemanticsBuiltinKind::RealType, type_type));
 
   CARBON_CHECK(semantics.node_blocks_.size() == 1)
       << "BuildBuiltins should only produce 1 block, actual: "

+ 1 - 1
toolchain/semantics/semantics_node.cpp

@@ -25,7 +25,7 @@ void SemanticsNode::Print(llvm::raw_ostream& out) const {
   out << kind_ << "(";
   switch (kind_) {
 #define CARBON_SEMANTICS_NODE_KIND(Name) \
-  case SemanticsNodeKind::Name():        \
+  case SemanticsNodeKind::Name:          \
     PrintArgs(out, GetAs##Name());       \
     break;
 #include "toolchain/semantics/semantics_node_kind.def"

+ 33 - 33
toolchain/semantics/semantics_node.h

@@ -65,34 +65,34 @@ class SemanticsNode {
   static auto MakeAssign(ParseTree::Node parse_node, SemanticsNodeId type,
                          SemanticsNodeId lhs, SemanticsNodeId rhs)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::Assign(), type,
-                         lhs.index, rhs.index);
+    return SemanticsNode(parse_node, SemanticsNodeKind::Assign, type, lhs.index,
+                         rhs.index);
   }
   auto GetAsAssign() const -> std::pair<SemanticsNodeId, SemanticsNodeId> {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::Assign());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::Assign);
     return {SemanticsNodeId(arg0_), SemanticsNodeId(arg1_)};
   }
 
   static auto MakeBinaryOperatorAdd(ParseTree::Node parse_node,
                                     SemanticsNodeId type, SemanticsNodeId lhs,
                                     SemanticsNodeId rhs) -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::BinaryOperatorAdd(),
-                         type, lhs.index, rhs.index);
+    return SemanticsNode(parse_node, SemanticsNodeKind::BinaryOperatorAdd, type,
+                         lhs.index, rhs.index);
   }
   auto GetAsBinaryOperatorAdd() const
       -> std::pair<SemanticsNodeId, SemanticsNodeId> {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::BinaryOperatorAdd());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::BinaryOperatorAdd);
     return {SemanticsNodeId(arg0_), SemanticsNodeId(arg1_)};
   }
 
   static auto MakeBindName(ParseTree::Node parse_node, SemanticsNodeId type,
                            SemanticsStringId name, SemanticsNodeId node)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::BindName(), type,
+    return SemanticsNode(parse_node, SemanticsNodeKind::BindName, type,
                          name.index, node.index);
   }
   auto GetAsBindName() const -> std::pair<SemanticsStringId, SemanticsNodeId> {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::BindName());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::BindName);
     return {SemanticsStringId(arg0_), SemanticsNodeId(arg1_)};
   }
 
@@ -100,21 +100,21 @@ class SemanticsNode {
                           SemanticsNodeId type) -> SemanticsNode {
     // Builtins won't have a ParseTree node associated, so we provide the
     // default invalid one.
-    return SemanticsNode(ParseTree::Node(), SemanticsNodeKind::Builtin(), type,
+    return SemanticsNode(ParseTree::Node(), SemanticsNodeKind::Builtin, type,
                          builtin_kind.AsInt());
   }
   auto GetAsBuiltin() const -> SemanticsBuiltinKind {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::Builtin());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::Builtin);
     return SemanticsBuiltinKind::FromInt(arg0_);
   }
 
   static auto MakeCodeBlock(ParseTree::Node parse_node,
                             SemanticsNodeBlockId node_block) -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::CodeBlock(),
+    return SemanticsNode(parse_node, SemanticsNodeKind::CodeBlock,
                          SemanticsNodeId(), node_block.index);
   }
   auto GetAsCodeBlock() const -> SemanticsNodeBlockId {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::CodeBlock());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::CodeBlock);
     return SemanticsNodeBlockId(arg0_);
   }
 
@@ -122,23 +122,23 @@ class SemanticsNode {
                                  SemanticsCrossReferenceIRId ir,
                                  SemanticsNodeId node) -> SemanticsNode {
     return SemanticsNode(ParseTree::Node::MakeInvalid(),
-                         SemanticsNodeKind::CrossReference(), type, ir.index,
+                         SemanticsNodeKind::CrossReference, type, ir.index,
                          node.index);
   }
   auto GetAsCrossReference() const
       -> std::pair<SemanticsCrossReferenceIRId, SemanticsNodeBlockId> {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::CrossReference());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::CrossReference);
     return {SemanticsCrossReferenceIRId(arg0_), SemanticsNodeBlockId(arg1_)};
   }
 
   // TODO: The signature should be added as a parameter.
   static auto MakeFunctionDeclaration(ParseTree::Node parse_node)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::FunctionDeclaration(),
+    return SemanticsNode(parse_node, SemanticsNodeKind::FunctionDeclaration,
                          SemanticsNodeId());
   }
   auto GetAsFunctionDeclaration() const -> NoArgs {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDeclaration());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDeclaration);
     return {};
   }
 
@@ -146,35 +146,35 @@ class SemanticsNode {
                                      SemanticsNodeId decl,
                                      SemanticsNodeBlockId node_block)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::FunctionDefinition(),
+    return SemanticsNode(parse_node, SemanticsNodeKind::FunctionDefinition,
                          SemanticsNodeId(), decl.index, node_block.index);
   }
   auto GetAsFunctionDefinition() const
       -> std::pair<SemanticsNodeId, SemanticsNodeBlockId> {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDefinition());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::FunctionDefinition);
     return {SemanticsNodeId(arg0_), SemanticsNodeBlockId(arg1_)};
   }
 
   static auto MakeIntegerLiteral(ParseTree::Node parse_node,
                                  SemanticsIntegerLiteralId integer)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::IntegerLiteral(),
+    return SemanticsNode(parse_node, SemanticsNodeKind::IntegerLiteral,
                          SemanticsNodeId::MakeBuiltinReference(
-                             SemanticsBuiltinKind::IntegerType()),
+                             SemanticsBuiltinKind::IntegerType),
                          integer.index);
   }
   auto GetAsIntegerLiteral() const -> SemanticsIntegerLiteralId {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::IntegerLiteral());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::IntegerLiteral);
     return SemanticsIntegerLiteralId(arg0_);
   }
 
   static auto MakeRealLiteral(ParseTree::Node parse_node) -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::RealLiteral(),
-                         SemanticsNodeId::MakeBuiltinReference(
-                             SemanticsBuiltinKind::RealType()));
+    return SemanticsNode(
+        parse_node, SemanticsNodeKind::RealLiteral,
+        SemanticsNodeId::MakeBuiltinReference(SemanticsBuiltinKind::RealType));
   }
   auto GetAsRealLiteral() const -> NoArgs {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::RealLiteral());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::RealLiteral);
     return {};
   }
 
@@ -182,36 +182,36 @@ class SemanticsNode {
     // The actual type is `()`. However, code dealing with `return;` should
     // understand the type without checking, so it's not necessary but could be
     // specified if needed.
-    return SemanticsNode(parse_node, SemanticsNodeKind::Return(),
+    return SemanticsNode(parse_node, SemanticsNodeKind::Return,
                          SemanticsNodeId());
   }
   auto GetAsReturn() const -> NoArgs {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::Return());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::Return);
     return {};
   }
 
   static auto MakeReturnExpression(ParseTree::Node parse_node,
                                    SemanticsNodeId type, SemanticsNodeId expr)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::ReturnExpression(),
-                         type, expr.index);
+    return SemanticsNode(parse_node, SemanticsNodeKind::ReturnExpression, type,
+                         expr.index);
   }
   auto GetAsReturnExpression() const -> SemanticsNodeId {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::ReturnExpression());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::ReturnExpression);
     return SemanticsNodeId(arg0_);
   }
 
   static auto MakeVarStorage(ParseTree::Node parse_node, SemanticsNodeId type)
       -> SemanticsNode {
-    return SemanticsNode(parse_node, SemanticsNodeKind::VarStorage(), type);
+    return SemanticsNode(parse_node, SemanticsNodeKind::VarStorage, type);
   }
   auto GetAsVarStorage() const -> NoArgs {
-    CARBON_CHECK(kind_ == SemanticsNodeKind::VarStorage());
+    CARBON_CHECK(kind_ == SemanticsNodeKind::VarStorage);
     return NoArgs();
   }
 
   SemanticsNode()
-      : SemanticsNode(ParseTree::Node(), SemanticsNodeKind::Invalid(),
+      : SemanticsNode(ParseTree::Node(), SemanticsNodeKind::Invalid,
                       SemanticsNodeId()) {}
 
   auto parse_node() const -> ParseTree::Node { return parse_node_; }

+ 3 - 8
toolchain/semantics/semantics_node_kind.cpp

@@ -4,16 +4,11 @@
 
 #include "toolchain/semantics/semantics_node_kind.h"
 
-#include "llvm/ADT/StringRef.h"
-
 namespace Carbon {
 
-auto SemanticsNodeKind::name() const -> llvm::StringRef {
-  static constexpr llvm::StringLiteral Names[] = {
-#define CARBON_SEMANTICS_NODE_KIND(Name, ...) #Name,
+CARBON_DEFINE_ENUM_CLASS_NAMES(SemanticsNodeKind) = {
+#define CARBON_SEMANTICS_NODE_KIND(Name) CARBON_ENUM_CLASS_NAME_STRING(Name)
 #include "toolchain/semantics/semantics_node_kind.def"
-  };
-  return Names[static_cast<int>(kind_)];
-}
+};
 
 }  // namespace Carbon

+ 10 - 37
toolchain/semantics/semantics_node_kind.h

@@ -5,52 +5,25 @@
 #ifndef CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_NODE_KIND_H_
 #define CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_NODE_KIND_H_
 
-#include <cstdint>
-
-#include "common/ostream.h"
+#include "toolchain/common/enum_base.h"
 
 namespace Carbon {
 
-class SemanticsNodeKind {
- private:
-  // Note that this must be declared earlier in the class so that its type can
-  // be used, for example in the conversion operator.
-  enum class KindEnum : uint8_t {
-#define CARBON_SEMANTICS_NODE_KIND(Name, ...) Name,
+CARBON_DEFINE_RAW_ENUM_CLASS(SemanticsNodeKind, 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) {
  public:
-  // `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_SEMANTICS_NODE_KIND(Name, ...)       \
-  static constexpr auto Name()->SemanticsNodeKind { \
-    return SemanticsNodeKind(KindEnum::Name);       \
-  }
+#define CARBON_SEMANTICS_NODE_KIND(Name) CARBON_ENUM_CONSTANT_DECLARATION(Name)
 #include "toolchain/semantics/semantics_node_kind.def"
-
-  // The default constructor is deleted because objects of this type should
-  // always be constructed using the above factory functions for each unique
-  // kind.
-  SemanticsNodeKind() = delete;
-
-  // Gets a friendly name for the token for logging or debugging.
-  [[nodiscard]] auto name() const -> llvm::StringRef;
-
-  // Enable conversion to our private enum, including in a `constexpr` context,
-  // to enable usage in `switch` and `case`. The enum remains private and
-  // nothing else should be using this function.
-  // NOLINTNEXTLINE(google-explicit-constructor)
-  constexpr operator KindEnum() const { return kind_; }
-
-  void Print(llvm::raw_ostream& out) const { out << name(); }
-
- private:
-  constexpr explicit SemanticsNodeKind(KindEnum k) : kind_(k) {}
-
-  KindEnum kind_;
 };
 
+#define CARBON_SEMANTICS_NODE_KIND(Name) \
+  CARBON_ENUM_CONSTANT_DEFINITION(SemanticsNodeKind, 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!");
 

+ 3 - 3
toolchain/semantics/semantics_parse_tree_handler.cpp

@@ -246,7 +246,7 @@ auto SemanticsParseTreeHandler::TryTypeConversion(ParseTree::Node parse_node,
   // implemented to do that right now.
   if (lhs_type != rhs_type) {
     auto invalid_type = SemanticsNodeId::MakeBuiltinReference(
-        SemanticsBuiltinKind::InvalidType());
+        SemanticsBuiltinKind::InvalidType);
     if (lhs_type != invalid_type && rhs_type != invalid_type) {
       // TODO: This is a poor diagnostic, and should be expanded.
       CARBON_DIAGNOSTIC(TypeMismatch, Error,
@@ -491,7 +491,7 @@ auto SemanticsParseTreeHandler::HandleLiteral(ParseTree::Node parse_node)
       auto text = tokens_->GetTokenText(token);
       CARBON_CHECK(text == "i32") << "Currently only i32 is allowed";
       Push(parse_node, SemanticsNodeId::MakeBuiltinReference(
-                           SemanticsBuiltinKind::IntegerType()));
+                           SemanticsBuiltinKind::IntegerType));
       break;
     }
     default:
@@ -508,7 +508,7 @@ auto SemanticsParseTreeHandler::HandleNameReference(ParseTree::Node parse_node)
                       llvm::StringRef);
     emitter_->Emit(parse_node, NameNotFound, name_str);
     Push(parse_node, SemanticsNodeId::MakeBuiltinReference(
-                         SemanticsBuiltinKind::InvalidType()));
+                         SemanticsBuiltinKind::InvalidType));
   };
 
   auto name_id = semantics_->GetString(name_str);