Use TypeEnum for ScopeId to refactor call structure (#5491)

I'm trying to make the offsetting a little easier to understand, and
also get a better `requires` structure on calls. The second is for an
attempt to refactor the `Formatter` API, but also changing the `InstId`
`derived_from` requires seems helpful for clarity on what's really
happening.

common/BUILD

@@ -583,6 +583,12 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "type_enum",
+    hdrs = ["type_enum.h"],
+    deps = [":ostream"],
+)
+
 # The base version source file only uses non-stamped parts of the version
 # information so we expand it once here without any stamping.
 expand_version_build_info(

common/type_enum.h

@@ -0,0 +1,128 @@
+// 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_TYPE_ENUM_H_
+#define CARBON_COMMON_TYPE_ENUM_H_
+
+#include <algorithm>
+
+#include "common/ostream.h"
+
+namespace Carbon {
+
+// An enum whose values are the specified types.
+template <typename... Types>
+class TypeEnum : public Printable<TypeEnum<Types...>> {
+ public:
+  using TypeTuple = std::tuple<Types...>;
+
+  static constexpr size_t NumTypes = sizeof...(Types);
+  static constexpr size_t NumValues = NumTypes + 2;
+
+  static_assert(NumValues <= 256, "Too many types for raw enum.");
+
+  // The underlying raw enumeration type.
+  //
+  // The enum_extensibility attribute indicates that this enum is intended to
+  // take values that do not correspond to its declared enumerators.
+  enum class [[clang::enum_extensibility(open)]] RawEnumType : uint8_t {
+    // The first sizeof...(Types) values correspond to the types.
+
+    // An explicitly invalid value.
+    Invalid = NumTypes,
+
+    // Indicates that no type should be used.
+    // TODO: This doesn't really fit the model of this type, but it's convenient
+    // for all of its users.
+    None,
+  };
+
+  // Accesses the type given an enum value.
+  template <RawEnumType K>
+    requires(K != RawEnumType::Invalid)
+  using TypeFor = __type_pack_element<static_cast<size_t>(K), Types...>;
+
+  // Workaround for Clang bug https://github.com/llvm/llvm-project/issues/85461
+  template <RawEnumType Value>
+  static constexpr auto FromRaw = TypeEnum(Value);
+
+  // Names for the `Invalid` and `None` enumeration values.
+  static constexpr const TypeEnum& Invalid = FromRaw<RawEnumType::Invalid>;
+  static constexpr const TypeEnum& None = FromRaw<RawEnumType::None>;
+
+  // Accesses the enumeration value for the type `Type`. If `AllowInvalid` is
+  // set, any unexpected type is mapped to `Invalid`, otherwise an invalid type
+  // results in a compile error.
+  //
+  // The `Self` parameter is an implementation detail to allow `ForImpl` to be
+  // defined after this template, and should not be specified.
+  template <typename Type, bool AllowInvalid = false, typename Self = TypeEnum>
+  static constexpr auto For = Self::template ForImpl<Type, AllowInvalid>();
+
+  // This bool indicates whether the specified type corresponds to a value in
+  // this enum.
+  template <typename Type>
+  static constexpr bool Contains = For<Type, true>.is_valid();
+
+  // Explicitly convert from the raw enum type.
+  explicit constexpr TypeEnum(RawEnumType value) : value_(value) {}
+
+  // Implicitly convert to the raw enum type, for use in `switch`.
+  //
+  // 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 raw enum value.
+  constexpr auto ToRaw() const -> RawEnumType { return value_; }
+
+  // Returns a value that can be used as an array index. Returned value will be
+  // < NumValues.
+  constexpr auto ToIndex() const -> size_t {
+    return static_cast<size_t>(value_);
+  }
+
+  // Returns whether this is a valid value, not `Invalid`.
+  constexpr auto is_valid() const -> bool {
+    return value_ != RawEnumType::Invalid;
+  }
+
+  auto Print(llvm::raw_ostream& out) const -> void {
+    out << "TypeEnum(";
+    if (value_ == RawEnumType::None) {
+      out << "None";
+    } else {
+      static constexpr std::array<llvm::StringLiteral, sizeof...(Types)> Names =
+          {
+              Types::Label...,
+          };
+      out << Names[static_cast<int>(value_)];
+    }
+    out << ")";
+  }
+
+ private:
+  // Translates a type to its enum value, or `Invalid`.
+  template <typename IdT, bool AllowInvalid>
+  static constexpr auto ForImpl() -> TypeEnum {
+    // A bool for each type saying whether it matches. The result is the index
+    // of the first `true` in this list. If none matches, then the result is the
+    // length of the list, which is mapped to `Invalid`.
+    constexpr bool TypeMatches[] = {std::same_as<IdT, Types>...};
+    constexpr int Index =
+        std::find(TypeMatches, TypeMatches + NumTypes, true) - TypeMatches;
+    static_assert(Index != NumTypes || AllowInvalid,
+                  "Unexpected type passed to TypeEnum::For<...>");
+    return TypeEnum(static_cast<RawEnumType>(Index));
+  }
+
+  RawEnumType value_;
+};
+
+}  // namespace Carbon
+
+#endif  // CARBON_COMMON_TYPE_ENUM_H_

toolchain/base/kind_switch.h

@@ -186,7 +186,7 @@ constexpr auto CaseValueOfTypeInStdVariant =
 // Given `CARBON_KIND_SWITCH(value)` this returns the actual value to switch on.
 //
 // For types with a `kind()` accessor, this is the just the value of `kind()`.
-// The type returned from `kind()` is expected to be a `SemIR::TypeEnum`, as it
+// The type returned from `kind()` is expected to be a `TypeEnum`, as it
 // is required to have its API, including a nested `RawEnumType`.
 //
 // For std::variant<...> this is an enum synthesized from the types in the

toolchain/check/eval.cpp

@@ -679,8 +679,7 @@ using ArgHandlerFnT = auto(EvalContext& context, int32_t arg, Phase* phase)
 // Returns a lookup table to get constants by Id::Kind. Requires a null IdKind
 // as a parameter in order to get the type pack.
 template <typename... Types>
-static constexpr auto MakeArgHandlerTable(
-    SemIR::TypeEnum<Types...>* /*id_kind*/)
+static constexpr auto MakeArgHandlerTable(TypeEnum<Types...>* /*id_kind*/)
     -> std::array<ArgHandlerFnT*, SemIR::IdKind::NumValues> {
   std::array<ArgHandlerFnT*, SemIR::IdKind::NumValues> table = {};
   ((table[SemIR::IdKind::template For<Types>.ToIndex()] =

toolchain/sem_ir/BUILD

@@ -36,6 +36,7 @@ cc_library(
         "//common:check",
         "//common:enum_base",
         "//common:ostream",
+        "//common:type_enum",
         "//toolchain/base:index_base",
         "//toolchain/base:int",
         "//toolchain/base:value_ids",
@@ -162,6 +163,7 @@ cc_library(
         "//common:concepts",
         "//common:ostream",
         "//common:raw_string_ostream",
+        "//common:type_enum",
         "//toolchain/base:kind_switch",
         "//toolchain/base:shared_value_stores",
         "//toolchain/base:value_ids",

toolchain/sem_ir/formatter.h

@@ -318,8 +318,8 @@ class Formatter {
   // equivalent name formatting from InstNamer, although there are a few special
   // formats below.
   template <typename IdT>
-  // Force `InstId` children to use the `InstId` overload.
-    requires(!std::derived_from<IdT, InstId>)
+    requires(InstNamer::ScopeIdTypeEnum::Contains<IdT> ||
+             std::same_as<IdT, GenericId>)
   auto FormatName(IdT id) -> void {
     out_ << inst_namer_.GetNameFor(id);
   }

toolchain/sem_ir/id_kind.h

@@ -5,126 +5,12 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_ID_KIND_H_
 #define CARBON_TOOLCHAIN_SEM_IR_ID_KIND_H_
 
-#include <algorithm>
-
-#include "common/ostream.h"
+#include "common/type_enum.h"
 #include "toolchain/base/int.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::SemIR {
 
-// An enum whose values are the specified types.
-template <typename... Types>
-class TypeEnum : public Printable<TypeEnum<Types...>> {
- public:
-  using TypeTuple = std::tuple<Types...>;
-
-  static constexpr size_t NumTypes = sizeof...(Types);
-  static constexpr size_t NumValues = NumTypes + 2;
-
-  static_assert(NumValues <= 256, "Too many types for raw enum.");
-
-  // The underlying raw enumeration type.
-  //
-  // The enum_extensibility attribute indicates that this enum is intended to
-  // take values that do not correspond to its declared enumerators.
-  enum class [[clang::enum_extensibility(open)]] RawEnumType : uint8_t {
-    // The first sizeof...(Types) values correspond to the types.
-
-    // An explicitly invalid value.
-    Invalid = NumTypes,
-
-    // Indicates that no type should be used.
-    // TODO: This doesn't really fit the model of this type, but it's convenient
-    // for all of its users.
-    None,
-  };
-
-  // Accesses the type given an enum value.
-  template <RawEnumType K>
-    requires(K != RawEnumType::Invalid)
-  using TypeFor = __type_pack_element<static_cast<size_t>(K), Types...>;
-
-  // Workaround for Clang bug https://github.com/llvm/llvm-project/issues/85461
-  template <RawEnumType Value>
-  static constexpr auto FromRaw = TypeEnum(Value);
-
-  // Names for the `Invalid` and `None` enumeration values.
-  static constexpr const TypeEnum& Invalid = FromRaw<RawEnumType::Invalid>;
-  static constexpr const TypeEnum& None = FromRaw<RawEnumType::None>;
-
-  // Accesses the enumeration value for the type `IdT`. If `AllowInvalid` is
-  // set, any unexpected type is mapped to `Invalid`, otherwise an invalid type
-  // results in a compile error.
-  //
-  // The `Self` parameter is an implementation detail to allow `ForImpl` to be
-  // defined after this template, and should not be specified.
-  template <typename IdT, bool AllowInvalid = false, typename Self = TypeEnum>
-  static constexpr auto For = Self::template ForImpl<IdT, AllowInvalid>();
-
-  // This bool indicates whether the specified type corresponds to a value in
-  // this enum.
-  template <typename IdT>
-  static constexpr bool Contains = For<IdT, true>.is_valid();
-
-  // Explicitly convert from the raw enum type.
-  explicit constexpr TypeEnum(RawEnumType value) : value_(value) {}
-
-  // Implicitly convert to the raw enum type, for use in `switch`.
-  //
-  // 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 raw enum value.
-  constexpr auto ToRaw() const -> RawEnumType { return value_; }
-
-  // Returns a value that can be used as an array index. Returned value will be
-  // < NumValues.
-  constexpr auto ToIndex() const -> size_t {
-    return static_cast<size_t>(value_);
-  }
-
-  // Returns whether this is a valid value, not `Invalid`.
-  constexpr auto is_valid() const -> bool {
-    return value_ != RawEnumType::Invalid;
-  }
-
-  auto Print(llvm::raw_ostream& out) const -> void {
-    out << "IdKind(";
-    if (value_ == RawEnumType::None) {
-      out << "None";
-    } else {
-      static constexpr std::array<llvm::StringLiteral, sizeof...(Types)> Names =
-          {
-              Types::Label...,
-          };
-      out << Names[static_cast<int>(value_)];
-    }
-    out << ")";
-  }
-
- private:
-  // Translates a type to its enum value, or `Invalid`.
-  template <typename IdT, bool AllowInvalid>
-  static constexpr auto ForImpl() -> TypeEnum {
-    // A bool for each type saying whether it matches. The result is the index
-    // of the first `true` in this list. If none matches, then the result is the
-    // length of the list, which is mapped to `Invalid`.
-    constexpr bool TypeMatches[] = {std::same_as<IdT, Types>...};
-    constexpr int Index =
-        std::find(TypeMatches, TypeMatches + NumTypes, true) - TypeMatches;
-    static_assert(Index != NumTypes || AllowInvalid,
-                  "Unexpected type passed to TypeEnum::For<...>");
-    return TypeEnum(static_cast<RawEnumType>(Index));
-  }
-
-  RawEnumType value_;
-};
-
 // An enum of all the ID types used as instruction operands.
 //
 // As instruction operands, the types listed here can appear as fields of typed

toolchain/sem_ir/inst_namer.cpp

@@ -33,7 +33,7 @@ namespace Carbon::SemIR {
 InstNamer::InstNamer(const File* sem_ir) : sem_ir_(sem_ir) {
   insts_.resize(sem_ir->insts().size(), {ScopeId::None, Namespace::Name()});
   labels_.resize(sem_ir->inst_blocks().size());
-  scopes_.resize(static_cast<size_t>(GetScopeFor(NumberOfScopesTag())));
+  scopes_.resize(GetScopeIdOffset(ScopeIdTypeEnum::None));
   generic_scopes_.resize(sem_ir->generics().size(), ScopeId::None);
 
   // Build the constants scope.
@@ -122,6 +122,42 @@ InstNamer::InstNamer(const File* sem_ir) : sem_ir_(sem_ir) {
   }
 }
 
+auto InstNamer::GetScopeIdOffset(ScopeIdTypeEnum id_enum) const -> int {
+  int offset = 0;
+
+  // For each Id type, add the number of entities *above* its case; for example,
+  // the offset for functions excludes the functions themselves. The fallthrough
+  // handles summing to get uniqueness; order isn't special.
+  switch (id_enum) {
+    case ScopeIdTypeEnum::None:
+      // `None` will be getting a full count of scopes.
+      offset += sem_ir_->associated_constants().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<AssociatedConstantId>:
+      offset += sem_ir_->classes().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<ClassId>:
+      offset += sem_ir_->functions().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<FunctionId>:
+      offset += sem_ir_->impls().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<ImplId>:
+      offset += sem_ir_->interfaces().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<InterfaceId>:
+      offset += sem_ir_->specific_interfaces().size();
+      [[fallthrough]];
+    case ScopeIdTypeEnum::For<SpecificInterfaceId>:
+      // All type-specific scopes are offset by `FirstEntityScope`.
+      offset += static_cast<int>(ScopeId::FirstEntityScope);
+      return offset;
+
+    default:
+      CARBON_FATAL("Unexpected ScopeIdTypeEnum: {0}", id_enum);
+  }
+}
+
 auto InstNamer::GetScopeName(ScopeId scope) const -> std::string {
   switch (scope) {
     case ScopeId::None:

toolchain/sem_ir/inst_namer.h

@@ -5,6 +5,7 @@
 #ifndef CARBON_TOOLCHAIN_SEM_IR_INST_NAMER_H_
 #define CARBON_TOOLCHAIN_SEM_IR_INST_NAMER_H_
 
+#include "common/type_enum.h"
 #include "llvm/Support/raw_ostream.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/tree.h"
@@ -20,14 +21,18 @@ class InstNamer {
   // int32_t matches the input value size.
   enum class ScopeId : int32_t {
     None = -1,
+    // The three top-level scopes.
     File = 0,
     ImportRefs = 1,
     Constants = 2,
-    FirstFunction = 3,
+    // The first entity scope; see entities in `ScopeIdTypeEnum`.
+    FirstEntityScope = 3,
   };
-  static_assert(sizeof(ScopeId) == sizeof(FunctionId));
+  static_assert(sizeof(ScopeId) == sizeof(AnyIdBase));
 
-  struct NumberOfScopesTag {};
+  // Entities whose scopes get entries from `ScopeId`.
+  using ScopeIdTypeEnum = TypeEnum<AssociatedConstantId, ClassId, FunctionId,
+                                   ImplId, InterfaceId, SpecificInterfaceId>;
 
   // Construct the instruction namer, and assign names to all instructions in
   // the provided file.
@@ -36,33 +41,10 @@ class InstNamer {
   // Returns the scope ID corresponding to an ID of a function, class, or
   // interface.
   template <typename IdT>
+    requires ScopeIdTypeEnum::Contains<IdT>
   auto GetScopeFor(IdT id) const -> ScopeId {
-    auto index = static_cast<int32_t>(ScopeId::FirstFunction);
-
-    if constexpr (!std::same_as<FunctionId, IdT>) {
-      index += sem_ir_->functions().size();
-      if constexpr (!std::same_as<ClassId, IdT>) {
-        index += sem_ir_->classes().size();
-        if constexpr (!std::same_as<InterfaceId, IdT>) {
-          index += sem_ir_->interfaces().size();
-          if constexpr (!std::same_as<AssociatedConstantId, IdT>) {
-            index += sem_ir_->associated_constants().size();
-            if constexpr (!std::same_as<ImplId, IdT>) {
-              index += sem_ir_->impls().size();
-              if constexpr (!std::same_as<SpecificInterfaceId, IdT>) {
-                index += sem_ir_->specific_interfaces().size();
-                static_assert(std::same_as<NumberOfScopesTag, IdT>,
-                              "Unknown ID kind for scope");
-              }
-            }
-          }
-        }
-      }
-    }
-    if constexpr (!std::same_as<NumberOfScopesTag, IdT>) {
-      index += id.index;
-    }
-    return static_cast<ScopeId>(index);
+    return static_cast<ScopeId>(GetScopeIdOffset(ScopeIdTypeEnum::For<IdT>) +
+                                id.index);
   }
 
   // Returns the scope ID corresponding to a generic. A generic object shares
@@ -76,6 +58,7 @@ class InstNamer {
 
   // Returns the IR name to use for a function, class, or interface.
   template <typename IdT>
+    requires(ScopeIdTypeEnum::Contains<IdT> || std::same_as<IdT, GenericId>)
   auto GetNameFor(IdT id) const -> std::string {
     if (!id.has_value()) {
       return "invalid";
@@ -160,6 +143,11 @@ class InstNamer {
     return scopes_[static_cast<int>(scope_id)];
   }
 
+  // For the given `IdT`, returns its start offset in the `ScopeId` space. Each
+  // of `ScopeIdTypeEnum` is stored sequentially. When called with
+  // `ScopeIdTypeEnum::None`, returns the full count of scopes.
+  auto GetScopeIdOffset(ScopeIdTypeEnum id_enum) const -> int;
+
   auto AddBlockLabel(
       ScopeId scope_id, InstBlockId block_id, std::string name = "",
       std::variant<LocId, uint64_t> loc_id_or_fingerprint = LocId::None)