Refactor LookupCopyImpl and LookupDestroyImpl to share logic. (#6649)

Assisted-by: Google Antigravity with Gemini 3 Flash

toolchain/check/cpp/impl_lookup.cpp

@@ -49,97 +49,74 @@ static auto TypeAsClassDecl(Context& context,
       context.clang_decls().Get(decl_id).key.decl);
 }
 
-static auto BuildSingleFunctionWitness(
-    Context& context, SemIR::LocId loc_id, clang::FunctionDecl* cpp_fn,
-    clang::DeclAccessPair found_decl, int num_params,
-    SemIR::ConstantId query_self_const_id,
-    SemIR::SpecificInterfaceId query_specific_interface_id) -> SemIR::InstId {
-  auto fn_id = context.clang_sema().DiagnoseUseOfOverloadedDecl(
-                   cpp_fn, GetCppLocation(context, loc_id))
-                   ? SemIR::ErrorInst::InstId
-                   : ImportCppFunctionDecl(context, loc_id, cpp_fn, num_params);
-  if (auto fn_decl =
-          context.insts().TryGetAsWithId<SemIR::FunctionDecl>(fn_id)) {
-    CheckCppOverloadAccess(context, loc_id, found_decl, fn_decl->inst_id);
-  } else {
-    CARBON_CHECK(fn_id == SemIR::ErrorInst::InstId);
-    return SemIR::ErrorInst::InstId;
-  }
-  return BuildCustomWitness(context, loc_id, query_self_const_id,
-                            query_specific_interface_id, {fn_id});
-}
-
-static auto LookupCopyImpl(
-    Context& context, SemIR::LocId loc_id,
-    SemIR::ConstantId query_self_const_id,
-    SemIR::SpecificInterfaceId query_specific_interface_id) -> SemIR::InstId {
-  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;
-  }
+namespace {
+// See `GetDeclForCoreInterface`.
+struct DeclInfo {
+  clang::NamedDecl* decl;
+  int num_params;
+};
+}  // namespace
+
+// Retrieves a `core_interface`'s corresponding `NamedDecl`, also with the
+// expected number of parameters. May return a null decl.
+auto GetDeclForCoreInterface(clang::Sema& clang_sema,
+                             CoreInterface core_interface,
+                             clang::CXXRecordDecl* class_decl) -> DeclInfo {
+  // TODO: Handle other interfaces.
 
-  auto* ctor = context.clang_sema().LookupCopyingConstructor(
-      class_decl, clang::Qualifiers::Const);
-  if (!ctor) {
-    // TODO: If the impl lookup failure is an error, we should produce a
-    // diagnostic explaining why the class is not copyable.
-    return SemIR::InstId::None;
+  switch (core_interface) {
+    case CoreInterface::Copy:
+      return {.decl = clang_sema.LookupCopyingConstructor(
+                  class_decl, clang::Qualifiers::Const),
+              .num_params = 1};
+    case CoreInterface::Destroy:
+      return {.decl = clang_sema.LookupDestructor(class_decl), .num_params = 0};
+    case CoreInterface::Unknown:
+      CARBON_FATAL("shouldn't be called with `Unknown`");
   }
-
-  return BuildSingleFunctionWitness(
-      context, loc_id, ctor,
-      clang::DeclAccessPair::make(ctor, ctor->getAccess()), /*num_params=*/1,
-      query_self_const_id, query_specific_interface_id);
 }
 
-static auto LookupDestroyImpl(
-    Context& context, SemIR::LocId loc_id,
-    SemIR::ConstantId query_self_const_id,
-    SemIR::SpecificInterfaceId query_specific_interface_id) -> SemIR::InstId {
+auto LookupCppImpl(Context& context, SemIR::LocId loc_id,
+                   CoreInterface core_interface,
+                   SemIR::ConstantId query_self_const_id,
+                   SemIR::SpecificInterfaceId query_specific_interface_id,
+                   const TypeStructure* best_impl_type_structure,
+                   SemIR::LocId best_impl_loc_id) -> SemIR::InstId {
   auto* class_decl = TypeAsClassDecl(context, query_self_const_id);
   if (!class_decl) {
     return SemIR::InstId::None;
   }
 
-  auto* dtor = context.clang_sema().LookupDestructor(class_decl);
-  if (!dtor) {
+  auto decl_info =
+      GetDeclForCoreInterface(context.clang_sema(), core_interface, class_decl);
+  if (!decl_info.decl) {
     // TODO: If the impl lookup failure is an error, we should produce a
-    // diagnostic explaining why the class is not destructible.
+    // diagnostic explaining why the class is not copyable/destructible.
     return SemIR::InstId::None;
   }
+  auto* cpp_fn = cast<clang::FunctionDecl>(decl_info.decl);
 
-  return BuildSingleFunctionWitness(
-      context, loc_id, dtor,
-      clang::DeclAccessPair::make(dtor, dtor->getAccess()), /*num_params=*/0,
-      query_self_const_id, query_specific_interface_id);
-}
-
-auto LookupCppImpl(Context& context, SemIR::LocId loc_id,
-                   CoreInterface core_interface,
-                   SemIR::ConstantId query_self_const_id,
-                   SemIR::SpecificInterfaceId query_specific_interface_id,
-                   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, query_self_const_id,
-                            query_specific_interface_id);
-    case CoreInterface::Destroy:
-      return LookupDestroyImpl(context, loc_id, query_self_const_id,
-                               query_specific_interface_id);
+  if (context.clang_sema().DiagnoseUseOfOverloadedDecl(
+          cpp_fn, GetCppLocation(context, loc_id))) {
+    return SemIR::ErrorInst::InstId;
+  }
 
-    case CoreInterface::Unknown:
-      CARBON_FATAL("shouldn't be called with `Unknown`");
+  auto fn_id =
+      ImportCppFunctionDecl(context, loc_id, cpp_fn, decl_info.num_params);
+  if (fn_id == SemIR::ErrorInst::InstId) {
+    return SemIR::ErrorInst::InstId;
   }
+  CheckCppOverloadAccess(
+      context, loc_id, clang::DeclAccessPair::make(cpp_fn, cpp_fn->getAccess()),
+      context.insts().GetAsKnownInstId<SemIR::FunctionDecl>(fn_id));
 
   // TODO: Infer a C++ type structure and check whether it's less strict than
   // the best Carbon type structure.
   static_cast<void>(best_impl_type_structure);
   static_cast<void>(best_impl_loc_id);
 
-  return SemIR::InstId::None;
+  return BuildCustomWitness(context, loc_id, query_self_const_id,
+                            query_specific_interface_id, {fn_id});
 }
 
 }  // namespace Carbon::Check

toolchain/check/cpp/operators.cpp

@@ -262,12 +262,10 @@ auto LookupCppOperator(Context& context, SemIR::LocId loc_id, Operator op,
           // If this is an operator method, the first arg will be used as self.
           arg_ids.size() -
               (isa<clang::CXXMethodDecl>(best_viable_fn->Function) ? 1 : 0));
-      if (auto fn_decl =
-              context.insts().TryGetAsWithId<SemIR::FunctionDecl>(result_id)) {
-        CheckCppOverloadAccess(context, loc_id, best_viable_fn->FoundDecl,
-                               fn_decl->inst_id);
-      } else {
-        CARBON_CHECK(result_id == SemIR::ErrorInst::InstId);
+      if (result_id != SemIR::ErrorInst::InstId) {
+        CheckCppOverloadAccess(
+            context, loc_id, best_viable_fn->FoundDecl,
+            context.insts().GetAsKnownInstId<SemIR::FunctionDecl>(result_id));
       }
       return result_id;
     }

toolchain/check/cpp/overload_resolution.cpp

@@ -164,12 +164,11 @@ auto PerformCppOverloadResolution(Context& context, SemIR::LocId loc_id,
       CARBON_CHECK(!best_viable_fn->RewriteKind);
       SemIR::InstId result_id = ImportCppFunctionDecl(
           context, loc_id, best_viable_fn->Function, arg_exprs.size());
-      if (auto fn_decl =
-              context.insts().TryGetAsWithId<SemIR::FunctionDecl>(result_id)) {
-        CheckCppOverloadAccess(context, loc_id, best_viable_fn->FoundDecl,
-                               fn_decl->inst_id, overload_set.parent_scope_id);
-      } else {
-        CARBON_CHECK(result_id == SemIR::ErrorInst::InstId);
+      if (result_id != SemIR::ErrorInst::InstId) {
+        CheckCppOverloadAccess(
+            context, loc_id, best_viable_fn->FoundDecl,
+            context.insts().GetAsKnownInstId<SemIR::FunctionDecl>(result_id),
+            overload_set.parent_scope_id);
       }
       return result_id;
     }

toolchain/sem_ir/inst.h

@@ -546,21 +546,21 @@ class InstStore {
     return TryGetAs<InstT>(inst_id);
   }
 
+  // Returns the `KnownInstId` form of `inst_id`. Requires a matching
+  // instruction type.
+  template <typename InstT>
+  auto GetAsKnownInstId(InstId inst_id) const -> KnownInstId<InstT> {
+    CARBON_CHECK(Is<InstT>(inst_id), "Casting inst {0} to wrong kind {1}",
+                 Get(inst_id), Internal::InstLikeTypeInfo<InstT>::DebugName());
+    return KnownInstId<InstT>::UnsafeMake(inst_id);
+  }
+
   template <typename InstT>
   struct GetAsWithIdResult {
     KnownInstId<InstT> inst_id;
     InstT inst;
   };
 
-  // Returns the requested instruction, which is known to have the specified
-  // type, along with the original `InstId`, encoding the work of checking its
-  // type in a `KnownInstId`.
-  template <typename InstT>
-  auto GetAsWithId(InstId inst_id) const -> GetAsWithIdResult<InstT> {
-    auto inst = GetAs<InstT>(inst_id);
-    return {.inst_id = KnownInstId<InstT>::UnsafeMake(inst_id), .inst = inst};
-  }
-
   // Returns the requested instruction, if it is of that type, along with the
   // original `InstId`, encoding the work of checking its type in a
   // `KnownInstId`.