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/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/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/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..2e7cc19814a99 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); } } @@ -13662,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()) { 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/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/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 { 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/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/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}} 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}} } } }