diff --git a/toolchain/check/facet_type.cpp b/toolchain/check/facet_type.cpp index 183fc6793704..b4e02386d424 100644 --- a/toolchain/check/facet_type.cpp +++ b/toolchain/check/facet_type.cpp @@ -134,8 +134,8 @@ auto InitialFacetTypeImplWitness( // for it to use to attempt recovery. continue; } - auto rewrite_value = rewrite.rhs_id; - if (rewrite_value == SemIR::ErrorInst::SingletonInstId) { + auto rewrite_inst_id = rewrite.rhs_id; + if (rewrite_inst_id == SemIR::ErrorInst::SingletonInstId) { table_entry = SemIR::ErrorInst::SingletonInstId; continue; } @@ -165,9 +165,9 @@ auto InitialFacetTypeImplWitness( } if (table_entry != SemIR::ImplWitnessTablePlaceholder::SingletonInstId) { - if (table_entry != rewrite_value) { + if (table_entry != rewrite_inst_id) { // TODO: Figure out how to print the two different values - // `const_id` & `rewrite_value` in the diagnostic + // `const_id` & `rewrite_inst_id` in the diagnostic // message. CARBON_DIAGNOSTIC(AssociatedConstantWithDifferentValues, Error, "associated constant {0} given two different values", @@ -196,16 +196,16 @@ auto InitialFacetTypeImplWitness( // forming the facet type because the type of the associated constant // was symbolic. auto converted_inst_id = ConvertToValueOfType( - context, context.insts().GetLocId(facet_type_inst_id), rewrite_value, - assoc_const_type_id); + context, context.insts().GetLocId(facet_type_inst_id), + rewrite_inst_id, assoc_const_type_id); // Canonicalize the converted constant value. converted_inst_id = context.constant_values().GetConstantInstId(converted_inst_id); // The result of conversion can be non-constant even if the original // value was constant. if (converted_inst_id.has_value()) { - rewrite_value = converted_inst_id; - } else if (rewrite_value != SemIR::ErrorInst::SingletonInstId) { + rewrite_inst_id = converted_inst_id; + } else if (rewrite_inst_id != SemIR::ErrorInst::SingletonInstId) { const auto& assoc_const = context.associated_constants().Get( assoc_constant_decl->assoc_const_id); CARBON_DIAGNOSTIC( @@ -216,18 +216,18 @@ auto InitialFacetTypeImplWitness( context.emitter().Emit(facet_type_inst_id, AssociatedConstantNotConstantAfterConversion, assoc_const.name_id, assoc_const_type_id); - rewrite_value = SemIR::ErrorInst::SingletonInstId; + rewrite_inst_id = SemIR::ErrorInst::SingletonInstId; } } - CARBON_CHECK(rewrite_value == - context.constant_values().GetConstantInstId(rewrite_value), + CARBON_CHECK(rewrite_inst_id == context.constant_values().GetConstantInstId( + rewrite_inst_id), "Rewritten value for associated constant is not canonical."); table_entry = AddInst( context, witness_loc_id, - {.type_id = context.insts().Get(rewrite_value).type_id(), - .inst_id = rewrite_value}); + {.type_id = context.insts().Get(rewrite_inst_id).type_id(), + .inst_id = rewrite_inst_id}); } return witness_inst_id; } diff --git a/toolchain/sem_ir/facet_type_info.h b/toolchain/sem_ir/facet_type_info.h index 05c776b17ca8..69d56f4d6a82 100644 --- a/toolchain/sem_ir/facet_type_info.h +++ b/toolchain/sem_ir/facet_type_info.h @@ -33,7 +33,11 @@ struct FacetTypeInfo : Printable { // These are the required interfaces that are not lookup contexts. llvm::SmallVector self_impls_constraints; - // Rewrite constraints of the form `.T = U` + // Rewrite constraints of the form `.T = U`. + // + // The InstIds here must be canonical instructions (which come from the + // instruction in a constant value) in order to ensure comparison works + // correctly. struct RewriteConstraint { InstId lhs_id; InstId rhs_id;