Skip to content

Make BindSymbolicName the canonical form of a FacetValue wrapping the BindSymbolicName #6107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 23, 2025
+2,239 −2,641
Merged

Make BindSymbolicName the canonical form of a FacetValue wrapping the BindSymbolicName #6107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
66a28e9
facetaccesstype-roundtrip
danakj Sep 19, 2025
b127990
canonicalize-facet-value-of-binding
danakj Sep 23, 2025
f74e5da
rephrase comment as shortcut again
danakj Sep 23, 2025
8bb4261
rephrase
danakj Sep 23, 2025
47813ad
limit-semir
danakj Sep 23, 2025
53d327f
comment
danakj Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions toolchain/check/convert.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1211,11 +1211,17 @@ static auto PerformBuiltinConversion(

if (auto facet_access_type_inst =
sem_ir.insts().TryGetAs<SemIR::FacetAccessType>(const_value_id)) {
// Conversion from a `FacetAccessType` to a `FacetValue` of the target
// `FacetType` if the instruction in the `FacetAccessType` is of a
// `FacetType` that satisfies the requirements of the target `FacetType`.
// If the `FacetType` exactly matches the target `FacetType` then we can
// shortcut and use that value, and avoid impl lookup.
// Shortcut for lossless round trips through a FacetAccessType when
// converting back to the type of its original facet value.
//
// In the case where the FacetAccessType wraps a BindSymbolicName with the
// exact facet type that we are converting to, the resulting FacetValue
// would evaluate back to the original BindSymbolicName as its canonical
// form. We can skip past the whole impl lookup step then and do that
// here.
//
// See also test:
// facet_access_type_converts_back_to_original_facet_value.carbon
auto facet_value_inst_id = facet_access_type_inst->facet_value_inst_id;
if (sem_ir.insts().Get(facet_value_inst_id).type_id() == target.type_id) {
return facet_value_inst_id;
Expand Down
37 changes: 29 additions & 8 deletions toolchain/check/eval_inst.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,14 @@ auto EvalConstantInst(Context& context, SemIR::Converted inst)
context.constant_values().Get(inst.result_id));
}

// TODO: This should not be necessary since the constant kind is
// WheneverPossible.
auto EvalConstantInst(Context& /*context*/, SemIR::CppOverloadSetValue inst)
-> ConstantEvalResult {
return ConstantEvalResult::NewSamePhase(SemIR::StructValue{
.type_id = inst.type_id, .elements_id = SemIR::InstBlockId::Empty});
}

auto EvalConstantInst(Context& /*context*/, SemIR::Deref /*inst*/)
-> ConstantEvalResult {
// TODO: Handle this.
Expand All @@ -203,21 +211,34 @@ auto EvalConstantInst(Context& context, SemIR::FacetAccessType inst)
return ConstantEvalResult::NewSamePhase(inst);
}

auto EvalConstantInst(Context& context, SemIR::FacetValue inst)
-> ConstantEvalResult {
// A FacetValue that just wraps a BindSymbolicName without adding/removing any
// witnesses is evaluated back to the BindSymbolicName itself.
if (auto access =
context.insts().TryGetAs<SemIR::FacetAccessType>(inst.type_inst_id)) {
auto bind_id = access->facet_value_inst_id;
auto bind = context.insts().Get(bind_id);
if (bind.Is<SemIR::BindSymbolicName>()) {
// If the FacetTypes are the same, then the FacetValue didn't add/remove
// any witnesses.
if (bind.type_id() == inst.type_id) {
return ConstantEvalResult::Existing(
context.constant_values().Get(bind_id));
}
}
}

return ConstantEvalResult::NewSamePhase(inst);
}

auto EvalConstantInst(Context& context, SemIR::InstId inst_id,
SemIR::FloatType inst) -> ConstantEvalResult {
return ValidateFloatTypeAndSetKind(context, SemIR::LocId(inst_id), inst)
? ConstantEvalResult::NewSamePhase(inst)
: ConstantEvalResult::Error;
}

// TODO: This should not be necessary since the constant kind is
// WheneverPossible.
auto EvalConstantInst(Context& /*context*/, SemIR::CppOverloadSetValue inst)
-> ConstantEvalResult {
return ConstantEvalResult::NewSamePhase(SemIR::StructValue{
.type_id = inst.type_id, .elements_id = SemIR::InstBlockId::Empty});
}

auto EvalConstantInst(Context& /*context*/, SemIR::FunctionDecl inst)
-> ConstantEvalResult {
// A function declaration evaluates to a function object, which is an empty
Expand Down
4 changes: 2 additions & 2 deletions toolchain/check/testdata/alias/export_name.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ var d: D* = &c;
// CHECK:STDOUT: %Copy.impl_witness.999: <witness> = impl_witness imports.%Copy.impl_witness_table.53c, @ptr.as.Copy.impl(%C) [concrete]
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.type.c3f: type = fn_type @ptr.as.Copy.impl.Op, @ptr.as.Copy.impl(%C) [concrete]
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.fb8: %ptr.as.Copy.impl.Op.type.c3f = struct_value () [concrete]
// CHECK:STDOUT: %Copy.facet.9e3: %Copy.type = facet_value %ptr.019, (%Copy.impl_witness.999) [concrete]
// CHECK:STDOUT: %.7e9: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet.9e3 [concrete]
// CHECK:STDOUT: %Copy.facet: %Copy.type = facet_value %ptr.019, (%Copy.impl_witness.999) [concrete]
// CHECK:STDOUT: %.7e9: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet [concrete]
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.bound: <bound method> = bound_method %addr, %ptr.as.Copy.impl.Op.fb8 [concrete]
// CHECK:STDOUT: %ptr.as.Copy.impl.Op.specific_fn: <specific function> = specific_function %ptr.as.Copy.impl.Op.fb8, @ptr.as.Copy.impl.Op(%C) [concrete]
// CHECK:STDOUT: %bound_method: <bound method> = bound_method %addr, %ptr.as.Copy.impl.Op.specific_fn [concrete]
Expand Down
4 changes: 2 additions & 2 deletions toolchain/check/testdata/array/import.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ fn F() -> array(i32, 1) {
// CHECK:STDOUT: %Copy.impl_witness.a32: <witness> = impl_witness imports.%Copy.impl_witness_table.1ed, @Int.as.Copy.impl(%int_32) [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.type.276: type = fn_type @Int.as.Copy.impl.Op, @Int.as.Copy.impl(%int_32) [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.f59: %Int.as.Copy.impl.Op.type.276 = struct_value () [concrete]
// CHECK:STDOUT: %Copy.facet.c49: %Copy.type = facet_value %i32, (%Copy.impl_witness.a32) [concrete]
// CHECK:STDOUT: %.7fa: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet.c49 [concrete]
// CHECK:STDOUT: %Copy.facet: %Copy.type = facet_value %i32, (%Copy.impl_witness.a32) [concrete]
// CHECK:STDOUT: %.7fa: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.specific_fn: <specific function> = specific_function %Int.as.Copy.impl.Op.f59, @Int.as.Copy.impl.Op(%int_32) [concrete]
// CHECK:STDOUT: %type_where: type = facet_type <type where .Self impls <CanAggregateDestroy>> [concrete]
// CHECK:STDOUT: %facet_value: %type_where = facet_value %array_type, () [concrete]
Expand Down
4 changes: 2 additions & 2 deletions toolchain/check/testdata/array/index_not_literal.carbon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ fn F(a: array({}, 3)) -> {} {
// CHECK:STDOUT: %Copy.impl_witness.a32: <witness> = impl_witness imports.%Copy.impl_witness_table.1ed, @Int.as.Copy.impl(%int_32) [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.type.276: type = fn_type @Int.as.Copy.impl.Op, @Int.as.Copy.impl(%int_32) [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.f59: %Int.as.Copy.impl.Op.type.276 = struct_value () [concrete]
// CHECK:STDOUT: %Copy.facet.c49: %Copy.type = facet_value %i32, (%Copy.impl_witness.a32) [concrete]
// CHECK:STDOUT: %.7fa: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet.c49 [concrete]
// CHECK:STDOUT: %Copy.facet: %Copy.type = facet_value %i32, (%Copy.impl_witness.a32) [concrete]
// CHECK:STDOUT: %.7fa: type = fn_type_with_self_type %Copy.Op.type, %Copy.facet [concrete]
// CHECK:STDOUT: %Int.as.Copy.impl.Op.specific_fn: <specific function> = specific_function %Int.as.Copy.impl.Op.f59, @Int.as.Copy.impl.Op(%int_32) [concrete]
// CHECK:STDOUT: %int_1.5b8: Core.IntLiteral = int_value 1 [concrete]
// CHECK:STDOUT: %int_2.ecc: Core.IntLiteral = int_value 2 [concrete]
Expand Down
Loading
Loading