From b3e9dd3ea2c955a3fcdbc86fd2ab17054cd39101 Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Thu, 16 Jul 2026 14:09:46 -0400 Subject: [PATCH] Consolidate checking for and rejecting `other_requirements` in impl lookup (#7518) Since the introduction of `other_requirements`, we now have a dedicated step in impl lookup for checking that the requirements of the query facet type are satisfied. That is the place where we will be checking same-type constraints, which `other_requirements` signals the presence of. Consolidate all checking of `other_requirements` to that step, which reduces our use of `FacetTypeInfo` (as opposed to the `IdentifiedFacetType`) and removes interest in same-type constraints from code that is not related to them. --- toolchain/check/impl_lookup.cpp | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/toolchain/check/impl_lookup.cpp b/toolchain/check/impl_lookup.cpp index 67ef13838e32..3927c7e5c1f0 100644 --- a/toolchain/check/impl_lookup.cpp +++ b/toolchain/check/impl_lookup.cpp @@ -203,25 +203,15 @@ static auto FindAndDiagnoseImplLookupCycle( return false; } -struct RequiredImplsFromConstraint { - llvm::ArrayRef req_impls; - bool other_requirements; -}; - // Gets the set of `SpecificInterface`s that are required by a facet type // (as a constant value), and any special requirements. static auto GetRequiredImplsFromConstraint( Context& context, SemIR::LocId loc_id, SemIR::ConstantId query_self_const_id, SemIR::ConstantId query_facet_type_const_id, bool diagnose) - -> std::optional { + -> std::optional> { auto facet_type_inst_id = context.types().GetTypeInstIdForTypeConstantId(query_facet_type_const_id); - auto facet_type_inst = - context.insts().GetAs(facet_type_inst_id); - const auto& facet_type_info = - context.facet_types().Get(facet_type_inst.facet_type_id); - auto identified_id = RequireIdentifiedFacetType( context, loc_id, query_self_const_id, facet_type_inst_id, [&](auto& builder) { @@ -234,10 +224,7 @@ static auto GetRequiredImplsFromConstraint( if (!identified_id.has_value()) { return std::nullopt; } - return { - {.req_impls = - context.identified_facet_types().Get(identified_id).required_impls(), - .other_requirements = facet_type_info.other_requirements}}; + return context.identified_facet_types().Get(identified_id).required_impls(); } static auto TreatImplAsFinal(Context& context, const SemIR::Impl& impl) @@ -304,10 +291,6 @@ static auto TryGetSpecificWitnessIdForImpl( CARBON_CHECK(deduced_constraint_facet_type_info.extend_constraints.size() == 1); - if (deduced_constraint_facet_type_info.other_requirements) { - return SemIR::ConstantId::None; - } - // The specifics in the queried interface must match the deduced specifics in // the impl's constraint facet type. auto impl_interface_specific_id = @@ -558,6 +541,9 @@ static auto VerifyQueryFacetTypeConstraints( // TODO: Validate that the witnesses satisfy the other requirements in the // `facet_type_info`. + if (facet_type_info.other_requirements) { + return false; + } return true; } @@ -1002,14 +988,10 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id, auto req_impls_from_constraint = GetRequiredImplsFromConstraint(context, loc_id, query_self_const_id, query_facet_type_const_id, diagnose); - if (!req_impls_from_constraint) { + if (!req_impls_from_constraint.has_value()) { return SemIR::InstBlockIdOrError::MakeError(); } - auto [req_impls, other_requirements] = *req_impls_from_constraint; - if (other_requirements) { - // TODO: Remove this when other requirements go away. - return SemIR::InstBlockId::None; - } + auto req_impls = *req_impls_from_constraint; if (req_impls.empty()) { return SemIR::InstBlockId::Empty; }