Push GetFacetAsType from impl_lookup to custom_witness (#6520)

This delays the conversion of `query_self_const_id` until the actual
witness creation, mainly so that users of `BuildCustomWitness` don't
need to do extra work to ensure `GetFacetAsType` logic is
shared/applied. This is coming up for destroy logic.

toolchain/check/cpp/impl_lookup.cpp

@@ -23,9 +23,11 @@ namespace Carbon::Check {
 // If the given type is a C++ class type, returns the corresponding class
 // declaration. Otherwise returns nullptr.
 // TODO: Handle qualified types.
-static auto TypeAsClassDecl(Context& context, SemIR::TypeId type_id)
+static auto TypeAsClassDecl(Context& context,
+                            SemIR::ConstantId query_self_const_id)
     -> clang::CXXRecordDecl* {
-  auto class_type = context.types().TryGetAs<SemIR::ClassType>(type_id);
+  auto self_inst_id = context.constant_values().GetInstId(query_self_const_id);
+  auto class_type = context.insts().TryGetAs<SemIR::ClassType>(self_inst_id);
   if (!class_type) {
     // Not a class.
     return nullptr;
@@ -50,8 +52,8 @@ static auto TypeAsClassDecl(Context& context, SemIR::TypeId type_id)
 static auto BuildSingleFunctionWitness(
     Context& context, SemIR::LocId loc_id, clang::FunctionDecl* cpp_fn,
     clang::DeclAccessPair found_decl, int num_params,
-    SemIR::TypeId self_type_id, SemIR::SpecificInterface specific_interface)
-    -> SemIR::InstId {
+    SemIR::ConstantId query_self_const_id,
+    SemIR::SpecificInterface specific_interface) -> SemIR::InstId {
   auto fn_id = context.clang_sema().DiagnoseUseOfOverloadedDecl(
                    cpp_fn, GetCppLocation(context, loc_id))
                    ? SemIR::ErrorInst::InstId
@@ -63,15 +65,15 @@ static auto BuildSingleFunctionWitness(
     CARBON_CHECK(fn_id == SemIR::ErrorInst::InstId);
     return SemIR::ErrorInst::InstId;
   }
-  return BuildCustomWitness(context, loc_id, self_type_id, specific_interface,
-                            {fn_id});
+  return BuildCustomWitness(context, loc_id, query_self_const_id,
+                            specific_interface, {fn_id});
 }
 
 static auto LookupCopyImpl(Context& context, SemIR::LocId loc_id,
-                           SemIR::TypeId self_type_id,
+                           SemIR::ConstantId query_self_const_id,
                            SemIR::SpecificInterface specific_interface)
     -> SemIR::InstId {
-  auto* class_decl = TypeAsClassDecl(context, self_type_id);
+  auto* class_decl = TypeAsClassDecl(context, query_self_const_id);
   if (!class_decl) {
     // TODO: Should we also provide a `Copy` implementation for enumerations?
     return SemIR::InstId::None;
@@ -88,14 +90,14 @@ static auto LookupCopyImpl(Context& context, SemIR::LocId loc_id,
   return BuildSingleFunctionWitness(
       context, loc_id, ctor,
       clang::DeclAccessPair::make(ctor, ctor->getAccess()), /*num_params=*/1,
-      self_type_id, specific_interface);
+      query_self_const_id, specific_interface);
 }
 
 static auto LookupDestroyImpl(Context& context, SemIR::LocId loc_id,
-                              SemIR::TypeId self_type_id,
+                              SemIR::ConstantId query_self_const_id,
                               SemIR::SpecificInterface specific_interface)
     -> SemIR::InstId {
-  auto* class_decl = TypeAsClassDecl(context, self_type_id);
+  auto* class_decl = TypeAsClassDecl(context, query_self_const_id);
   if (!class_decl) {
     return SemIR::InstId::None;
   }
@@ -110,20 +112,22 @@ static auto LookupDestroyImpl(Context& context, SemIR::LocId loc_id,
   return BuildSingleFunctionWitness(
       context, loc_id, dtor,
       clang::DeclAccessPair::make(dtor, dtor->getAccess()), /*num_params=*/0,
-      self_type_id, specific_interface);
+      query_self_const_id, specific_interface);
 }
 
 auto LookupCppImpl(Context& context, SemIR::LocId loc_id,
-                   SemIR::TypeId self_type_id, CoreInterface core_interface,
+                   CoreInterface core_interface,
+                   SemIR::ConstantId query_self_const_id,
                    SemIR::SpecificInterface specific_interface,
                    const TypeStructure* best_impl_type_structure,
                    SemIR::LocId best_impl_loc_id) -> SemIR::InstId {
   // TODO: Handle other interfaces.
   switch (core_interface) {
     case CoreInterface::Copy:
-      return LookupCopyImpl(context, loc_id, self_type_id, specific_interface);
+      return LookupCopyImpl(context, loc_id, query_self_const_id,
+                            specific_interface);
     case CoreInterface::Destroy:
-      return LookupDestroyImpl(context, loc_id, self_type_id,
+      return LookupDestroyImpl(context, loc_id, query_self_const_id,
                                specific_interface);
 
     case CoreInterface::Unknown:

toolchain/check/cpp/impl_lookup.h

@@ -35,7 +35,8 @@ namespace Carbon::Check {
 // type structure, and can be `None` if `best_impl_type_structure` is null. This
 // parameter is used only for ambiguity diagnostics.
 auto LookupCppImpl(Context& context, SemIR::LocId loc_id,
-                   SemIR::TypeId self_type_id, CoreInterface core_interface,
+                   CoreInterface core_interface,
+                   SemIR::ConstantId query_self_const_id,
                    SemIR::SpecificInterface specific_interface,
                    const TypeStructure* best_impl_type_structure,
                    SemIR::LocId best_impl_loc_id) -> SemIR::InstId;

toolchain/check/custom_witness.cpp

@@ -12,8 +12,28 @@
 
 namespace Carbon::Check {
 
+// Given a value whose type `IsFacetTypeOrError`, returns the corresponding
+// type.
+static auto GetFacetAsType(Context& context, SemIR::LocId loc_id,
+                           SemIR::ConstantId facet_or_type_const_id)
+    -> SemIR::TypeId {
+  auto facet_or_type_id =
+      context.constant_values().GetInstId(facet_or_type_const_id);
+  auto type_type_id = context.insts().Get(facet_or_type_id).type_id();
+  CARBON_CHECK(context.types().IsFacetTypeOrError(type_type_id));
+
+  if (context.types().Is<SemIR::FacetType>(type_type_id)) {
+    // It's a facet; access its type.
+    facet_or_type_id = GetOrAddInst<SemIR::FacetAccessType>(
+        context, loc_id,
+        {.type_id = SemIR::TypeType::TypeId,
+         .facet_value_inst_id = facet_or_type_id});
+  }
+  return context.types().GetTypeIdForTypeInstId(facet_or_type_id);
+}
+
 auto BuildCustomWitness(Context& context, SemIR::LocId loc_id,
-                        SemIR::TypeId self_type_id,
+                        SemIR::ConstantId query_self_const_id,
                         SemIR::SpecificInterface specific_interface,
                         llvm::ArrayRef<SemIR::InstId> values) -> SemIR::InstId {
   const auto& interface =
@@ -64,6 +84,8 @@ auto BuildCustomWitness(Context& context, SemIR::LocId loc_id,
         if (struct_value.type_id == SemIR::ErrorInst::TypeId) {
           return SemIR::ErrorInst::InstId;
         }
+        auto self_type_id =
+            GetFacetAsType(context, loc_id, query_self_const_id);
         // TODO: If a thunk is needed, this will build a different value each
         // time it's called, so we won't properly deduplicate repeated
         // witnesses.

toolchain/check/custom_witness.h

@@ -15,7 +15,7 @@ namespace Carbon::Check {
 // lookup result. Produces a diagnostic and returns `None` if the specified
 // values aren't suitable for the interface.
 auto BuildCustomWitness(Context& context, SemIR::LocId loc_id,
-                        SemIR::TypeId self_type_id,
+                        SemIR::ConstantId query_self_const_id,
                         SemIR::SpecificInterface specific_interface,
                         llvm::ArrayRef<SemIR::InstId> values) -> SemIR::InstId;
 

toolchain/check/impl_lookup.cpp

@@ -896,26 +896,6 @@ static auto CollectCandidateImplsForQuery(
   return candidates;
 }
 
-// Given a value whose type `IsFacetTypeOrError`, returns the corresponding
-// type.
-static auto GetFacetAsType(Context& context, SemIR::LocId loc_id,
-                           SemIR::ConstantId facet_or_type_const_id)
-    -> SemIR::TypeId {
-  auto facet_or_type_id =
-      context.constant_values().GetInstId(facet_or_type_const_id);
-  auto type_type_id = context.insts().Get(facet_or_type_id).type_id();
-  CARBON_CHECK(context.types().IsFacetTypeOrError(type_type_id));
-
-  if (context.types().Is<SemIR::FacetType>(type_type_id)) {
-    // It's a facet; access its type.
-    facet_or_type_id = GetOrAddInst<SemIR::FacetAccessType>(
-        context, loc_id,
-        {.type_id = SemIR::TypeType::TypeId,
-         .facet_value_inst_id = facet_or_type_id});
-  }
-  return context.types().GetTypeIdForTypeInstId(facet_or_type_id);
-}
-
 // Record the query which found a final impl witness. It's illegal to
 // write a final impl afterward that would match the same query.
 static auto PoisonImplLookupQuery(Context& context, SemIR::LocId loc_id,
@@ -1060,9 +1040,9 @@ auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id,
       // Also check for a C++ candidate that is a better match than whatever
       // `impl` we may have found in Carbon.
       auto cpp_witness_id = LookupCppImpl(
-          context, loc_id, GetFacetAsType(context, loc_id, query_self_const_id),
-          core_interface, query_specific_interface,
-          lookup_result.impl_type_structure, lookup_result.impl_loc_id);
+          context, loc_id, core_interface, query_self_const_id,
+          query_specific_interface, lookup_result.impl_type_structure,
+          lookup_result.impl_loc_id);
       if (cpp_witness_id.has_value()) {
         lookup_result = {.result =
                              EvalImplLookupResult::MakeFinal(cpp_witness_id)};