From 2543d2ea4ff52e35a75b1f64a93da172a23ca75c Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 17 Dec 2025 15:11:52 -0800 Subject: [PATCH] Add CoreInterface for consistent tracking of CoreIdentifier interfaces (#6516) We're going to want to track `Destroy` and some other interfaces in ways similar to what the C++ logic wants. Rather than having both do their own comparisons with similar values, this tries to centralize logic. Note the prior `context.identifiers().Get(interface.name_id.AsIdentifierId())` is also obsolete due to #6486, this is just replacing it in a single swoop and avoiding string comparisons as a consequence. --- toolchain/check/cpp/impl_lookup.cpp | 30 +++++++++++------------------ toolchain/check/cpp/impl_lookup.h | 7 ++++--- toolchain/check/custom_witness.cpp | 19 ++++++++++++++++++ toolchain/check/custom_witness.h | 14 ++++++++++++++ toolchain/check/impl_lookup.cpp | 23 +++++++++++++--------- 5 files changed, 62 insertions(+), 31 deletions(-) diff --git a/toolchain/check/cpp/impl_lookup.cpp b/toolchain/check/cpp/impl_lookup.cpp index 227c227ba276..3756f9ba7970 100644 --- a/toolchain/check/cpp/impl_lookup.cpp +++ b/toolchain/check/cpp/impl_lookup.cpp @@ -114,29 +114,21 @@ static auto LookupDestroyImpl(Context& context, SemIR::LocId loc_id, } auto LookupCppImpl(Context& context, SemIR::LocId loc_id, - SemIR::TypeId self_type_id, + SemIR::TypeId self_type_id, CoreInterface core_interface, SemIR::SpecificInterface specific_interface, const TypeStructure* best_impl_type_structure, SemIR::LocId best_impl_loc_id) -> SemIR::InstId { - // Determine whether this is an interface that we have special knowledge of. - auto& interface = context.interfaces().Get(specific_interface.interface_id); - if (!context.name_scopes().IsCorePackage(interface.parent_scope_id)) { - return SemIR::InstId::None; - } - if (!interface.name_id.AsIdentifierId().has_value()) { - return SemIR::InstId::None; - } - - if (context.identifiers().Get(interface.name_id.AsIdentifierId()) == "Copy") { - return LookupCopyImpl(context, loc_id, self_type_id, specific_interface); - } - - if (context.identifiers().Get(interface.name_id.AsIdentifierId()) == - "Destroy") { - return LookupDestroyImpl(context, loc_id, self_type_id, specific_interface); - } - // TODO: Handle other interfaces. + switch (core_interface) { + case CoreInterface::Copy: + return LookupCopyImpl(context, loc_id, self_type_id, specific_interface); + case CoreInterface::Destroy: + return LookupDestroyImpl(context, loc_id, self_type_id, + specific_interface); + + case CoreInterface::Unknown: + CARBON_FATAL("shouldn't be called with `Unknown`"); + } // TODO: Infer a C++ type structure and check whether it's less strict than // the best Carbon type structure. diff --git a/toolchain/check/cpp/impl_lookup.h b/toolchain/check/cpp/impl_lookup.h index dfd6db45fb42..bee21a0d8c9d 100644 --- a/toolchain/check/cpp/impl_lookup.h +++ b/toolchain/check/cpp/impl_lookup.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_CHECK_CPP_IMPL_LOOKUP_H_ #include "toolchain/check/context.h" +#include "toolchain/check/custom_witness.h" #include "toolchain/check/impl_lookup.h" #include "toolchain/check/type_structure.h" #include "toolchain/sem_ir/ids.h" @@ -16,8 +17,8 @@ namespace Carbon::Check { // Performs lookup for an impl witness for a query involving C++ types. Returns // a witness value, or `None` if a synthesized C++ witness should not be used. // -// If `interface` is an interface for which we can synthesize a witness based on -// C++ operator overloads or special member functions, performs the suitable C++ +// Given a known `core_interface`, we can synthesize a witness based on C++ +// operator overloads or special member functions. Performs the suitable C++ // lookup to determine if this interface should be considered implemented for // the specified type, and if so, synthesizes and returns a suitable witness. // @@ -34,7 +35,7 @@ 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, + SemIR::TypeId self_type_id, CoreInterface core_interface, SemIR::SpecificInterface specific_interface, const TypeStructure* best_impl_type_structure, SemIR::LocId best_impl_loc_id) -> SemIR::InstId; diff --git a/toolchain/check/custom_witness.cpp b/toolchain/check/custom_witness.cpp index affa9e1cfbc2..4039361d3f68 100644 --- a/toolchain/check/custom_witness.cpp +++ b/toolchain/check/custom_witness.cpp @@ -91,4 +91,23 @@ auto BuildCustomWitness(Context& context, SemIR::LocId loc_id, return make_witness(); } +auto GetCoreInterface(Context& context, SemIR::InterfaceId interface_id) + -> CoreInterface { + const auto& interface = context.interfaces().Get(interface_id); + if (!context.name_scopes().IsCorePackage(interface.parent_scope_id) || + !interface.name_id.AsIdentifierId().has_value()) { + return CoreInterface::Unknown; + } + + for (auto [core_identifier, core_interface] : + {std::pair{CoreIdentifier::Copy, CoreInterface::Copy}, + std::pair{CoreIdentifier::Destroy, CoreInterface::Destroy}}) { + if (interface.name_id == + context.core_identifiers().AddNameId(core_identifier)) { + return core_interface; + } + } + return CoreInterface::Unknown; +} + } // namespace Carbon::Check diff --git a/toolchain/check/custom_witness.h b/toolchain/check/custom_witness.h index a2afed9da3ba..2b08ba7788c1 100644 --- a/toolchain/check/custom_witness.h +++ b/toolchain/check/custom_witness.h @@ -19,6 +19,20 @@ auto BuildCustomWitness(Context& context, SemIR::LocId loc_id, SemIR::SpecificInterface specific_interface, llvm::ArrayRef values) -> SemIR::InstId; +// Significant interfaces in `Core` which correspond to language features and +// can have custom witnesses. +enum class CoreInterface { + Copy, + Destroy, + + Unknown, +}; + +// Given an interface, returns the corresponding enum if it's covered by +// `CoreInterface`, or `Unknown` if it's some other interface. +auto GetCoreInterface(Context& context, SemIR::InterfaceId interface_id) + -> CoreInterface; + } // namespace Carbon::Check #endif // CARBON_TOOLCHAIN_CHECK_CUSTOM_WITNESS_H_ diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index bbed3eba6dd1..cdb053990e3c 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -11,6 +11,7 @@ #include "toolchain/base/kind_switch.h" #include "toolchain/check/cpp/impl_lookup.h" +#include "toolchain/check/custom_witness.h" #include "toolchain/check/deduce.h" #include "toolchain/check/diagnostic_helpers.h" #include "toolchain/check/eval.h" @@ -1053,15 +1054,19 @@ auto EvalLookupSingleImplWitness(Context& context, SemIR::LocId loc_id, } if (query_is_concrete && candidates.consider_cpp_candidates) { - // 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), - 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)}; + auto core_interface = + GetCoreInterface(context, query_specific_interface.interface_id); + if (core_interface != CoreInterface::Unknown) { + // 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); + if (cpp_witness_id.has_value()) { + lookup_result = {.result = + EvalImplLookupResult::MakeFinal(cpp_witness_id)}; + } } }