Sfoglia il codice sorgente

Revert "refactors `LookupCppImpl` to handle multiple associated functions (#6816)" (#6900)

We discussed whether associated functions should be processed in a
general manner. Since many associated functions will have some amount of
unique processing, we're probably better off not having a general
utility, and we can return to the original `CoreInterface`, which was
much simpler in design.

This reverts commit 4d0003765d92a9e90649c6bd28b137c2243b79a8.
Christopher Di Bella 1 mese fa
parent
commit
ffe8f8f67d

+ 21 - 79
toolchain/check/cpp/impl_lookup.cpp

@@ -4,8 +4,6 @@
 
 #include "toolchain/check/cpp/impl_lookup.h"
 
-#include <type_traits>
-
 #include "clang/Sema/Sema.h"
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/cpp/import.h"
@@ -59,39 +57,11 @@ struct DeclInfo {
 };
 }  // namespace
 
-// Describes the function that needs to be looked up.
-enum class AssociatedFunction : std::underlying_type_t<CoreInterface> {
-  // CoreInterface::Copy
-  CopyConstructor = llvm::to_underlying(CoreInterface::Copy),
-
-  // CoreInterface::Destroy
-  Destructor = llvm::to_underlying(CoreInterface::Destroy),
-
-  // CoreInterface::CppUnsafeDeref
-  CppUnsafeDeref = llvm::to_underlying(CoreInterface::CppUnsafeDeref),
-};
-
-// Maps a `CoreInterface` to its corresponding set of `CppCoreFunction`s.
-static auto GetCppAssociatedFunctions(const CoreInterface core_interface)
-    -> std::bitset<8> {
-  switch (core_interface) {
-    case CoreInterface::Copy:
-      return {llvm::to_underlying(AssociatedFunction::CopyConstructor)};
-    case CoreInterface::Destroy:
-      return {llvm::to_underlying(AssociatedFunction::Destructor)};
-    case CoreInterface::CppUnsafeDeref:
-      return {llvm::to_underlying(AssociatedFunction::CppUnsafeDeref)};
-    case CoreInterface::Unknown:
-    case CoreInterface::IntFitsIn:
-      CARBON_FATAL("No AssociatedFunction mapping for this interface");
-  }
-}
-
 // Retrieves a `core_interface`'s corresponding `NamedDecl`, also with the
 // expected number of parameters. May return a null decl to indicate nothing was
 // found, or nullopt to indicate `SemIR::ErrInst::InstId` should be propagated.
 auto GetDeclForCoreInterface(Context& context, SemIR::LocId loc_id,
-                             AssociatedFunction associated_function,
+                             CoreInterface core_interface,
                              clang::CXXRecordDecl* class_decl)
     -> std::optional<DeclInfo> {
   auto& clang_sema = context.clang_sema();
@@ -99,15 +69,15 @@ auto GetDeclForCoreInterface(Context& context, SemIR::LocId loc_id,
 
   // TODO: Handle other interfaces.
 
-  switch (associated_function) {
-    case AssociatedFunction::CopyConstructor:
+  switch (core_interface) {
+    case CoreInterface::Copy:
       return DeclInfo{.decl = clang_sema.LookupCopyingConstructor(
                           class_decl, clang::Qualifiers::Const),
                       .signature = {.num_params = 1}};
-    case AssociatedFunction::Destructor:
+    case CoreInterface::Destroy:
       return DeclInfo{.decl = clang_sema.LookupDestructor(class_decl),
                       .signature = {.num_params = 0}};
-    case AssociatedFunction::CppUnsafeDeref: {
+    case CoreInterface::CppUnsafeDeref: {
       auto candidates = class_decl->lookup(
           clang_sema.getASTContext().DeclarationNames.getCXXOperatorName(
               clang::OO_Star));
@@ -123,17 +93,27 @@ auto GetDeclForCoreInterface(Context& context, SemIR::LocId loc_id,
       return DeclInfo{.decl = *candidates.begin(),
                       .signature = {.num_params = 0}};
     }
+    case CoreInterface::IntFitsIn:
+    case CoreInterface::Unknown:
+      CARBON_FATAL("shouldn't be called with `{}`", core_interface);
   }
 }
 
-static auto FindCppAssociatedFunction(Context& context, SemIR::LocId loc_id,
-                                      AssociatedFunction associated_function,
-                                      clang::CXXRecordDecl* class_decl)
-    -> 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 {
   // TODO: This should provide `Destroy` for enums and other trivially
   // destructible types.
+  auto* class_decl = TypeAsClassDecl(context, query_self_const_id);
+  if (!class_decl) {
+    return SemIR::InstId::None;
+  }
+
   auto decl_info =
-      GetDeclForCoreInterface(context, loc_id, associated_function, class_decl);
+      GetDeclForCoreInterface(context, loc_id, core_interface, class_decl);
   if (!decl_info.has_value()) {
     return SemIR::ErrorInst::InstId;
   }
@@ -159,51 +139,13 @@ static auto FindCppAssociatedFunction(Context& context, SemIR::LocId loc_id,
       context, loc_id, clang::DeclAccessPair::make(cpp_fn, cpp_fn->getAccess()),
       context.insts().GetAsKnownInstId<SemIR::FunctionDecl>(fn_id));
 
-  return fn_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 {
-  auto* class_decl = TypeAsClassDecl(context, query_self_const_id);
-  if (!class_decl) {
-    return SemIR::InstId::None;
-  }
-
-  auto witness_id = SemIR::ErrorInst::InstId;
-
-  switch (core_interface) {
-    case CoreInterface::Copy:
-    case CoreInterface::Destroy:
-    case CoreInterface::CppUnsafeDeref: {
-      auto associated_functions = GetCppAssociatedFunctions(core_interface);
-      CARBON_CHECK(associated_functions.count() == 1);
-      witness_id = FindCppAssociatedFunction(
-          context, loc_id,
-          static_cast<AssociatedFunction>(associated_functions.to_ullong()),
-          class_decl);
-    } break;
-    case CoreInterface::IntFitsIn:
-      return SemIR::InstId::None;
-    case CoreInterface::Unknown:
-      CARBON_FATAL("shouldn't be called with `Unknown`");
-  }
-
-  if (witness_id == SemIR::InstId::None ||
-      witness_id == SemIR::ErrorInst::InstId) {
-    return witness_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 BuildCustomWitness(context, loc_id, query_self_const_id,
-                            query_specific_interface_id, {witness_id});
+                            query_specific_interface_id, {fn_id});
 }
 
 }  // namespace Carbon::Check

+ 0 - 3
toolchain/check/custom_witness.cpp

@@ -210,9 +210,6 @@ auto BuildCustomWitness(Context& context, SemIR::LocId loc_id,
   // Fill in the witness table.
   for (const auto& [assoc_entity_id, value_id] :
        llvm::zip_equal(assoc_entities, values)) {
-    CARBON_DCHECK(value_id != SemIR::InstId::None);
-    CARBON_DCHECK(value_id != SemIR::ErrorInst::InstId);
-
     LoadImportRef(context, assoc_entity_id);
 
     // Build a witness with the current contents of the witness table. This will

+ 6 - 6
toolchain/check/custom_witness.h

@@ -21,13 +21,13 @@ auto BuildCustomWitness(Context& context, SemIR::LocId loc_id,
 
 // Significant interfaces in `Core` which correspond to language features and
 // can have custom witnesses.
-enum class CoreInterface : std::int8_t {
-  Copy = 1 << 0,
-  Destroy = 1 << 1,
-  IntFitsIn = 1 << 2,
-  CppUnsafeDeref = 1 << 3,
+enum class CoreInterface {
+  Copy,
+  Destroy,
+  IntFitsIn,
+  CppUnsafeDeref,
 
-  Unknown = -1,
+  Unknown,
 };
 
 // Given an interface, returns the corresponding enum if it's covered by