Allow using CARBON_KIND_SWITCH on a std::variant (#5433)

We use CARBON_KIND_SWITCH for handling the output of TypeIterator in the
TypeStructureBuilder

Here is how the errors look when it is misused:
- If you don't cover ever type in the variant with a case
  ```
Enumeration value 'VariantTypeT1NotHandledInSwitch' not handled in
switch
  ```
Is attached to the CARBON_KIND_SWITCH() usage, the `T1` being a 0-based
index into the std::variant's type list, indicating which type was
missed.
- If you have a case for a type that is not in the variant
  ```
In template: constraints not satisfied for class template
'ValidCaseType' [with T = char]
  ... bunch of instantiation stuff ...
kind_switch.h(124, 12): Because 'char' does not satisfy
'TypeFoundInVariant'
  ```
Where `char` was the type I put in the `CARBON_KIND` macro, which was
not in the variant.
- If you have too many types in your variant (currently > 12)
  ```
In template: static assertion failed due to requirement 'sizeof...(Ts)
<= 12': CARBON_KIND_SWITCH supports std::variant with up to 12 types.
Add more if needed.
  ```
  Is attached to the CARBON_KIND_SWITCH() usage.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

toolchain/base/BUILD

@@ -7,6 +7,14 @@ load("llvm_tools.bzl", "LLVM_MAIN_TOOLS", "generate_llvm_tools_def")
 
 package(default_visibility = ["//visibility:public"])
 
+cc_library(
+    name = "for_each_macro",
+    hdrs = ["for_each_macro.h"],
+    deps = [
+        "@llvm-project//llvm:Support",
+    ],
+)
+
 cc_library(
     name = "index_base",
     hdrs = ["index_base.h"],
@@ -20,10 +28,23 @@ cc_library(
     name = "kind_switch",
     hdrs = ["kind_switch.h"],
     deps = [
+        ":for_each_macro",
         "@llvm-project//llvm:Support",
     ],
 )
 
+cc_test(
+    name = "kind_switch_test",
+    size = "small",
+    srcs = ["kind_switch_test.cpp"],
+    deps = [
+        ":kind_switch",
+        "//common:raw_string_ostream",
+        "//testing/base:gtest_main",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "pretty_stack_trace_function",
     hdrs = ["pretty_stack_trace_function.h"],

toolchain/base/for_each_macro.h

@@ -0,0 +1,44 @@
+// 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_BASE_FOR_EACH_MACRO_H_
+#define CARBON_TOOLCHAIN_BASE_FOR_EACH_MACRO_H_
+
+/// CARBON_FOR_EACH() will apply `macro` to each argument in the variadic
+/// argument list, putting the output of `sep()` between each one.
+///
+/// The `sep` should be a function macro that returns a separator. Premade
+/// separataors are provided as CARBON_FOR_EACH_XYZ() macros.
+#define CARBON_FOR_EACH(macro, sep, ...)      \
+  __VA_OPT__(CARBON_INTERNAL_FOR_EACH_EXPAND( \
+      CARBON_INTERNAL_FOR_EACH(macro, sep, __VA_ARGS__)))
+
+#define CARBON_FOR_EACH_COMMA() ,
+#define CARBON_FOR_EACH_SEMI() ;
+#define CARBON_FOR_EACH_CONCAT()
+
+// Internal helpers
+
+#define CARBON_INTERNAL_FOR_EACH(macro, sep, a1, ...)                 \
+  macro(a1) __VA_OPT__(sep()) __VA_OPT__(                             \
+      CARBON_INTERNAL_FOR_EACH_AGAIN CARBON_INTERNAL_FOR_EACH_PARENS( \
+          macro, sep, __VA_ARGS__))
+#define CARBON_INTERNAL_FOR_EACH_PARENS ()
+#define CARBON_INTERNAL_FOR_EACH_AGAIN() CARBON_INTERNAL_FOR_EACH
+
+#define CARBON_INTERNAL_FOR_EACH_EXPAND(...)                             \
+  CARBON_INTERNAL_FOR_EACH_EXPAND1(                                      \
+      CARBON_INTERNAL_FOR_EACH_EXPAND1(CARBON_INTERNAL_FOR_EACH_EXPAND1( \
+          CARBON_INTERNAL_FOR_EACH_EXPAND1(__VA_ARGS__))))
+#define CARBON_INTERNAL_FOR_EACH_EXPAND1(...)                            \
+  CARBON_INTERNAL_FOR_EACH_EXPAND2(                                      \
+      CARBON_INTERNAL_FOR_EACH_EXPAND2(CARBON_INTERNAL_FOR_EACH_EXPAND2( \
+          CARBON_INTERNAL_FOR_EACH_EXPAND2(__VA_ARGS__))))
+#define CARBON_INTERNAL_FOR_EACH_EXPAND2(...)                            \
+  CARBON_INTERNAL_FOR_EACH_EXPAND3(                                      \
+      CARBON_INTERNAL_FOR_EACH_EXPAND3(CARBON_INTERNAL_FOR_EACH_EXPAND3( \
+          CARBON_INTERNAL_FOR_EACH_EXPAND3(__VA_ARGS__))))
+#define CARBON_INTERNAL_FOR_EACH_EXPAND3(...) __VA_ARGS__
+
+#endif  // CARBON_TOOLCHAIN_BASE_FOR_EACH_MACRO_H_

toolchain/base/kind_switch.h

@@ -8,6 +8,7 @@
 #include <type_traits>
 
 #include "llvm/ADT/STLExtras.h"
+#include "toolchain/base/for_each_macro.h"
 
 // This library provides switch-like behaviors for Carbon's kind-based types.
 //
@@ -39,29 +40,174 @@
 // requirements should change.
 namespace Carbon::Internal::Kind {
 
-// Given `CARBON_KIND_SWITCH(value)` this returns `value.kind()` to switch on.
+template <typename T>
+constexpr bool IsStdVariantValue = false;
+
+template <typename... Ts>
+constexpr bool IsStdVariantValue<std::variant<Ts...>> = true;
+
+template <typename T>
+concept IsStdVariant = IsStdVariantValue<std::decay_t<T>>;
+
+#define CARBON_INTERNAL_KIND_IDENTIFIER(name) T##name
+// Turns a list of numbers into a list `T0, T1, ...`.
+#define CARBON_INTERNAL_KIND_IDENTIFIERS(...)                             \
+  CARBON_FOR_EACH(CARBON_INTERNAL_KIND_IDENTIFIER, CARBON_FOR_EACH_COMMA, \
+                  __VA_ARGS__)
+
+#define CARBON_INTERNAL_KIND_TYPENAME(name) \
+  typename CARBON_INTERNAL_KIND_IDENTIFIER(name)
+// Turns a list of numbers into a list `typename T0, typename T1, ...`.
+#define CARBON_INTERNAL_KIND_TYPENAMES(...)                             \
+  CARBON_FOR_EACH(CARBON_INTERNAL_KIND_TYPENAME, CARBON_FOR_EACH_COMMA, \
+                  __VA_ARGS__)
+
+#define CARBON_INTERNAL_KIND_ENUM_NAME(n) VariantType##n##NotHandledInSwitch
+// Turns a list of numbers into a list `VariantType0NotHandledInSwitch, ...`.
+#define CARBON_INTERNAL_KIND_TYPES_TO_ENUM_NAMES(...)                    \
+  CARBON_FOR_EACH(CARBON_INTERNAL_KIND_ENUM_NAME, CARBON_FOR_EACH_COMMA, \
+                  __VA_ARGS__)
+
+// Turns a list of numbers into a set of template specializations of the
+// variable `EnumType EnumValue`, with each specialization having the Nth value
+// in the EnumType (as defined by CARBON_INTERNAL_KIND_TYPES_TO_ENUM_NAMES).
+#define CARBON_INTERNAL_KIND_TYPE_TO_ENUM_NAME(n)                    \
+  template <>                                                        \
+  constexpr EnumType EnumValue<CARBON_INTERNAL_KIND_IDENTIFIER(n)> = \
+      EnumType::CARBON_INTERNAL_KIND_ENUM_NAME(n)
+
+// Used to provide a reason in the compiler error from `ValidCaseType`, which
+// will state that "T does not satisfy TypeFoundInVariant".
+template <class T>
+concept TypeFoundInVariant = false;
+
+// Used to cause a compler error, which will state that "ValidCaseType was not
+// satisfied" for T and std::variant<...>.
+template <class T, class StdVariant>
+  requires TypeFoundInVariant<T>
+struct ValidCaseType;
+
+template <typename T>
+struct StdVariantTypeMap;
+
+#define CARBON_INTERNAL_KIND_TYPE_MAP(...)                                     \
+  template <CARBON_INTERNAL_KIND_TYPENAMES(__VA_ARGS__)>                       \
+  struct StdVariantTypeMap<                                                    \
+      std::variant<CARBON_INTERNAL_KIND_IDENTIFIERS(__VA_ARGS__)>> {           \
+    /* An enum with a value for each type in the std::variant. The switch will \
+     * be on this enum so that we get a warning if one of the enum values is   \
+     * not handled. They are named in a way to help explain the warning, that  \
+     * it means a type in the std::variant<...> type list does not have a      \
+     * matching case statement.                                                \
+     */                                                                        \
+    enum class EnumType {                                                      \
+      CARBON_INTERNAL_KIND_TYPES_TO_ENUM_NAMES(__VA_ARGS__)                    \
+    };                                                                         \
+    /* A mapping from a single type in the std::variant<...> type list to a    \
+     * value in the EnumType. This value is only used in the case a type is    \
+     * queried which is not part of the type list, and ValidCaseType is used   \
+     * to produce a diagnostic explaining the situation.  */                   \
+    template <typename Tn>                                                     \
+    static constexpr EnumType EnumValue = ValidCaseType<                       \
+        Tn, std::variant<CARBON_INTERNAL_KIND_IDENTIFIERS(__VA_ARGS__)>>();    \
+    /**/                                                                       \
+    CARBON_FOR_EACH(CARBON_INTERNAL_KIND_TYPE_TO_ENUM_NAME,                    \
+                    CARBON_FOR_EACH_SEMI, __VA_ARGS__);                        \
+  }
+
+template <typename... Ts>
+struct StdVariantTypeMap<std::variant<Ts...>> {
+  // The number here should match the number of arguments in the largest
+  // `CARBON_INTERNAL_KIND_TYPE_MAP` invocation below.
+  static_assert(sizeof...(Ts) <= 12,
+                "CARBON_KIND_SWITCH supports std::variant with up to 12 types. "
+                "Add more if needed.");
+};
+
+// Generate StdVariantTypeMap specializations for each number of types in the
+// std::variant<...> type list.
+CARBON_INTERNAL_KIND_TYPE_MAP(0);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6, 7);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6, 7, 8);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6, 7, 8, 9);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+CARBON_INTERNAL_KIND_TYPE_MAP(0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11);
+
+#undef CARBON_INTERNAL_KIND_IDENTIFIER
+#undef CARBON_INTERNAL_KIND_IDENTIFIERS
+#undef CARBON_INTERNAL_KIND_TYPENAME
+#undef CARBON_INTERNAL_KIND_TYPENAMES
+#undef CARBON_INTERNAL_KIND_ENUM_NAME
+#undef CARBON_INTERNAL_KIND_TYPES_TO_ENUM_NAMES
+#undef CARBON_INTERNAL_KIND_TYPE_TO_ENUM_NAME
+#undef CARBON_INTERNAL_KIND_TYPE_MAP
+
+// Uses the above `CARBON_INTERNAL_KIND_TYPE_MAP` expansions to make an enum
+// with a value for each type in a std::variant<...> type list.
+template <typename StdVariant>
+using StdVariantEnum = StdVariantTypeMap<std::decay_t<StdVariant>>::EnumType;
+
+// Uses the `CARBON_INTERNAL_KIND_TYPE_MAP` expanstions to find the enum value
+// in `StdVariantEnum` for a given type `T` in the type list of a
+// std::variant<...>.
+template <typename T, typename StdVariant>
+constexpr auto CaseValueOfTypeInStdVariant =
+    StdVariantTypeMap<std::decay_t<StdVariant>>::template EnumValue<T>;
+
+// 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
+// is required to have its API, including a nested `RawEnumType`.
+//
+// For std::variant<...> this is an enum synthesized from the types in the
+// variant's type list.
 template <typename SwitchT>
-auto SwitchOn(SwitchT&& switch_value) -> auto {
-  return switch_value.kind();
+constexpr auto SwitchOn(SwitchT&& switch_value) -> auto {
+  if constexpr (IsStdVariant<SwitchT>) {
+    return static_cast<StdVariantEnum<SwitchT>>(switch_value.index());
+  } else {
+    return switch_value.kind();
+  }
 }
 
-// Given `CARBON_KIND(CaseT name)` this generates `CaseT::Kind`. It explicitly
-// returns `KindT` because that may differ from `CaseT::Kind`, and may not be
-// copyable.
+// Given `CARBON_KIND(CaseT name)` this generates the case value to compare
+// against the switch value from `SwitchOn`.
+//
+// For types with a `kind()` accessor that returns a `TypeEnum`,
+// this gets the `TypeEnum<...>::RawTypeEnum` for the case type `CaseT`.
+//
+// For std::variant<...> this returns the value corresponding to the case type
+// from the enum synthesized (in `SwitchOn`) for the types in the variant's
+// type list.
 template <typename SwitchT, typename CaseFnT>
 consteval auto ForCase() -> auto {
-  using KindT = llvm::function_traits<
-      decltype(&std::remove_cvref_t<SwitchT>::kind)>::result_t;
   using CaseT = llvm::function_traits<CaseFnT>::template arg_t<0>;
-  return static_cast<KindT::RawEnumType>(KindT::template For<CaseT>);
+  if constexpr (IsStdVariant<SwitchT>) {
+    return CaseValueOfTypeInStdVariant<CaseT, SwitchT>;
+  } else {
+    using KindT = llvm::function_traits<
+        decltype(&std::remove_cvref_t<SwitchT>::kind)>::result_t;
+    return static_cast<KindT::RawEnumType>(KindT::template For<CaseT>);
+  }
 }
 
 // Given `CARBON_KIND_SWITCH(value)` and `CARBON_KIND(CaseT name)` this
 // generates `value.As<CaseT>()`.
-template <typename FnT, typename ValueT>
-auto Cast(ValueT&& kind_switch_value) -> auto {
-  using CaseT = llvm::function_traits<FnT>::template arg_t<0>;
-  return kind_switch_value.template As<CaseT>();
+template <typename CaseFnT, typename SwitchT>
+auto Cast(SwitchT&& kind_switch_value) -> decltype(auto) {
+  using CaseT = llvm::function_traits<CaseFnT>::template arg_t<0>;
+  if constexpr (IsStdVariant<SwitchT>) {
+    return std::get<CaseT>(kind_switch_value);
+  } else {
+    return kind_switch_value.template As<CaseT>();
+  }
 }
 
 #define CARBON_INTERNAL_KIND_MERGE(Prefix, Line) Prefix##Line

toolchain/base/kind_switch_test.cpp

@@ -0,0 +1,57 @@
+// 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/base/kind_switch.h"
+
+#include <gtest/gtest.h>
+
+#include <string>
+#include <variant>
+
+#include "common/raw_string_ostream.h"
+
+namespace Carbon {
+namespace {
+
+TEST(KindSwitch, Variant) {
+  auto f = [](std::variant<int, float, char> v) -> std::string {
+    RawStringOstream str;
+    CARBON_KIND_SWITCH(v) {
+      case CARBON_KIND(int n):
+        str << "int = " << n;
+        return str.TakeStr();
+      case CARBON_KIND(float f):
+        str << "float = " << f;
+        return str.TakeStr();
+      case CARBON_KIND(char c):
+        str << "char = " << c;
+        return str.TakeStr();
+    }
+  };
+
+  EXPECT_EQ(f(int{1}), "int = 1");
+  EXPECT_EQ(f(float{2}), "float = 2.000000e+00");
+  EXPECT_EQ(f(char{'h'}), "char = h");
+}
+
+TEST(KindSwitch, VariantUnusedValue) {
+  auto f = [](std::variant<int, float> v) -> std::string {
+    RawStringOstream str;
+    CARBON_KIND_SWITCH(v) {
+      case CARBON_KIND(int n):
+        str << "int = " << n;
+        return str.TakeStr();
+      case CARBON_KIND(float _):
+        // The float value is not used, we see that using `_` works.
+        str << "float";
+        return str.TakeStr();
+    }
+  };
+
+  EXPECT_EQ(f(int{1}), "int = 1");
+  EXPECT_EQ(f(float{2}), "float");
+}
+
+}  // namespace
+}  // namespace Carbon

toolchain/check/BUILD

@@ -110,7 +110,6 @@ cc_library(
         "//common:ostream",
         "//common:raw_string_ostream",
         "//common:set",
-        "//common:variant_helpers",
         "//common:vlog",
         "//toolchain/base:index_base",
         "//toolchain/base:kind_switch",

toolchain/check/type_structure.cpp

@@ -8,7 +8,7 @@
 #include <utility>
 #include <variant>
 
-#include "common/variant_helpers.h"
+#include "toolchain/base/kind_switch.h"
 #include "toolchain/check/context.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/type_iterator.h"
@@ -187,93 +187,84 @@ class TypeStructureBuilder {
 auto TypeStructureBuilder::Build(SemIR::TypeIterator type_iter)
     -> TypeStructure {
   while (true) {
-    auto step = type_iter.Next();
-    if (step.Is<SemIR::TypeIterator::Step::Done>()) {
-      break;
+    using Step = SemIR::TypeIterator::Step;
+    CARBON_KIND_SWITCH(type_iter.Next().any) {
+      case CARBON_KIND(Step::Done _):
+        // TODO: This requires 4 SmallVector moves (two here and two in the
+        // constructor). Find a way to reduce that.
+        return TypeStructure(std::exchange(structure_, {}),
+                             std::exchange(symbolic_type_indices_, {}),
+                             std::exchange(concrete_types_, {}));
+      case CARBON_KIND(Step::End _):
+        AppendStructuralConcreteCloseParen();
+        break;
+      case CARBON_KIND(Step::ConcreteType concrete):
+        AppendStructuralConcrete(concrete.type_id);
+        break;
+      case CARBON_KIND(Step::SymbolicType _):
+        AppendStructuralSymbolic();
+        break;
+      case CARBON_KIND(Step::TemplateType _):
+        AppendStructuralSymbolic();
+        break;
+      case CARBON_KIND(Step::ConcreteValue value):
+        AppendStructuralConcrete(
+            context_->constant_values().Get(value.inst_id));
+        break;
+      case CARBON_KIND(Step::SymbolicValue _):
+        AppendStructuralSymbolic();
+        break;
+      case CARBON_KIND(Step::StructFieldName field_name):
+        AppendStructuralConcrete(field_name.name_id);
+        break;
+      case CARBON_KIND(Step::StartOnly start):
+        CARBON_KIND_SWITCH(start.any) {
+          case CARBON_KIND(Step::ClassStart class_start):
+            AppendStructuralConcrete(class_start.class_id);
+            break;
+          case CARBON_KIND(Step::StructStart _):
+            AppendStructuralConcrete(TypeStructure::ConcreteStructType());
+            break;
+          case CARBON_KIND(Step::TupleStart _):
+            AppendStructuralConcrete(TypeStructure::ConcreteTupleType());
+            break;
+          case CARBON_KIND(Step::InterfaceStart interface_start):
+            AppendStructuralConcrete(interface_start.interface_id);
+            break;
+        }
+        break;
+      case CARBON_KIND(Step::StartWithEnd start_with_end): {
+        CARBON_KIND_SWITCH(start_with_end.any) {
+          case CARBON_KIND(Step::ClassStart class_start):
+            AppendStructuralConcreteOpenParen(class_start.class_id);
+            break;
+          case CARBON_KIND(Step::StructStart _):
+            AppendStructuralConcreteOpenParen(
+                TypeStructure::ConcreteStructType());
+            break;
+          case CARBON_KIND(Step::TupleStart _):
+            AppendStructuralConcreteOpenParen(
+                TypeStructure::ConcreteTupleType());
+            break;
+          case CARBON_KIND(Step::InterfaceStart interface_start):
+            AppendStructuralConcreteOpenParen(interface_start.interface_id);
+            break;
+          case CARBON_KIND(Step::IntStart int_start):
+            AppendStructuralConcreteOpenParen(int_start.type_id);
+            break;
+          case CARBON_KIND(Step::ArrayStart _):
+            AppendStructuralConcreteOpenParen(
+                TypeStructure::ConcreteArrayType());
+            break;
+          case CARBON_KIND(Step::PointerStart _):
+            AppendStructuralConcreteOpenParen(
+                TypeStructure::ConcretePointerType());
+            break;
+        }
+        break;
+      }
     }
-
-    VariantMatch(
-        step.any,
-        [&](SemIR::TypeIterator::Step::Done) {
-          CARBON_FATAL("already handled above");
-        },
-        [&](SemIR::TypeIterator::Step::End) {
-          AppendStructuralConcreteCloseParen();
-        },
-        [&](SemIR::TypeIterator::Step::ConcreteType concrete) {
-          AppendStructuralConcrete(concrete.type_id);
-        },
-        [&](SemIR::TypeIterator::Step::SymbolicType) {
-          AppendStructuralSymbolic();
-        },
-        [&](SemIR::TypeIterator::Step::TemplateType) {
-          AppendStructuralSymbolic();
-        },
-        [&](SemIR::TypeIterator::Step::ConcreteValue value) {
-          AppendStructuralConcrete(
-              context_->constant_values().Get(value.inst_id));
-        },
-        [&](SemIR::TypeIterator::Step::SymbolicValue) {
-          AppendStructuralSymbolic();
-        },
-        [&](SemIR::TypeIterator::Step::StructFieldName field_name) {
-          AppendStructuralConcrete(field_name.name_id);
-        },
-        [&](SemIR::TypeIterator::Step::StartOnly start) {
-          VariantMatch(
-              start.any,
-              [&](SemIR::TypeIterator::Step::ClassStart class_start) {
-                AppendStructuralConcrete(class_start.class_id);
-              },
-              [&](SemIR::TypeIterator::Step::StructStart) {
-                AppendStructuralConcrete(TypeStructure::ConcreteStructType());
-              },
-              [&](SemIR::TypeIterator::Step::TupleStart) {
-                AppendStructuralConcrete(TypeStructure::ConcreteTupleType());
-              },
-              [&](SemIR::TypeIterator::Step::InterfaceStart interface_start) {
-                AppendStructuralConcrete(interface_start.interface_id);
-              },
-              [&](SemIR::TypeIterator::Step::IntStart int_start) {
-                AppendStructuralConcrete(int_start.type_id);
-              });
-        },
-        [&](SemIR::TypeIterator::Step::StartWithEnd start_with_end) {
-          VariantMatch(
-              start_with_end.any,
-              [&](SemIR::TypeIterator::Step::ClassStart class_start) {
-                AppendStructuralConcreteOpenParen(class_start.class_id);
-              },
-              [&](SemIR::TypeIterator::Step::StructStart) {
-                AppendStructuralConcreteOpenParen(
-                    TypeStructure::ConcreteStructType());
-              },
-              [&](SemIR::TypeIterator::Step::TupleStart) {
-                AppendStructuralConcreteOpenParen(
-                    TypeStructure::ConcreteTupleType());
-              },
-              [&](SemIR::TypeIterator::Step::InterfaceStart interface_start) {
-                AppendStructuralConcreteOpenParen(interface_start.interface_id);
-              },
-              [&](SemIR::TypeIterator::Step::IntStart int_start) {
-                AppendStructuralConcreteOpenParen(int_start.type_id);
-              },
-              [&](SemIR::TypeIterator::Step::ArrayStart) {
-                AppendStructuralConcreteOpenParen(
-                    TypeStructure::ConcreteArrayType());
-              },
-              [&](SemIR::TypeIterator::Step::PointerStart) {
-                AppendStructuralConcreteOpenParen(
-                    TypeStructure::ConcretePointerType());
-              });
-        });
   }
-
-  // TODO: This requires 4 SmallVector moves (two here and two in the
-  // constructor). Find a way to reduce that.
-  return TypeStructure(std::exchange(structure_, {}),
-                       std::exchange(symbolic_type_indices_, {}),
-                       std::exchange(concrete_types_, {}));
 }
 
 auto BuildTypeStructure(Context& context, SemIR::InstId self_inst_id,