From 6cc08ae6e6de4a08e58b2ff02abe99160e50418f Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Thu, 9 Apr 2026 09:25:16 -0400 Subject: [PATCH] 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 --- toolchain/check/handle_require.cpp | 4 +-- toolchain/check/type_structure.cpp | 4 --- toolchain/sem_ir/type_iterator.cpp | 51 ++++++++++++++++++++++++------ toolchain/sem_ir/type_iterator.h | 21 ++++++------ 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/toolchain/check/handle_require.cpp b/toolchain/check/handle_require.cpp index 7a0db2d5fcc6..69bc39959489 100644 --- a/toolchain/check/handle_require.cpp +++ b/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; } diff --git a/toolchain/check/type_structure.cpp b/toolchain/check/type_structure.cpp index 8f87221aeca3..0da5b544c45d 100644 --- a/toolchain/check/type_structure.cpp +++ b/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; diff --git a/toolchain/sem_ir/type_iterator.cpp b/toolchain/sem_ir/type_iterator.cpp index 65f14dcd762a..6be953cc8fb5 100644 --- a/toolchain/sem_ir/type_iterator.cpp +++ b/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 { 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 { 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( + 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( + 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(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(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) { diff --git a/toolchain/sem_ir/type_iterator.h b/toolchain/sem_ir/type_iterator.h index 4c47f99b42f5..55679617a9a0 100644 --- a/toolchain/sem_ir/type_iterator.h +++ b/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; template auto Is() const -> bool {