From 23a7ca04bf389885c110484dd7904929b6642966 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 12 Nov 2025 18:03:00 -0500 Subject: [PATCH 01/13] Sema: Remove BindingSet::operator bool --- include/swift/Sema/CSBindings.h | 4 ---- lib/Sema/CSBindings.cpp | 7 +++++-- lib/Sema/CSOptimizer.cpp | 4 +++- lib/Sema/ConstraintGraph.cpp | 3 ++- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 3e17979652d90..0b0a06b687f72 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -469,10 +469,6 @@ class BindingSet { /// checking. bool isViable(PotentialBinding &binding, bool isTransitive); - explicit operator bool() const { - return hasViableBindings() || isDirectHole(); - } - /// Determine whether this set has any "viable" (or non-hole) bindings. /// /// A viable binding could be - a direct or transitive binding diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 04a4704344cbc..d4bf051d4f557 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1171,7 +1171,7 @@ std::optional ConstraintSystem::determineBestBindings( if (shouldAttemptFixes() && typeVar->getImpl().canBindToHole()) return true; - return bool(bindings); + return bindings.hasViableBindings() || bindings.isDirectHole(); }; // Now let's see if we could infer something for related type @@ -1198,7 +1198,10 @@ std::optional ConstraintSystem::determineBestBindings( if (!bindings.finalize(true)) continue; - if (!bindings || !isViable) + if (!bindings.hasViableBindings() && !bindings.isDirectHole()) + continue; + + if (!isViable) continue; onCandidate(bindings); diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 31992713ceb63..aab3431df14bb 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1105,7 +1105,9 @@ static void determineBestChoicesInContext( // Simply adding it as a binding won't work because if the second argument // is non-optional the overload that returns `T?` would still have a lower // score. - if (!bindingSet && isNilCoalescingOperator(disjunction)) { + if (!bindingSet.hasViableBindings() && + !bindingSet.isDirectHole() && + isNilCoalescingOperator(disjunction)) { auto &cg = cs.getConstraintGraph(); if (llvm::any_of(cg[typeVar].getConstraints(), [&typeVar](Constraint *constraint) { diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index 1c47c257ec983..a8909d352392f 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -921,7 +921,8 @@ bool ConstraintGraph::contractEdges() { // us enough information to decided on l-valueness. if (tyvar1->getImpl().canBindToInOut()) { bool isNotContractable = true; - if (auto bindings = CS.getBindingsFor(tyvar1)) { + auto bindings = CS.getBindingsFor(tyvar1); + if (bindings.hasViableBindings() || bindings.isDirectHole()) { // Holes can't be contracted. if (bindings.isHole()) continue; From 5f2db0c96c96ddc6c73b47927e2e00afba1345f3 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 13 Nov 2025 18:00:59 -0500 Subject: [PATCH 02/13] Sema: Pull call to determineLiteralCoverage() out of BindingSet::finalize() --- include/swift/Sema/CSBindings.h | 8 ++++---- lib/Sema/CSBindings.cpp | 5 +++-- unittests/Sema/BindingInferenceTests.cpp | 1 + unittests/Sema/SemaFixture.cpp | 1 + 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 0b0a06b687f72..4305f53fb5b63 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -556,6 +556,10 @@ class BindingSet { /// requirements down the subtype or equivalence chain. void inferTransitiveProtocolRequirements(); + /// Check whether the given binding set covers any of the + /// literal protocols associated with this type variable. + void determineLiteralCoverage(); + /// Finalize binding computation for this type variable by /// inferring bindings from context e.g. transitive bindings. /// @@ -586,10 +590,6 @@ class BindingSet { void addDefault(Constraint *constraint); - /// Check whether the given binding set covers any of the - /// literal protocols associated with this type variable. - void determineLiteralCoverage(); - StringRef getLiteralBindingKind(LiteralBindingKind K) const { #define ENTRY(Kind, String) \ case LiteralBindingKind::Kind: \ diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index d4bf051d4f557..4cb42c555a0c8 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -668,8 +668,6 @@ bool BindingSet::finalize(bool transitive) { if (transitive) inferTransitiveBindings(); - determineLiteralCoverage(); - if (auto *locator = TypeVar->getImpl().getLocator()) { if (locator->isLastElement()) { // If this is a base of an unresolved member chain, as a last @@ -1198,6 +1196,8 @@ std::optional ConstraintSystem::determineBestBindings( if (!bindings.finalize(true)) continue; + bindings.determineLiteralCoverage(); + if (!bindings.hasViableBindings() && !bindings.isDirectHole()) continue; @@ -1595,6 +1595,7 @@ BindingSet ConstraintSystem::getBindingsFor(TypeVariableType *typeVar) { BindingSet bindings(*this, typeVar, CG[typeVar].getPotentialBindings()); bindings.finalize(false); + bindings.determineLiteralCoverage(); return bindings; } diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index fdd6c6682a489..5993fce1b82ec 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -126,6 +126,7 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { cs.getConstraintGraph()[floatLiteralTy].initBindingSet(); bindings.finalize(/*transitive=*/true); + bindings.determineLiteralCoverage(); // Inferred a single transitive binding through `$T_float`. ASSERT_EQ(bindings.Bindings.size(), (unsigned)1); diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index 5ee9c005a5e93..677f2070aa50b 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -142,6 +142,7 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, auto &bindings = node.getBindingSet(); bindings.inferTransitiveProtocolRequirements(); bindings.finalize(/*transitive=*/true); + bindings.determineLiteralCoverage(); } auto &node = cs.getConstraintGraph()[typeVar]; From 53d802a23950e52a91c748bde62616597f54f07f Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 13 Nov 2025 18:03:36 -0500 Subject: [PATCH 03/13] Sema: Re-organize BindingSet::finalize() --- lib/Sema/CSBindings.cpp | 61 +++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 4cb42c555a0c8..70ca3aee98719 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -665,43 +665,46 @@ static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability, } bool BindingSet::finalize(bool transitive) { - if (transitive) + if (transitive) { inferTransitiveBindings(); - if (auto *locator = TypeVar->getImpl().getLocator()) { - if (locator->isLastElement()) { - // If this is a base of an unresolved member chain, as a last - // resort effort let's infer base to be a protocol type based - // on contextual conformance requirements. - // - // This allows us to find solutions in cases like this: - // - // \code - // func foo(_: T) {} - // foo(.bar) <- `.bar` should be a static member of `P`. - // \endcode - if (transitive && !hasViableBindings()) { - inferTransitiveProtocolRequirements(); - - if (TransitiveProtocols.has_value()) { - for (auto *constraint : *TransitiveProtocols) { - Type protocolTy = constraint->getSecondType(); - - // Compiler-known marker protocols cannot be extended with members, - // so do not consider them. - if (auto p = protocolTy->getAs()) { - if (ProtocolDecl *decl = p->getDecl()) - if (decl->getKnownProtocolKind() && decl->isMarkerProtocol()) - continue; - } + if (!hasViableBindings()) { + if (auto *locator = TypeVar->getImpl().getLocator()) { + if (locator->isLastElement()) { + // If this is a base of an unresolved member chain, as a last + // resort effort let's infer base to be a protocol type based + // on contextual conformance requirements. + // + // This allows us to find solutions in cases like this: + // + // \code + // func foo(_: T) {} + // foo(.bar) <- `.bar` should be a static member of `P`. + // \endcode + inferTransitiveProtocolRequirements(); + + if (TransitiveProtocols.has_value()) { + for (auto *constraint : *TransitiveProtocols) { + Type protocolTy = constraint->getSecondType(); + + // Compiler-known marker protocols cannot be extended with members, + // so do not consider them. + if (auto p = protocolTy->getAs()) { + if (ProtocolDecl *decl = p->getDecl()) + if (decl->getKnownProtocolKind() && decl->isMarkerProtocol()) + continue; + } - addBinding({protocolTy, AllowedBindingKind::Exact, constraint}, - /*isTransitive=*/false); + addBinding({protocolTy, AllowedBindingKind::Exact, constraint}, + /*isTransitive=*/false); + } } } } } + } + if (auto *locator = TypeVar->getImpl().getLocator()) { if (TypeVar->getImpl().isKeyPathType()) { auto &ctx = CS.getASTContext(); From eede7e4bb0c08254752cd08c2c4e6a33a5d2f1ad Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 13 Nov 2025 18:04:43 -0500 Subject: [PATCH 04/13] Sema: Move some code from BindingSet::finalize() to BindingSet::inferTransitiveBindings() --- lib/Sema/CSBindings.cpp | 73 ++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 70ca3aee98719..44b8c51572244 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -625,6 +625,41 @@ void BindingSet::inferTransitiveBindings() { /*isTransitive=*/true); } } + + if (!hasViableBindings()) { + if (auto *locator = TypeVar->getImpl().getLocator()) { + if (locator->isLastElement()) { + // If this is a base of an unresolved member chain, as a last + // resort effort let's infer base to be a protocol type based + // on contextual conformance requirements. + // + // This allows us to find solutions in cases like this: + // + // \code + // func foo(_: T) {} + // foo(.bar) <- `.bar` should be a static member of `P`. + // \endcode + inferTransitiveProtocolRequirements(); + + if (TransitiveProtocols.has_value()) { + for (auto *constraint : *TransitiveProtocols) { + Type protocolTy = constraint->getSecondType(); + + // Compiler-known marker protocols cannot be extended with members, + // so do not consider them. + if (auto p = protocolTy->getAs()) { + if (ProtocolDecl *decl = p->getDecl()) + if (decl->getKnownProtocolKind() && decl->isMarkerProtocol()) + continue; + } + + addBinding({protocolTy, AllowedBindingKind::Exact, constraint}, + /*isTransitive=*/false); + } + } + } + } + } } static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability, @@ -665,45 +700,9 @@ static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability, } bool BindingSet::finalize(bool transitive) { - if (transitive) { + if (transitive) inferTransitiveBindings(); - if (!hasViableBindings()) { - if (auto *locator = TypeVar->getImpl().getLocator()) { - if (locator->isLastElement()) { - // If this is a base of an unresolved member chain, as a last - // resort effort let's infer base to be a protocol type based - // on contextual conformance requirements. - // - // This allows us to find solutions in cases like this: - // - // \code - // func foo(_: T) {} - // foo(.bar) <- `.bar` should be a static member of `P`. - // \endcode - inferTransitiveProtocolRequirements(); - - if (TransitiveProtocols.has_value()) { - for (auto *constraint : *TransitiveProtocols) { - Type protocolTy = constraint->getSecondType(); - - // Compiler-known marker protocols cannot be extended with members, - // so do not consider them. - if (auto p = protocolTy->getAs()) { - if (ProtocolDecl *decl = p->getDecl()) - if (decl->getKnownProtocolKind() && decl->isMarkerProtocol()) - continue; - } - - addBinding({protocolTy, AllowedBindingKind::Exact, constraint}, - /*isTransitive=*/false); - } - } - } - } - } - } - if (auto *locator = TypeVar->getImpl().getLocator()) { if (TypeVar->getImpl().isKeyPathType()) { auto &ctx = CS.getASTContext(); From 02129f2b1f78de493d822fc1105a0d05d5e6ba01 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 13 Nov 2025 18:09:52 -0500 Subject: [PATCH 05/13] Sema: Pull call to inferTransitiveBindings() out of BindingSet::finalize() --- include/swift/Sema/CSBindings.h | 2 +- lib/Sema/CSBindings.cpp | 11 +++++------ unittests/Sema/BindingInferenceTests.cpp | 3 ++- unittests/Sema/SemaFixture.cpp | 3 ++- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 4305f53fb5b63..42be869214d4c 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -565,7 +565,7 @@ class BindingSet { /// /// \returns true if finalization successful (which makes binding set viable), /// and false otherwise. - bool finalize(bool transitive); + bool finalize(); static BindingScore formBindingScore(const BindingSet &b); diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 44b8c51572244..294d641b29efa 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -699,10 +699,7 @@ static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability, return keyPathTy; } -bool BindingSet::finalize(bool transitive) { - if (transitive) - inferTransitiveBindings(); - +bool BindingSet::finalize() { if (auto *locator = TypeVar->getImpl().getLocator()) { if (TypeVar->getImpl().isKeyPathType()) { auto &ctx = CS.getASTContext(); @@ -1195,7 +1192,9 @@ std::optional ConstraintSystem::determineBestBindings( // produce a default type. bool isViable = isViableForRanking(bindings); - if (!bindings.finalize(true)) + bindings.inferTransitiveBindings(); + + if (!bindings.finalize()) continue; bindings.determineLiteralCoverage(); @@ -1596,7 +1595,7 @@ BindingSet ConstraintSystem::getBindingsFor(TypeVariableType *typeVar) { assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type"); BindingSet bindings(*this, typeVar, CG[typeVar].getPotentialBindings()); - bindings.finalize(false); + bindings.finalize(); bindings.determineLiteralCoverage(); return bindings; diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index 5993fce1b82ec..f973b9f0610da 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -125,7 +125,8 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { cs.getConstraintGraph()[floatLiteralTy].initBindingSet(); - bindings.finalize(/*transitive=*/true); + bindings.inferTransitiveBindings(); + bindings.finalize(); bindings.determineLiteralCoverage(); // Inferred a single transitive binding through `$T_float`. diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index 677f2070aa50b..c4471a253c9e9 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -141,7 +141,8 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, auto &bindings = node.getBindingSet(); bindings.inferTransitiveProtocolRequirements(); - bindings.finalize(/*transitive=*/true); + bindings.inferTransitiveBindings(); + bindings.finalize(); bindings.determineLiteralCoverage(); } From 45d03e152c4811083b95e50ecbd877c3791148bd Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 13 Nov 2025 20:16:12 -0500 Subject: [PATCH 06/13] Sema: Split up BindingSet::finalize() into finalizeKeyPathBindings() and finalizeUnresolvedMemberChain() --- include/swift/Sema/CSBindings.h | 8 +++++--- lib/Sema/CSBindings.cpp | 25 +++++++++++++----------- unittests/Sema/BindingInferenceTests.cpp | 3 ++- unittests/Sema/SemaFixture.cpp | 7 ++++++- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 42be869214d4c..09fbec2cdf686 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -560,12 +560,14 @@ class BindingSet { /// literal protocols associated with this type variable. void determineLiteralCoverage(); - /// Finalize binding computation for this type variable by - /// inferring bindings from context e.g. transitive bindings. + /// Finalize binding computation for key path type variables. /// /// \returns true if finalization successful (which makes binding set viable), /// and false otherwise. - bool finalize(); + bool finalizeKeyPathBindings(); + + /// Handle diagnostics of unresolved member chains. + void finalizeUnresolvedMemberChainResult(); static BindingScore formBindingScore(const BindingSet &b); diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 294d641b29efa..8450cdc4eeb55 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -699,13 +699,11 @@ static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability, return keyPathTy; } -bool BindingSet::finalize() { +bool BindingSet::finalizeKeyPathBindings() { if (auto *locator = TypeVar->getImpl().getLocator()) { if (TypeVar->getImpl().isKeyPathType()) { auto &ctx = CS.getASTContext(); - - auto *keyPathLoc = TypeVar->getImpl().getLocator(); - auto *keyPath = castToExpr(keyPathLoc->getAnchor()); + auto *keyPath = castToExpr(locator->getAnchor()); bool isValid; std::optional capability; @@ -772,7 +770,7 @@ bool BindingSet::finalize() { auto keyPathTy = getKeyPathType(ctx, *capability, rootTy, CS.getKeyPathValueType(keyPath)); updatedBindings.insert( - {keyPathTy, AllowedBindingKind::Exact, keyPathLoc}); + {keyPathTy, AllowedBindingKind::Exact, locator}); } else if (CS.shouldAttemptFixes()) { auto fixedRootTy = CS.getFixedType(rootTy); // If key path is structurally correct and has a resolved root @@ -799,10 +797,14 @@ bool BindingSet::finalize() { Bindings = std::move(updatedBindings); Defaults.clear(); - - return true; } + } + return true; +} + +void BindingSet::finalizeUnresolvedMemberChainResult() { + if (auto *locator = TypeVar->getImpl().getLocator()) { if (CS.shouldAttemptFixes() && locator->isLastElement()) { // Let's see whether this chain is valid, if it isn't then to avoid @@ -825,8 +827,6 @@ bool BindingSet::finalize() { } } } - - return true; } void BindingSet::addBinding(PotentialBinding binding, bool isTransitive) { @@ -1194,9 +1194,10 @@ std::optional ConstraintSystem::determineBestBindings( bindings.inferTransitiveBindings(); - if (!bindings.finalize()) + if (!bindings.finalizeKeyPathBindings()) continue; + bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); if (!bindings.hasViableBindings() && !bindings.isDirectHole()) @@ -1595,7 +1596,9 @@ BindingSet ConstraintSystem::getBindingsFor(TypeVariableType *typeVar) { assert(!typeVar->getImpl().getFixedType(nullptr) && "has a fixed type"); BindingSet bindings(*this, typeVar, CG[typeVar].getPotentialBindings()); - bindings.finalize(); + + (void) bindings.finalizeKeyPathBindings(); + bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); return bindings; diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index f973b9f0610da..b2cfdbcd8f5cf 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -126,7 +126,8 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { cs.getConstraintGraph()[floatLiteralTy].initBindingSet(); bindings.inferTransitiveBindings(); - bindings.finalize(); + (void) bindings.finalizeKeyPathBindings(); + bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); // Inferred a single transitive binding through `$T_float`. diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index c4471a253c9e9..b4c08844e49f2 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -140,9 +140,14 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, continue; auto &bindings = node.getBindingSet(); + + // FIXME: This is also called in inferTransitiveBindings(), why do we need + // to call it again? bindings.inferTransitiveProtocolRequirements(); + bindings.inferTransitiveBindings(); - bindings.finalize(); + (void) bindings.finalizeKeyPathBindings(); + bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); } From 19a2eeb322676a4736eb816085ca061005b6d224 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 09:35:46 -0500 Subject: [PATCH 07/13] Sema: Simplify BindingSet::isDirectHole() --- lib/Sema/CSBindings.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 8450cdc4eeb55..7943e12bb1ec4 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -103,8 +103,7 @@ bool BindingSet::isDirectHole() const { if (!CS.shouldAttemptFixes()) return false; - return Bindings.empty() && getNumViableLiteralBindings() == 0 && - Defaults.empty() && TypeVar->getImpl().canBindToHole(); + return !hasViableBindings() && TypeVar->getImpl().canBindToHole(); } static bool isGenericParameter(TypeVariableType *TypeVar) { From ab72c5310383d2046325a9203f80fa73ce09cd9c Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 13:44:03 -0500 Subject: [PATCH 08/13] Sema: Split off inferTransitiveKeyPathBindings() from BindingSet::inferTransitiveBindings() --- include/swift/Sema/CSBindings.h | 2 ++ lib/Sema/CSBindings.cpp | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 09fbec2cdf686..86eb6fac8a709 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -551,6 +551,8 @@ class BindingSet { /// variables in the workset. void inferTransitiveBindings(); + void inferTransitiveKeyPathBindings(); + /// Detect subtype, conversion or equivalence relationship /// between two type variables and attempt to propagate protocol /// requirements down the subtype or equivalence chain. diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 7943e12bb1ec4..70a6d62e576a9 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -493,9 +493,7 @@ void BindingSet::inferTransitiveProtocolRequirements() { } while (!workList.empty()); } -void BindingSet::inferTransitiveBindings() { - using BindingKind = AllowedBindingKind; - +void BindingSet::inferTransitiveKeyPathBindings() { // If the current type variable represents a key path root type // let's try to transitively infer its type through bindings of // a key path type. @@ -550,7 +548,7 @@ void BindingSet::inferTransitiveBindings() { } } else { addBinding( - binding.withSameSource(inferredRootTy, BindingKind::Exact), + binding.withSameSource(inferredRootTy, AllowedBindingKind::Exact), /*isTransitive=*/true); } } @@ -558,6 +556,10 @@ void BindingSet::inferTransitiveBindings() { } } } +} + +void BindingSet::inferTransitiveBindings() { + inferTransitiveKeyPathBindings(); for (const auto &entry : Info.SupertypeOf) { auto &node = CS.getConstraintGraph()[entry.first]; @@ -608,8 +610,8 @@ void BindingSet::inferTransitiveBindings() { // either be Exact or Supertypes in order for it to make sense // to add Supertype bindings based on the relationship between // our type variables. - if (binding.Kind != BindingKind::Exact && - binding.Kind != BindingKind::Supertypes) + if (binding.Kind != AllowedBindingKind::Exact && + binding.Kind != AllowedBindingKind::Supertypes) continue; auto type = binding.BindingType; @@ -620,7 +622,7 @@ void BindingSet::inferTransitiveBindings() { if (ConstraintSystem::typeVarOccursInType(TypeVar, type)) continue; - addBinding(binding.withSameSource(type, BindingKind::Supertypes), + addBinding(binding.withSameSource(type, AllowedBindingKind::Supertypes), /*isTransitive=*/true); } } From c0238dbb4cb6f056214bf17bf9b964f5278a77a5 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 13:53:01 -0500 Subject: [PATCH 09/13] Sema: Split up BindingSet::inferTransitiveBindings() into three methods --- include/swift/Sema/CSBindings.h | 6 ++++-- lib/Sema/CSBindings.cpp | 10 ++++++---- unittests/Sema/BindingInferenceTests.cpp | 4 +++- unittests/Sema/SemaFixture.cpp | 9 ++++++--- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 86eb6fac8a709..4f106c19821e6 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -540,6 +540,8 @@ class BindingSet { /// Check if this binding is favored over a conjunction. bool favoredOverConjunction(Constraint *conjunction) const; + void inferTransitiveKeyPathBindings(); + /// Detect `subtype` relationship between two type variables and /// attempt to infer supertype bindings transitively e.g. /// @@ -549,9 +551,9 @@ class BindingSet { /// /// \param inferredBindings The set of all bindings inferred for type /// variables in the workset. - void inferTransitiveBindings(); + void inferTransitiveSupertypeBindings(); - void inferTransitiveKeyPathBindings(); + void inferTransitiveUnresolvedMemberRefBindings(); /// Detect subtype, conversion or equivalence relationship /// between two type variables and attempt to propagate protocol diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 70a6d62e576a9..e295845f217d7 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -558,9 +558,7 @@ void BindingSet::inferTransitiveKeyPathBindings() { } } -void BindingSet::inferTransitiveBindings() { - inferTransitiveKeyPathBindings(); - +void BindingSet::inferTransitiveSupertypeBindings() { for (const auto &entry : Info.SupertypeOf) { auto &node = CS.getConstraintGraph()[entry.first]; if (!node.hasBindingSet()) @@ -626,7 +624,9 @@ void BindingSet::inferTransitiveBindings() { /*isTransitive=*/true); } } +} +void BindingSet::inferTransitiveUnresolvedMemberRefBindings() { if (!hasViableBindings()) { if (auto *locator = TypeVar->getImpl().getLocator()) { if (locator->isLastElement()) { @@ -1193,7 +1193,9 @@ std::optional ConstraintSystem::determineBestBindings( // produce a default type. bool isViable = isViableForRanking(bindings); - bindings.inferTransitiveBindings(); + bindings.inferTransitiveKeyPathBindings(); + bindings.inferTransitiveSupertypeBindings(); + bindings.inferTransitiveUnresolvedMemberRefBindings(); if (!bindings.finalizeKeyPathBindings()) continue; diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index b2cfdbcd8f5cf..af8efb0400802 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -125,7 +125,9 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { cs.getConstraintGraph()[floatLiteralTy].initBindingSet(); - bindings.inferTransitiveBindings(); + bindings.inferTransitiveKeyPathBindings(); + bindings.inferTransitiveSupertypeBindings(); + bindings.inferTransitiveUnresolvedMemberRefBindings(); (void) bindings.finalizeKeyPathBindings(); bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index b4c08844e49f2..0dfe5faa9bff9 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -141,11 +141,14 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, auto &bindings = node.getBindingSet(); - // FIXME: This is also called in inferTransitiveBindings(), why do we need - // to call it again? + // FIXME: This is also called in inferTransitiveUnresolvedMemberRefBindings(), + // why do we need to call it here too? bindings.inferTransitiveProtocolRequirements(); - bindings.inferTransitiveBindings(); + bindings.inferTransitiveKeyPathBindings(); + bindings.inferTransitiveSupertypeBindings(); + bindings.inferTransitiveUnresolvedMemberRefBindings(); + (void) bindings.finalizeKeyPathBindings(); bindings.finalizeUnresolvedMemberChainResult(); bindings.determineLiteralCoverage(); From 403a11f79d9ad20ddf55775de4a1975073e0a1c2 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 15:05:34 -0500 Subject: [PATCH 10/13] Sema: Infer key path bindings before calling isViableForRanking() This allows us to remove the key path check from isViableForRanking(). --- lib/Sema/CSBindings.cpp | 20 +++++++++----------- unittests/Sema/BindingInferenceTests.cpp | 5 ++++- unittests/Sema/SemaFixture.cpp | 6 ++++-- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index e295845f217d7..191f6e50d560a 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1150,12 +1150,6 @@ std::optional ConstraintSystem::determineBestBindings( auto isViableForRanking = [this](const BindingSet &bindings) -> bool { auto *typeVar = bindings.getTypeVariable(); - // Key path root type variable is always viable because it can be - // transitively inferred from key path type during binding set - // finalization. - if (typeVar->getImpl().isKeyPathRoot()) - return true; - // Type variable representing a base of unresolved member chain should // always be considered viable for ranking since it's allow to infer // types from transitive protocol requirements. @@ -1181,6 +1175,11 @@ std::optional ConstraintSystem::determineBestBindings( auto &bindings = node.getBindingSet(); + // Special handling for key paths. + bindings.inferTransitiveKeyPathBindings(); + if (!bindings.finalizeKeyPathBindings()) + continue; + // Before attempting to infer transitive bindings let's check // whether there are any viable "direct" bindings associated with // current type variable, if there are none - it means that this type @@ -1193,14 +1192,13 @@ std::optional ConstraintSystem::determineBestBindings( // produce a default type. bool isViable = isViableForRanking(bindings); - bindings.inferTransitiveKeyPathBindings(); bindings.inferTransitiveSupertypeBindings(); - bindings.inferTransitiveUnresolvedMemberRefBindings(); - - if (!bindings.finalizeKeyPathBindings()) - continue; + // Special handling for "leading-dot" unresolved member references, + // like .foo. + bindings.inferTransitiveUnresolvedMemberRefBindings(); bindings.finalizeUnresolvedMemberChainResult(); + bindings.determineLiteralCoverage(); if (!bindings.hasViableBindings() && !bindings.isDirectHole()) diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index af8efb0400802..60babf20dfdb4 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -126,10 +126,13 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { cs.getConstraintGraph()[floatLiteralTy].initBindingSet(); bindings.inferTransitiveKeyPathBindings(); + (void) bindings.finalizeKeyPathBindings(); + bindings.inferTransitiveSupertypeBindings(); + bindings.inferTransitiveUnresolvedMemberRefBindings(); - (void) bindings.finalizeKeyPathBindings(); bindings.finalizeUnresolvedMemberChainResult(); + bindings.determineLiteralCoverage(); // Inferred a single transitive binding through `$T_float`. diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index 0dfe5faa9bff9..79d056bf42b2f 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -146,11 +146,13 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, bindings.inferTransitiveProtocolRequirements(); bindings.inferTransitiveKeyPathBindings(); + (void) bindings.finalizeKeyPathBindings(); + bindings.inferTransitiveSupertypeBindings(); - bindings.inferTransitiveUnresolvedMemberRefBindings(); - (void) bindings.finalizeKeyPathBindings(); + bindings.inferTransitiveUnresolvedMemberRefBindings(); bindings.finalizeUnresolvedMemberChainResult(); + bindings.determineLiteralCoverage(); } From b7e0083478dd8e818e1e0c1e480b2357bc2d1f32 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 15:16:53 -0500 Subject: [PATCH 11/13] Sema: Handle leading dot before calling isViableForRanking() --- lib/Sema/CSBindings.cpp | 18 +++++------------- unittests/Sema/BindingInferenceTests.cpp | 4 ++-- unittests/Sema/SemaFixture.cpp | 4 ++-- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 191f6e50d560a..94976edeba63d 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1150,14 +1150,6 @@ std::optional ConstraintSystem::determineBestBindings( auto isViableForRanking = [this](const BindingSet &bindings) -> bool { auto *typeVar = bindings.getTypeVariable(); - // Type variable representing a base of unresolved member chain should - // always be considered viable for ranking since it's allow to infer - // types from transitive protocol requirements. - if (auto *locator = typeVar->getImpl().getLocator()) { - if (locator->isLastElement()) - return true; - } - // If type variable is marked as a potential hole there is always going // to be at least one binding available for it. if (shouldAttemptFixes() && typeVar->getImpl().canBindToHole()) @@ -1180,6 +1172,11 @@ std::optional ConstraintSystem::determineBestBindings( if (!bindings.finalizeKeyPathBindings()) continue; + // Special handling for "leading-dot" unresolved member references, + // like .foo. + bindings.inferTransitiveUnresolvedMemberRefBindings(); + bindings.finalizeUnresolvedMemberChainResult(); + // Before attempting to infer transitive bindings let's check // whether there are any viable "direct" bindings associated with // current type variable, if there are none - it means that this type @@ -1194,11 +1191,6 @@ std::optional ConstraintSystem::determineBestBindings( bindings.inferTransitiveSupertypeBindings(); - // Special handling for "leading-dot" unresolved member references, - // like .foo. - bindings.inferTransitiveUnresolvedMemberRefBindings(); - bindings.finalizeUnresolvedMemberChainResult(); - bindings.determineLiteralCoverage(); if (!bindings.hasViableBindings() && !bindings.isDirectHole()) diff --git a/unittests/Sema/BindingInferenceTests.cpp b/unittests/Sema/BindingInferenceTests.cpp index 60babf20dfdb4..eef3bb841ac17 100644 --- a/unittests/Sema/BindingInferenceTests.cpp +++ b/unittests/Sema/BindingInferenceTests.cpp @@ -128,11 +128,11 @@ TEST_F(SemaTest, TestIntLiteralBindingInference) { bindings.inferTransitiveKeyPathBindings(); (void) bindings.finalizeKeyPathBindings(); - bindings.inferTransitiveSupertypeBindings(); - bindings.inferTransitiveUnresolvedMemberRefBindings(); bindings.finalizeUnresolvedMemberChainResult(); + bindings.inferTransitiveSupertypeBindings(); + bindings.determineLiteralCoverage(); // Inferred a single transitive binding through `$T_float`. diff --git a/unittests/Sema/SemaFixture.cpp b/unittests/Sema/SemaFixture.cpp index 79d056bf42b2f..0600e9ad5929e 100644 --- a/unittests/Sema/SemaFixture.cpp +++ b/unittests/Sema/SemaFixture.cpp @@ -148,11 +148,11 @@ BindingSet SemaTest::inferBindings(ConstraintSystem &cs, bindings.inferTransitiveKeyPathBindings(); (void) bindings.finalizeKeyPathBindings(); - bindings.inferTransitiveSupertypeBindings(); - bindings.inferTransitiveUnresolvedMemberRefBindings(); bindings.finalizeUnresolvedMemberChainResult(); + bindings.inferTransitiveSupertypeBindings(); + bindings.determineLiteralCoverage(); } From fb0e08408ea81d1718981b6180af0c3d00e90bdd Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 14 Nov 2025 15:21:13 -0500 Subject: [PATCH 12/13] Sema: Sema: Remove isViableForRanking() from determineBestBindings() --- lib/Sema/CSBindings.cpp | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 94976edeba63d..796e3c885aa4f 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1141,23 +1141,6 @@ std::optional ConstraintSystem::determineBestBindings( node.initBindingSet(); } - // Determine whether given type variable with its set of bindings is - // viable to be attempted on the next step of the solver. If type variable - // has no "direct" bindings of any kind e.g. direct bindings to concrete - // types, default types from "defaultable" constraints or literal - // conformances, such type variable is not viable to be evaluated to be - // attempted next. - auto isViableForRanking = [this](const BindingSet &bindings) -> bool { - auto *typeVar = bindings.getTypeVariable(); - - // If type variable is marked as a potential hole there is always going - // to be at least one binding available for it. - if (shouldAttemptFixes() && typeVar->getImpl().canBindToHole()) - return true; - - return bindings.hasViableBindings() || bindings.isDirectHole(); - }; - // Now let's see if we could infer something for related type // variables based on other bindings. for (auto *typeVar : getTypeVariables()) { @@ -1187,15 +1170,11 @@ std::optional ConstraintSystem::determineBestBindings( // associated with given type variable, any default constraints, // or any conformance requirements to literal protocols with can // produce a default type. - bool isViable = isViableForRanking(bindings); + bool isViable = bindings.hasViableBindings() || bindings.isDirectHole(); bindings.inferTransitiveSupertypeBindings(); - bindings.determineLiteralCoverage(); - if (!bindings.hasViableBindings() && !bindings.isDirectHole()) - continue; - if (!isViable) continue; From ce3b5eb8aeca97dcf719534e2ec45c2ce19c693e Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Mon, 17 Nov 2025 11:13:46 -0500 Subject: [PATCH 13/13] Sema: Bring back BindingSet::operator bool but call it BindingSet::isViable() --- include/swift/Sema/CSBindings.h | 6 ++++++ lib/Sema/CSBindings.cpp | 2 +- lib/Sema/ConstraintGraph.cpp | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index 4f106c19821e6..825990660c37b 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -482,6 +482,12 @@ class BindingSet { !Defaults.empty(); } + /// Determine whether this set can be chosen as the next binding set + /// to attempt. + bool isViable() const { + return hasViableBindings() || isDirectHole(); + } + ArrayRef getConformanceRequirements() const { return Protocols; } diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 796e3c885aa4f..785d5f8d87c6a 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1170,7 +1170,7 @@ std::optional ConstraintSystem::determineBestBindings( // associated with given type variable, any default constraints, // or any conformance requirements to literal protocols with can // produce a default type. - bool isViable = bindings.hasViableBindings() || bindings.isDirectHole(); + bool isViable = bindings.isViable(); bindings.inferTransitiveSupertypeBindings(); bindings.determineLiteralCoverage(); diff --git a/lib/Sema/ConstraintGraph.cpp b/lib/Sema/ConstraintGraph.cpp index a8909d352392f..346e2e175909d 100644 --- a/lib/Sema/ConstraintGraph.cpp +++ b/lib/Sema/ConstraintGraph.cpp @@ -922,7 +922,7 @@ bool ConstraintGraph::contractEdges() { if (tyvar1->getImpl().canBindToInOut()) { bool isNotContractable = true; auto bindings = CS.getBindingsFor(tyvar1); - if (bindings.hasViableBindings() || bindings.isDirectHole()) { + if (bindings.isViable()) { // Holes can't be contracted. if (bindings.isHole()) continue;