From 888de54b89d433e799ccb7b9289d8b2f12f653e4 Mon Sep 17 00:00:00 2001 From: danakj Date: Tue, 23 Sep 2025 10:33:43 -0400 Subject: [PATCH 1/4] mask-facet-value --- toolchain/check/impl_lookup.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index 6d8ff2fbd1d88..983d07f04cbcd 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -547,6 +547,17 @@ static auto TypeCanDestroy(Context& context, SemIR::ConstantId query_self_const_id) -> bool { auto inst = context.insts().Get( context.constant_values().GetInstId(query_self_const_id)); + + // For facet values, look if the FacetType provides the same. + if (auto facet_type = + context.types().TryGetAs(inst.type_id())) { + const auto& info = context.facet_types().Get(facet_type->facet_type_id); + if (info.builtin_constraint_mask.HasAnyOf( + SemIR::BuiltinConstraintMask::TypeCanDestroy)) { + return true; + } + } + CARBON_KIND_SWITCH(inst) { case CARBON_KIND(SemIR::ClassType class_type): { auto class_info = context.classes().Get(class_type.class_id); From 644e07318d117a5422912e2a5c1f4e91199fe7ca Mon Sep 17 00:00:00 2001 From: danakj Date: Tue, 23 Sep 2025 10:43:22 -0400 Subject: [PATCH 2/4] add-test --- .../testdata/builtins/type/can_destroy.carbon | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/toolchain/check/testdata/builtins/type/can_destroy.carbon b/toolchain/check/testdata/builtins/type/can_destroy.carbon index 78f25d45223b6..220262186bda8 100644 --- a/toolchain/check/testdata/builtins/type/can_destroy.carbon +++ b/toolchain/check/testdata/builtins/type/can_destroy.carbon @@ -38,6 +38,28 @@ fn G() { //@dump-sem-ir-end } +// --- symbolic.carbon +library "[[@TEST_NAME]]"; + +interface Z {} + +fn CanDestroy() -> type = "type.can_destroy"; + +fn G(U:! CanDestroy()) {} + +fn F(T:! Z & CanDestroy()) { + // Requires a conversion (and thus impl lookup into `T`) since `T` and `U` + // have different (but compatible) facet types. + // CHECK:STDERR: symbolic.carbon:[[@LINE+7]]:3: error: cannot convert type `T` that implements `Z where .Self impls Core.CanDestroy` into type implementing `type where .Self impls Core.CanDestroy` [ConversionFailureFacetToFacet] + // CHECK:STDERR: G(T); + // CHECK:STDERR: ^~~~ + // CHECK:STDERR: symbolic.carbon:[[@LINE-8]]:6: note: initializing generic parameter `U` declared here [InitializingGenericParam] + // CHECK:STDERR: fn G(U:! CanDestroy()) {} + // CHECK:STDERR: ^ + // CHECK:STDERR: + G(T); +} + // --- fail_incomplete.carbon library "[[@TEST_NAME]]"; From 7694d0275db79cfa12e42217a6ec954ce4b2122e Mon Sep 17 00:00:00 2001 From: danakj Date: Thu, 2 Oct 2025 14:09:20 -0400 Subject: [PATCH 3/4] review --- toolchain/check/convert.cpp | 3 --- toolchain/check/eval_inst.cpp | 14 ++++++-------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 060aca8c49926..701974a413fb5 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -1225,9 +1225,6 @@ static auto PerformBuiltinConversion( // 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 - // // TODO: This instruction is going to become a `SymbolicBindingType`, so // we'll need to handle that instead. auto const_type_inst_id = diff --git a/toolchain/check/eval_inst.cpp b/toolchain/check/eval_inst.cpp index eab04d69cd5ac..3eb83ad5ed43d 100644 --- a/toolchain/check/eval_inst.cpp +++ b/toolchain/check/eval_inst.cpp @@ -210,14 +210,12 @@ auto EvalConstantInst(Context& context, SemIR::FacetValue inst) if (auto access = context.insts().TryGetAs(inst.type_inst_id)) { auto bind_id = access->facet_value_inst_id; - auto bind = context.insts().Get(bind_id); - if (bind.Is()) { - // 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)); - } + auto bind = context.insts().TryGetAs(bind_id); + // If the FacetTypes are the same, then the FacetValue didn't add/remove + // any witnesses. + if (bind.has_value() && bind->type_id == inst.type_id) { + return ConstantEvalResult::Existing( + context.constant_values().Get(bind_id)); } } From 14ae9883c002dd2fb7a700938c7d3313e9c51754 Mon Sep 17 00:00:00 2001 From: danakj Date: Thu, 2 Oct 2025 14:38:38 -0400 Subject: [PATCH 4/4] re-fix --- toolchain/check/impl_lookup.cpp | 4 ++-- toolchain/check/testdata/builtins/type/can_destroy.carbon | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index 983d07f04cbcd..6f95091600a43 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -545,8 +545,8 @@ static auto GetOrAddLookupImplWitness(Context& context, SemIR::LocId loc_id, // Returns true if the `Self` should impl `Destroy`. static auto TypeCanDestroy(Context& context, SemIR::ConstantId query_self_const_id) -> bool { - auto inst = context.insts().Get( - context.constant_values().GetInstId(query_self_const_id)); + auto inst = context.insts().Get(context.constant_values().GetInstId( + GetCanonicalizedFacetOrTypeValue(context, query_self_const_id))); // For facet values, look if the FacetType provides the same. if (auto facet_type = diff --git a/toolchain/check/testdata/builtins/type/can_destroy.carbon b/toolchain/check/testdata/builtins/type/can_destroy.carbon index 220262186bda8..e833ec110906d 100644 --- a/toolchain/check/testdata/builtins/type/can_destroy.carbon +++ b/toolchain/check/testdata/builtins/type/can_destroy.carbon @@ -50,13 +50,6 @@ fn G(U:! CanDestroy()) {} fn F(T:! Z & CanDestroy()) { // Requires a conversion (and thus impl lookup into `T`) since `T` and `U` // have different (but compatible) facet types. - // CHECK:STDERR: symbolic.carbon:[[@LINE+7]]:3: error: cannot convert type `T` that implements `Z where .Self impls Core.CanDestroy` into type implementing `type where .Self impls Core.CanDestroy` [ConversionFailureFacetToFacet] - // CHECK:STDERR: G(T); - // CHECK:STDERR: ^~~~ - // CHECK:STDERR: symbolic.carbon:[[@LINE-8]]:6: note: initializing generic parameter `U` declared here [InitializingGenericParam] - // CHECK:STDERR: fn G(U:! CanDestroy()) {} - // CHECK:STDERR: ^ - // CHECK:STDERR: G(T); }