Remove SymbolicBinding step in TypeIterator (#7039)

TypeIterator has both SymbolicType and SymbolicBinding and these overlap
in their meaning. Clarify the API by removing SymbolicBinding and just
using SymbolicType for `SymbolicBinding` insts and when they are
converted to `type` to make a `SymbolicBindingType` inst. Add the
EntityNameId to the SymbolicType for when it is available, when the
instruction is just a simple reference to a binding.

---------

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>

toolchain/check/handle_require.cpp

@@ -117,8 +117,8 @@ static auto TypeStructureReferencesSelf(
           // Don't generate more diagnostics.
           return true;
         }
-        case CARBON_KIND(SemIR::TypeIterator::Step::SymbolicBinding bind): {
-          if (context.entity_names().Get(bind.entity_name_id).name_id ==
+        case CARBON_KIND(SemIR::TypeIterator::Step::SymbolicType symbolic): {
+          if (context.entity_names().Get(symbolic.entity_name_id).name_id ==
               SemIR::NameId::SelfType) {
             return true;
           }

toolchain/check/type_structure.cpp

@@ -229,10 +229,6 @@ auto TypeStructureBuilder::Build(SemIR::TypeIterator type_iter)
         AppendStructuralSymbolic();
         break;
       }
-      case CARBON_KIND(Step::SymbolicBinding _): {
-        AppendStructuralSymbolic();
-        break;
-      }
       case CARBON_KIND(Step::TemplateType _): {
         AppendStructuralSymbolic();
         break;

toolchain/sem_ir/type_iterator.cpp

@@ -44,7 +44,8 @@ auto TypeIterator::Next() -> Step {
         return Step::SymbolicValue{.inst_id = value.inst_id};
       }
       case CARBON_KIND(SymbolicType symbolic): {
-        return Step::SymbolicType{.facet_type_id = symbolic.facet_type_id};
+        return Step::SymbolicType{.entity_name_id = symbolic.entity_name_id,
+                                  .facet_type_id = symbolic.facet_type_id};
       }
       case CARBON_KIND(TypeId type_id): {
         if (auto step = ProcessTypeId(type_id)) {
@@ -65,9 +66,19 @@ auto TypeIterator::ProcessTypeId(TypeId type_id) -> std::optional<Step> {
   CARBON_KIND_SWITCH(inst) {
       // ==== Symbolic types ====
 
-    case SymbolicBinding::Kind:
-    case SymbolicBindingPattern::Kind: {
-      return Step::SymbolicType{.facet_type_id = type_id};
+    case CARBON_KIND(SymbolicBinding bind): {
+      return Step::SymbolicType{.entity_name_id = bind.entity_name_id,
+                                .facet_type_id = type_id};
+    }
+    case CARBON_KIND(SymbolicBindingPattern bind): {
+      return Step::SymbolicType{.entity_name_id = bind.entity_name_id,
+                                .facet_type_id = type_id};
+    }
+    case CARBON_KIND(SemIR::SymbolicBindingType bind): {
+      auto facet_type_id =
+          sem_ir_->insts().Get(bind.facet_value_inst_id).type_id();
+      return Step::SymbolicType{.entity_name_id = bind.entity_name_id,
+                                .facet_type_id = facet_type_id};
     }
 
     case Call::Kind:
@@ -78,14 +89,28 @@ auto TypeIterator::ProcessTypeId(TypeId type_id) -> std::optional<Step> {
     case CARBON_KIND(FacetAccessType access): {
       auto facet_type_id =
           sem_ir_->insts().Get(access.facet_value_inst_id).type_id();
-      return Step::SymbolicType{.facet_type_id = facet_type_id};
-    }
-    case CARBON_KIND(SemIR::SymbolicBindingType bind): {
-      return Step::SymbolicBinding{.entity_name_id = bind.entity_name_id};
+
+      auto entity_name_id = SemIR::EntityNameId::None;
+      if (auto facet_value = sem_ir_->insts().TryGetAs<SemIR::SymbolicBinding>(
+              access.facet_value_inst_id)) {
+        entity_name_id = facet_value->entity_name_id;
+      }
+
+      return Step::SymbolicType{.entity_name_id = entity_name_id,
+                                .facet_type_id = facet_type_id};
     }
+
     case CARBON_KIND(TupleAccess access): {
       auto facet_type_id = sem_ir_->insts().Get(access.tuple_id).type_id();
-      return Step::SymbolicType{.facet_type_id = facet_type_id};
+
+      auto entity_name_id = SemIR::EntityNameId::None;
+      if (auto facet_value = sem_ir_->insts().TryGetAs<SemIR::SymbolicBinding>(
+              access.tuple_id)) {
+        entity_name_id = facet_value->entity_name_id;
+      }
+
+      return Step::SymbolicType{.entity_name_id = entity_name_id,
+                                .facet_type_id = facet_type_id};
     }
 
       // ==== Concrete types ====
@@ -224,7 +249,13 @@ auto TypeIterator::TryGetInstIdAsTypeId(InstId inst_id) const
   // and thus does not match here.
   if (auto facet_type =
           sem_ir_->types().TryGetAs<FacetType>(type_id_of_inst_id)) {
-    return SymbolicType{.facet_type_id = type_id_of_inst_id};
+    auto entity_name_id = SemIR::EntityNameId::None;
+    if (auto bind =
+            sem_ir_->insts().TryGetAs<SemIR::SymbolicBinding>(inst_id)) {
+      entity_name_id = bind->entity_name_id;
+    }
+    return SymbolicType{.entity_name_id = entity_name_id,
+                        .facet_type_id = type_id_of_inst_id};
   }
   // Non-type values are concrete, only types are symbolic.
   if (type_id_of_inst_id != TypeType::TypeId) {

toolchain/sem_ir/type_iterator.h

@@ -59,6 +59,7 @@ class TypeIterator {
   struct EndType {};
   // A work item to mark a symbolic type.
   struct SymbolicType {
+    EntityNameId entity_name_id;
     TypeId facet_type_id;
   };
   // A work item to mark a concrete non-type value.
@@ -164,13 +165,12 @@ class TypeIterator::Step {
   };
   // A symbolic type value, constrained by `facet_type_id`.
   struct SymbolicType {
+    // If the symbolic type is simply a reference to a symbolic binding, this is
+    // the entity name of that binding. Otherwise, it is None.
+    EntityNameId entity_name_id;
     // Either a FacetType or the TypeType singleton.
     TypeId facet_type_id;
   };
-  // A symbolic type value, that comes from a binding named by `entity_name_id`.
-  struct SymbolicBinding {
-    EntityNameId entity_name_id;
-  };
   // A symbolic template type value.
   struct TemplateType {};
   // A concrete non-type value, which can be found as a generic parameter for a
@@ -201,12 +201,13 @@ class TypeIterator::Step {
   struct Error {};
 
   // Each step is one of these.
-  using Any = std::variant<
-      ConcreteType, SymbolicType, SymbolicBinding, TemplateType, ConcreteValue,
-      SymbolicValue, StructFieldName, ClassStartOnly, StructStartOnly,
-      TupleStartOnly, InterfaceStartOnly, ClassStart, StructStart, TupleStart,
-      InterfaceStart, IntStart, ArrayStart, ConstStart, MaybeUnformedStart,
-      PartialStart, PointerStart, End, Done, Error>;
+  using Any =
+      std::variant<ConcreteType, SymbolicType, TemplateType, ConcreteValue,
+                   SymbolicValue, StructFieldName, ClassStartOnly,
+                   StructStartOnly, TupleStartOnly, InterfaceStartOnly,
+                   ClassStart, StructStart, TupleStart, InterfaceStart,
+                   IntStart, ArrayStart, ConstStart, MaybeUnformedStart,
+                   PartialStart, PointerStart, End, Done, Error>;
 
   template <typename T>
   auto Is() const -> bool {