From bb30772037c4c1d2a53b1878211c17d1242f6b32 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 19 Oct 2025 19:41:34 +0100 Subject: [PATCH 1/4] [CS] Always eagerly bind member type var to hole for missing member We know this is where the issue is so we ought to always produce a concrete hole. --- lib/Sema/CSFix.cpp | 6 +++++- lib/Sema/CSSimplify.cpp | 19 ++++--------------- test/Constraints/closures.swift | 2 +- test/type/protocol_composition.swift | 5 +---- 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 6c734b8937643..c6e75f3d4cacc 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -939,7 +939,11 @@ DefineMemberBasedOnUse::diagnoseForAmbiguity(CommonFixesArray commonFixes) const const auto *solution = solutionAndFix.first; const auto *fix = solutionAndFix.second->getAs(); - auto baseType = solution->simplifyType(fix->BaseType); + // Ignore differences in optionality since we can look through an optional + // to resolve an implicit member `.foo`. + auto baseType = solution->simplifyType(fix->BaseType) + ->getMetatypeInstanceType() + ->lookThroughAllOptionalTypes(); if (!concreteBaseType) concreteBaseType = baseType; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 4cf84fecad1e8..00b1cdf87affb 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -11605,22 +11605,11 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint( // `key path` constraint can't be retired until all components // are simplified. addTypeVariableConstraintsToWorkList(memberTypeVar); - } else if (isa(locator->getAnchor()) && - !getSemanticsProvidingParentExpr( - getAsExpr(locator->getAnchor()))) { - // If there are no contextual expressions that could provide - // a type for the member type variable, let's default it to - // a placeholder eagerly so it could be propagated to the - // pattern if necessary. - recordTypeVariablesAsHoles(memberTypeVar); - } else if (locator->isLastElement()) { - // Let's handle member patterns specifically because they use - // equality instead of argument application constraint, so allowing - // them to bind member could mean missing valid hole positions in - // the pattern. - recordTypeVariablesAsHoles(memberTypeVar); } else { - recordPotentialHole(memberTypeVar); + // Eagerly turn the member type variable into a hole since we know + // this is where the issue is and we've recorded a fix for it. This + // avoids producing unnecessary holes for e.g generic parameters. + recordTypeVariablesAsHoles(memberTypeVar); } } diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift index 941317f3ac956..7f2e6020001cf 100644 --- a/test/Constraints/closures.swift +++ b/test/Constraints/closures.swift @@ -1362,7 +1362,7 @@ do { } } - test { // expected-error {{invalid conversion from throwing function of type '(Int) throws -> Void' to non-throwing function type '(Int) -> Void'}} + test { // expected-error {{invalid conversion from throwing function of type '(Int) throws -> _' to non-throwing function type '(Int) -> Void'}} try $0.missing // expected-error {{value of type 'Int' has no member 'missing'}} } } diff --git a/test/type/protocol_composition.swift b/test/type/protocol_composition.swift index f138c2681decb..a04093655b87f 100644 --- a/test/type/protocol_composition.swift +++ b/test/type/protocol_composition.swift @@ -197,10 +197,7 @@ takesP1AndP2([DoesNotExist & P1 & P2]()) // expected-error {{cannot find 'DoesNo // expected-note@-3 2 {{overloads for '&' exist with these partially matching parameter lists}} // expected-error@-4 {{cannot call value of non-function type '[UInt8]'}} takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}} -// expected-error@-1 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P2).Type'}} -// expected-error@-2 {{binary operator '&' cannot be applied to operands of type 'UInt8' and '(any P1).Type'}} -// expected-note@-3 2 {{overloads for '&' exist with these partially matching parameter lists}} -// expected-error@-4 {{cannot call value of non-function type '[UInt8]'}} +// expected-error@-1 {{cannot call value of non-function type '[Element]'}} typealias T08 = P1 & inout P2 // expected-error {{'inout' may only be used on parameters}} typealias T09 = P1 & __shared P2 // expected-error {{'__shared' may only be used on parameters}} From f7092dd1e724661966e739000a1ab04f9f4428dd Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 19 Oct 2025 19:41:34 +0100 Subject: [PATCH 2/4] [CS] Improve applicable fn handling of hole function type Unwrap a metatype before checking if we have an associated fix. --- lib/Sema/CSSimplify.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 00b1cdf87affb..2e7cc19814a99 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -13651,16 +13651,13 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyApplicableFnConstraint( // a fix, let's propagate holes to the "input" type. Doing so // provides more information to upcoming argument and result matching. if (shouldAttemptFixes()) { - if (auto *typeVar = type2->getAs()) { + Type underlyingType = desugar2->getMetatypeInstanceType(); + if (auto *typeVar = underlyingType->getAs()) { auto *locator = typeVar->getImpl().getLocator(); if (hasFixFor(locator)) { recordAnyTypeVarAsPotentialHole(func1); } } - Type underlyingType = desugar2; - while (auto *MT = underlyingType->getAs()) { - underlyingType = MT->getInstanceType(); - } underlyingType = getFixedTypeRecursive(underlyingType, flags, /*wantRValue=*/true); if (underlyingType->isPlaceholder()) { From 10e1cdf4563997aa9bf03f8f449886f6cf06eb25 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 19 Oct 2025 19:41:34 +0100 Subject: [PATCH 3/4] [CS] NFC: Split out locator checks for `isClosure(Parameter|Result)Type` --- include/swift/Sema/ConstraintLocator.h | 7 +++++++ lib/Sema/ConstraintLocator.cpp | 16 ++++++++++++++++ lib/Sema/TypeCheckConstraints.cpp | 12 ++---------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/include/swift/Sema/ConstraintLocator.h b/include/swift/Sema/ConstraintLocator.h index 3b6052e6df2ef..c3fa0940af9cb 100644 --- a/include/swift/Sema/ConstraintLocator.h +++ b/include/swift/Sema/ConstraintLocator.h @@ -233,6 +233,13 @@ class ConstraintLocator : public llvm::FoldingSetNode { /// e.g. `foo[0]` or `\Foo.[0]` bool isSubscriptMemberRef() const; + /// Determine whether this locator represents one of the parameter types + /// associated with a closure. + bool isClosureParameterType() const; + + /// Determine whether this locator represents a closure result type. + bool isClosureResultType() const; + /// Determine whether give locator points to the type of the /// key path expression. bool isKeyPathType() const; diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index 2cf038ea98be0..f4e98a40b98cc 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -560,6 +560,22 @@ bool ConstraintLocator::isSubscriptMemberRef() const { return path.back().getKind() == ConstraintLocator::SubscriptMember; } +bool ConstraintLocator::isClosureParameterType() const { + if (!getAnchor()) + return false; + + return isExpr(getAnchor()) && + isLastElement(); +} + +bool ConstraintLocator::isClosureResultType() const { + if (!getAnchor()) + return false; + + return isExpr(getAnchor()) && + isLastElement(); +} + bool ConstraintLocator::isKeyPathType() const { auto anchor = getAnchor(); auto path = getPath(); diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 9d3c4fe2310e2..9b57567df6a23 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -117,19 +117,11 @@ bool TypeVariableType::Implementation::isTapType() const { } bool TypeVariableType::Implementation::isClosureParameterType() const { - if (!(locator && locator->getAnchor())) - return false; - - return isExpr(locator->getAnchor()) && - locator->isLastElement(); + return locator && locator->isClosureParameterType(); } bool TypeVariableType::Implementation::isClosureResultType() const { - if (!(locator && locator->getAnchor())) - return false; - - return isExpr(locator->getAnchor()) && - locator->isLastElement(); + return locator && locator->isClosureResultType(); } bool TypeVariableType::Implementation::isKeyPathType() const { From d8719abfa6ab48a8d451bd9e0154c079bbb067ab Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 19 Oct 2025 19:41:34 +0100 Subject: [PATCH 4/4] [CS] Handle holes in `mergeEquivalenceClasses` Make sure we preserve `TVO_CanBindToHole`. This unfortunately requires introducing the concept of a "non-representative" hole for representatives where the actual hole is given by another type variable in the equivalence class. This avoids disturbing binding order and regressing diagnostics. Once binding is less sensitive to ordering we ought to be able to always choose the hole as the representative. --- include/swift/AST/Types.h | 6 +-- include/swift/Sema/ConstraintSystem.h | 43 +++++++++++++++++-- lib/Sema/CSBindings.cpp | 20 +++++---- lib/Sema/ConstraintSystem.cpp | 31 ++++++++++++- test/Constraints/diagnostics.swift | 10 +++++ test/Constraints/rdar45511837.swift | 4 +- .../Sema/SwiftUI/rdar88256059.swift | 3 +- 7 files changed, 95 insertions(+), 22 deletions(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index 8d6496dfe8ca1..0c09aa2c31154 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -454,11 +454,11 @@ class alignas(1 << TypeAlignInBits) TypeBase NumProtocols : 16 ); - SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 7+28, + SWIFT_INLINE_BITFIELD_FULL(TypeVariableType, TypeBase, 8+26, /// Type variable options. - Options : 7, + Options : 8, /// The unique number assigned to this type variable. - ID : 27 + ID : 26 ); SWIFT_INLINE_BITFIELD_FULL(ErrorUnionType, TypeBase, 32, diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 146126fd8a4f3..549fcea65556e 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -318,6 +318,14 @@ enum TypeVariableOptions { /// Whether the type variable can be bound only to a pack expansion type. TVO_PackExpansion = 0x40, + + /// Whether the hole for this type variable origates from a different + /// type variable in its equivalence class. + /// FIXME: This is an unfortunate hack due to the fact that choosing a + /// different representative changes binding order and regresses a bunch of + /// test cases. Once binding order is made more resilient we ought to be able + /// to remove this. + TVO_NonRepresentativeHole = 0x80, }; enum class KeyPathMutability : uint8_t { @@ -397,6 +405,12 @@ class TypeVariableType::Implementation { /// Whether this type variable can bind only to PackExpansionType. bool isPackExpansion() const { return getRawOptions() & TVO_PackExpansion; } + /// Whether the hole for this type variable origates from a different + /// type variable in its equivalence class. + bool isNonRepresentativeHole() const { + return getRawOptions() & TVO_NonRepresentativeHole; + } + /// Whether this type variable prefers a subtype binding over a supertype /// binding. bool prefersSubtypeBinding() const { @@ -565,28 +579,40 @@ class TypeVariableType::Implementation { otherRep->getImpl().recordBinding(*trail); otherRep->getImpl().ParentOrFixed = getTypeVariable(); + auto &bits = getTypeVariable()->Bits.TypeVariableType; + if (canBindToLValue() && !otherRep->getImpl().canBindToLValue()) { if (trail) recordBinding(*trail); - getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToLValue; + bits.Options &= ~TVO_CanBindToLValue; } if (canBindToInOut() && !otherRep->getImpl().canBindToInOut()) { if (trail) recordBinding(*trail); - getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToInOut; + bits.Options &= ~TVO_CanBindToInOut; } if (canBindToNoEscape() && !otherRep->getImpl().canBindToNoEscape()) { if (trail) recordBinding(*trail); - getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToNoEscape; + bits.Options &= ~TVO_CanBindToNoEscape; } if (canBindToPack() && !otherRep->getImpl().canBindToPack()) { if (trail) recordBinding(*trail); - getTypeVariable()->Bits.TypeVariableType.Options &= ~TVO_CanBindToPack; + bits.Options &= ~TVO_CanBindToPack; + } + + // If we're merging a non-hole with a hole, add the flag to allow binding + // to a hole. This type variable then becomes a "non-representative" hole, + // which is an unfortunate hack necessary to preserve binding order. See + // the comment on `TVO_NonRepresentativeHole` for more info. + if (!canBindToHole() && otherRep->getImpl().canBindToHole()) { + if (trail) + recordBinding(*trail); + bits.Options |= (TVO_CanBindToHole | TVO_NonRepresentativeHole); } } @@ -701,6 +727,7 @@ class TypeVariableType::Implementation { ENTRY(TVO_PrefersSubtypeBinding, ""); ENTRY(TVO_CanBindToPack, "pack"); ENTRY(TVO_PackExpansion, "pack expansion"); + ENTRY(TVO_NonRepresentativeHole, "non-rep hole"); } #undef ENTRY } @@ -3419,6 +3446,14 @@ class ConstraintSystem { getImplicitValueConversionLocator(ConstraintLocatorBuilder root, ConversionRestrictionKind restriction); + /// Retrieve the type variable that represents the originating hole in the + /// equivalence class for a given type variable. + TypeVariableType *getHoleTypeVar(TypeVariableType *tv); + + /// Retrieve the locator for the hole that represents the originating hole in + /// the equivalence class for a given type variable. + ConstraintLocator *getHoleLocator(TypeVariableType *tv); + /// Lookup and return parent associated with given expression. Expr *getParentExpr(Expr *expr) { if (auto result = getExprDepthAndParent(expr)) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 69c2db62c6edc..f21957508d234 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -169,7 +169,7 @@ bool BindingSet::isDelayed() const { return true; if (isHole()) { - auto *locator = TypeVar->getImpl().getLocator(); + auto *locator = CS.getHoleLocator(TypeVar); assert(locator && "a hole without locator?"); // Delay resolution of the code completion expression until @@ -2745,7 +2745,7 @@ bool TypeVarBindingProducer::computeNext() { std::optional> TypeVariableBinding::fixForHole(ConstraintSystem &cs) const { - auto *dstLocator = TypeVar->getImpl().getLocator(); + auto *dstLocator = cs.getHoleLocator(TypeVar); auto *srcLocator = Binding.getLocator(); // FIXME: This check could be turned into an assert once @@ -2766,7 +2766,7 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const { unsigned defaultImpact = 1; - if (auto *GP = TypeVar->getImpl().getGenericParameter()) { + if (auto *GP = dstLocator->getGenericParameter()) { // If it is representative for a key path root, let's emit a more // specific diagnostic. auto *keyPathRoot = @@ -2786,12 +2786,12 @@ TypeVariableBinding::fixForHole(ConstraintSystem &cs) const { } } - if (TypeVar->getImpl().isClosureParameterType()) { + if (dstLocator->isClosureParameterType()) { ConstraintFix *fix = SpecifyClosureParameterType::create(cs, dstLocator); return std::make_pair(fix, defaultImpact); } - if (TypeVar->getImpl().isClosureResultType()) { + if (dstLocator->isClosureResultType()) { auto *closure = castToExpr(dstLocator->getAnchor()); // If the whole body is being ignored due to a pre-check failure, // let's not record a fix about result type since there is @@ -2910,15 +2910,17 @@ static bool shouldIgnoreHoleForCodeCompletion(ConstraintSystem &cs, ConstraintLocator *srcLocator) { if (!cs.isForCodeCompletion()) return false; - + + auto *holeLoc = cs.getHoleLocator(typeVar); + // Don't penalize solutions with unresolved generics. - if (typeVar->getImpl().getGenericParameter()) + if (holeLoc->getGenericParameter()) return true; // Don't penalize solutions if we couldn't determine the type of the code // completion token. We still want to examine the surrounding types in // that case. - if (typeVar->getImpl().isCodeCompletionToken()) + if (holeLoc->directlyAt()) return true; // When doing completion in a result builder, we avoid solving unrelated @@ -2946,7 +2948,7 @@ static bool shouldIgnoreHoleForCodeCompletion(ConstraintSystem &cs, if (cs.hasArgumentsIgnoredForCodeCompletion()) { // Avoid simplifying the locator if the constraint system didn't ignore // any arguments. - auto argExpr = simplifyLocatorToAnchor(typeVar->getImpl().getLocator()); + auto argExpr = simplifyLocatorToAnchor(holeLoc); if (cs.isArgumentIgnoredForCodeCompletion(argExpr.dyn_cast())) { return true; } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 0cc585646950d..cd0100ad4b6db 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -972,6 +972,31 @@ void ConstraintSystem::restoreType(const KeyPathExpr *KP, unsigned I, Type T) { } } +TypeVariableType *ConstraintSystem::getHoleTypeVar(TypeVariableType *tv) { + if (!tv->getImpl().isNonRepresentativeHole()) + return tv; + + // If we have a non-representative hole, the hole type var is given by + // a member of its equivalence class. Pick the the one with the lowest ID to + // ensure consistency. + TypeVariableType *candidate = nullptr; + for (auto equiv : CG[tv].getEquivalenceClass()) { + auto &impl = equiv->getImpl(); + if (equiv == tv || !impl.canBindToHole() || impl.isNonRepresentativeHole()) + continue; + + if (!candidate || candidate->getID() > equiv->getID()) + candidate = equiv; + } + ASSERT(candidate && + "Non-representative hole ought to have hole in its equivalence class"); + return candidate; +} + +ConstraintLocator *ConstraintSystem::getHoleLocator(TypeVariableType *tv) { + return getHoleTypeVar(tv)->getImpl().getLocator(); +} + std::pair ConstraintSystem::openAnyExistentialType(Type type, ConstraintLocator *locator) { @@ -5320,7 +5345,9 @@ TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings) bindings.getTypeVariable()->getImpl().getLocator()), TypeVar(bindings.getTypeVariable()), CanBeNil(bindings.canBeNil()) { if (bindings.isDirectHole()) { - auto *locator = getLocator(); + auto *holeTV = CS.getHoleTypeVar(TypeVar); + auto *locator = holeTV->getImpl().getLocator(); + // If this type variable is associated with a code completion token // and it failed to infer any bindings let's adjust holes's locator // to point to a code completion token to avoid attempting to "fix" @@ -5331,7 +5358,7 @@ TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings) CS.getConstraintLocator(bindings.getAssociatedCodeCompletionToken()); } - Bindings.push_back(Binding::forHole(TypeVar, locator)); + Bindings.push_back(Binding::forHole(holeTV, locator)); return; } diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index 2f20663e5a060..f80c8bd7cbb71 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -1585,3 +1585,13 @@ do { let x: String = 0 // expected-error {{cannot convert value of type 'Int' to specified type 'String'}} }. // expected-error {{expected member name following '.'}} } + +func testArrayLiteralMetatypeMismatch() { + // Make sure we can correctly turn 'T' into a hole here. + func foo(_ xs: T.Type, _: Int) -> T {} // expected-note {{in call to function 'foo'}} + func foo(_ i: Int) { + let x = foo([], i) + // expected-error@-1 {{generic parameter 'T' could not be inferred}} + // expected-error@-2 {{cannot convert value of type '[Any]' to expected argument type 'T.Type'}} + } +} diff --git a/test/Constraints/rdar45511837.swift b/test/Constraints/rdar45511837.swift index 0bd2524aebda9..21bbbfbda3de0 100644 --- a/test/Constraints/rdar45511837.swift +++ b/test/Constraints/rdar45511837.swift @@ -18,8 +18,6 @@ class Foo { lazy var foo: () -> Void = { // TODO: improve diagnostic message - _ = self.foobar + nil // expected-error {{'Bar' is not convertible to 'String'}} - // expected-note@-1 {{did you mean to use 'as!' to force downcast?}} - // expected-error@-2 {{'nil' is not compatible with expected argument type 'String'}} + _ = self.foobar + nil // expected-error {{generic parameter 'Self' could not be inferred}} } } diff --git a/validation-test/Sema/SwiftUI/rdar88256059.swift b/validation-test/Sema/SwiftUI/rdar88256059.swift index bdb3a63d56439..c000770b43d4c 100644 --- a/validation-test/Sema/SwiftUI/rdar88256059.swift +++ b/validation-test/Sema/SwiftUI/rdar88256059.swift @@ -9,7 +9,8 @@ struct MyView: View { var body: some View { Table(self.data) { - // expected-error@-1 {{expected expression of type 'Columns' in result builder 'TableColumnBuilder'}} {{23-23=<#T##Columns#>}} + // expected-error@-1 {{expected expression of type 'Column' in result builder 'TableColumnBuilder'}} {{23-23=<#T##Column#>}} + // expected-error@-2 {{generic parameter 'Column' could not be inferred}} } } }