From 16e0ce741ba043d7994b2771eb748b2f13ad90a6 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:37 +0100 Subject: [PATCH 01/11] [CS] Fix a couple of constraints in `getTypeForPattern` The TypedPattern and IsPattern constraints were incorrectly written, with conversions propagating out of the patterns, when really conversions ought to propagate into patterns. In any case, it seems like we really want equality here. Fix the constraints to use equality, and have the cast constraint operate on the external pattern type instead of the subpattern type. --- lib/Sema/CSGen.cpp | 33 +++++++++---------- lib/Sema/CSSimplify.cpp | 3 +- test/Parse/matching_patterns.swift | 1 + ...matching_patterns_reference_bindings.swift | 1 + test/decl/var/property_wrappers.swift | 2 +- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 4c9de01f75a0f..20a745c7fd595 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2714,7 +2714,7 @@ namespace { return Type(); CS.addConstraint( - ConstraintKind::Conversion, subPatternType, openedType, + ConstraintKind::Equal, subPatternType, openedType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); // FIXME [OPAQUE SUPPORT]: the distinction between where we want opaque @@ -2805,21 +2805,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); if (!castType) return Type(); - auto *subPattern = isPattern->getSubPattern(); - Type subPatternType = getTypeForPattern( - subPattern, - locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - castType, bindPatternVarsOneWay); - - if (!subPatternType) - return Type(); - - // Make sure we can cast from the subpattern type to the type we're - // checking; if it's impossible, fail. - CS.addConstraint( - ConstraintKind::CheckedCast, subPatternType, castType, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); - // Allow `is` pattern to infer type from context which is then going // to be propaged down to its sub-pattern via conversion. This enables // correct handling of patterns like `_ as Foo` where `_` would @@ -2828,9 +2813,23 @@ namespace { auto isType = CS.createTypeVariable(CS.getConstraintLocator(pattern), TVO_CanBindToNoEscape | TVO_CanBindToHole); + + // Make sure we can cast from the subpattern type to the type we're + // checking; if it's impossible, fail. CS.addConstraint( - ConstraintKind::Conversion, subPatternType, isType, + ConstraintKind::CheckedCast, isType, castType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + + if (auto *subPattern = isPattern->getSubPattern()) { + auto subPatternType = getTypeForPattern( + subPattern, + locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), + castType, bindPatternVarsOneWay); + + CS.addConstraint( + ConstraintKind::Equal, subPatternType, castType, + locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + } return setType(isType); } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 5ad0372096024..a2eddaf813350 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14786,8 +14786,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( if (recordFix(fix)) return SolutionKind::Error; - (void)matchTypes(type1, OptionalType::get(type2), - ConstraintKind::Conversion, + (void)matchTypes(type1, OptionalType::get(type2), ConstraintKind::Equal, TypeMatchFlags::TMF_ApplyingFix, locator); return SolutionKind::Solved; diff --git a/test/Parse/matching_patterns.swift b/test/Parse/matching_patterns.swift index 932fb84e550c8..1fbec74a906e8 100644 --- a/test/Parse/matching_patterns.swift +++ b/test/Parse/matching_patterns.swift @@ -307,6 +307,7 @@ do { while case let _ as [Derived] = arr {} // expected-warning@-1 {{'let' pattern has no effect; sub-pattern didn't bind any variables}} + // https://github.com/apple/swift/issues/61850 for case _ as [Derived] in [arr] {} if case is [Derived] = arr {} diff --git a/test/Parse/matching_patterns_reference_bindings.swift b/test/Parse/matching_patterns_reference_bindings.swift index 906bea74dfae0..fadfd53ac9821 100644 --- a/test/Parse/matching_patterns_reference_bindings.swift +++ b/test/Parse/matching_patterns_reference_bindings.swift @@ -328,6 +328,7 @@ do { while case let _ as [Derived] = arr {} // expected-warning@-1 {{'let' pattern has no effect; sub-pattern didn't bind any variables}} + // https://github.com/apple/swift/issues/61850 for case _ as [Derived] in [arr] {} if case is [Derived] = arr {} diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index fa33d07393543..33acdbf13df9a 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -1338,7 +1338,7 @@ struct MissingPropertyWrapperUnwrap { struct InvalidPropertyDelegateUse { // TODO(diagnostics): We need to a tailored diagnostic for extraneous arguments in property delegate initialization - @Foo var x: Int = 42 // expected-error@:21 {{argument passed to call that takes no arguments}} + @Foo var x: Int = 42 // expected-error@:21 {{extra argument 'wrappedValue' in call}} func test() { self.x.foo() // expected-error {{value of type 'Int' has no member 'foo'}} From 9c2135e62c27cd9aded53cf40def21addf2afa60 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:37 +0100 Subject: [PATCH 02/11] [CS] Reverse the type order in a couple of pattern equality constraints Order them such that if they were changed to conversions, they would be sound. This shouldn't make a difference, but unfortunately it turns out pattern equality is not symmetric. As such, tweak the pattern equality logic to account for the reversed types. This allows us to remove a special case from function matching. --- lib/Sema/CSGen.cpp | 18 +++++++++++++++--- lib/Sema/CSSimplify.cpp | 32 ++++++++++++++------------------ 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 20a745c7fd595..4f7c7499a2e96 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2713,8 +2713,11 @@ namespace { if (!subPatternType) return Type(); + // NOTE: The order here is important! Pattern matching equality is + // not symmetric (we need to fix that either by using a different + // constraint, or actually making it symmetric). CS.addConstraint( - ConstraintKind::Equal, subPatternType, openedType, + ConstraintKind::Equal, openedType, subPatternType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); // FIXME [OPAQUE SUPPORT]: the distinction between where we want opaque @@ -2826,8 +2829,11 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), castType, bindPatternVarsOneWay); + // NOTE: The order here is important! Pattern matching equality is + // not symmetric (we need to fix that either by using a different + // constraint, or actually making it symmetric). CS.addConstraint( - ConstraintKind::Equal, subPatternType, castType, + ConstraintKind::Equal, castType, subPatternType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); } return setType(isType); @@ -2956,7 +2962,13 @@ namespace { // FIXME: Verify ExtInfo state is correct, not working by accident. FunctionType::ExtInfo info; Type functionType = FunctionType::get(params, outputType, info); - // TODO: Convert to FunctionInput/FunctionResult constraints. + + // TODO: Convert to own constraint? Note that ApplicableFn isn't quite + // right, as pattern matching has data flowing *into* the apply result + // and call arguments, not the other way around. + // NOTE: The order here is important! Pattern matching equality is + // not symmetric (we need to fix that either by using a different + // constraint, or actually making it symmetric). CS.addConstraint( ConstraintKind::Equal, functionType, memberType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index a2eddaf813350..f5d4e5f965c39 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2157,7 +2157,9 @@ class TupleMatcher { const auto &elt2 = tuple2->getElement(i); if (inPatternMatchingContext) { - if (elt1.hasName() && elt1.getName() != elt2.getName()) + // FIXME: The fact that this isn't symmetric is wrong since this logic + // is called for bind and equal constraints... + if (elt2.hasName() && elt1.getName() != elt2.getName()) return true; } else { // If the names don't match, we have a conflict. @@ -3515,21 +3517,12 @@ ConstraintSystem::matchFunctionTypes(FunctionType *func1, FunctionType *func2, // FIXME: We should check value ownership too, but it's not completely // trivial because of inout-to-pointer conversions. - // For equality contravariance doesn't matter, but let's make sure - // that types are matched in original order because that is important - // when function types are equated as part of pattern matching. - auto paramType1 = kind == ConstraintKind::Equal ? func1Param.getOldType() - : func2Param.getOldType(); - - auto paramType2 = kind == ConstraintKind::Equal ? func2Param.getOldType() - : func1Param.getOldType(); - - // Compare the parameter types. - auto result = matchTypes(paramType1, paramType2, subKind, subflags, - (func1Params.size() == 1 - ? argumentLocator - : argumentLocator.withPathElement( - LocatorPathElt::TupleElement(i)))); + // Compare the parameter types, taking contravariance into account. + auto result = matchTypes( + func2Param.getOldType(), func1Param.getOldType(), subKind, subflags, + (func1Params.size() == 1 ? argumentLocator + : argumentLocator.withPathElement( + LocatorPathElt::TupleElement(i)))); if (result.isFailure()) return result; } @@ -6394,7 +6387,7 @@ bool ConstraintSystem::repairFailures( if (auto *OA = var->getAttrs().getAttribute()) ROK = OA->get(); - if (!rhs->getOptionalObjectType() && + if (!lhs->getOptionalObjectType() && optionalityOf(ROK) == ReferenceOwnershipOptionality::Required) { conversionsOrFixes.push_back( AllowNonOptionalWeak::create(*this, getConstraintLocator(NP))); @@ -14786,7 +14779,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( if (recordFix(fix)) return SolutionKind::Error; - (void)matchTypes(type1, OptionalType::get(type2), ConstraintKind::Equal, + // NOTE: The order here is important! Pattern matching equality is + // not symmetric (we need to fix that either by using a different + // constraint, or actually making it symmetric). + (void)matchTypes(OptionalType::get(type1), type2, ConstraintKind::Equal, TypeMatchFlags::TMF_ApplyingFix, locator); return SolutionKind::Solved; From 543eac43f89ff1c2175a3fd5e5a64567a18e7a56 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 03/11] [CS] Remove external type logic from `getTypeForPattern` This shouldn't be necessary, we should be able to solve with type variables instead. This makes sure we don't end up with weird special cases that only occur when an external type is present. --- lib/Sema/CSDiagnostics.cpp | 2 +- lib/Sema/CSGen.cpp | 87 ++++----------------------------- test/Constraints/patterns.swift | 4 ++ 3 files changed, 15 insertions(+), 78 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index cf13be330c5e6..8bf6079b6155d 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -8756,7 +8756,7 @@ bool InvalidWeakAttributeUse::diagnoseAsError() { return false; auto *var = pattern->getDecl(); - auto varType = OptionalType::get(getType(var)); + auto varType = getType(var); auto diagnostic = emitDiagnosticAt(var, diag::invalid_ownership_not_optional, diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 4f7c7499a2e96..5528acecc767b 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2438,17 +2438,12 @@ namespace { /// \param locator The locator to use for generated constraints and /// type variables. /// - /// \param externalPatternType The type imposed by the enclosing pattern, - /// if any. This will be non-null in cases where there is, e.g., a - /// pattern such as "is SubClass". - /// /// \param bindPatternVarsOneWay When true, generate fresh type variables /// for the types of each variable declared within the pattern, along /// with a one-way constraint binding that to the type to which the /// variable will be ascribed or inferred. Type getTypeForPattern( Pattern *pattern, ConstraintLocatorBuilder locator, - Type externalPatternType, bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding = nullptr, unsigned patternBindingIndex = 0) { @@ -2476,19 +2471,11 @@ namespace { case PatternKind::Paren: { auto *paren = cast(pattern); - // Parentheses don't affect the canonical type, but record them as - // type sugar. - if (externalPatternType && - isa(externalPatternType.getPointer())) { - externalPatternType = cast(externalPatternType.getPointer()) - ->getUnderlyingType(); - } - auto *subPattern = paren->getSubPattern(); auto underlyingType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - externalPatternType, bindPatternVarsOneWay); + bindPatternVarsOneWay); if (!underlyingType) return Type(); @@ -2497,7 +2484,7 @@ namespace { } case PatternKind::Binding: { auto *subPattern = cast(pattern)->getSubPattern(); - auto type = getTypeForPattern(subPattern, locator, externalPatternType, + auto type = getTypeForPattern(subPattern, locator, bindPatternVarsOneWay); if (!type) @@ -2526,9 +2513,7 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(pattern)); // Always prefer a contextual type when it's available. - if (externalPatternType) { - type = externalPatternType; - } else if (auto *initializer = getInitializerExpr()) { + if (auto *initializer = getInitializerExpr()) { // For initialization always assume a type of initializer. type = CS.getType(initializer)->getRValueType(); } else { @@ -2608,18 +2593,13 @@ namespace { // diagnostic in the middle of the solver path. CS.addConstraint(ConstraintKind::OneWayEqual, oneWayVarType, - externalPatternType ? externalPatternType : varType, - locator); + varType, locator); } // Ascribe a type to the declaration so it's always available to // constraint system. if (oneWayVarType) { CS.setType(var, oneWayVarType); - } else if (externalPatternType) { - // If there is an externally imposed type, that's what the - // declaration is going to be bound to. - CS.setType(var, externalPatternType); } else { // Otherwise, let's use the type of the pattern. The type // of the declaration has to be r-value, so let's add an @@ -2708,7 +2688,7 @@ namespace { Type subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - openedType, bindPatternVarsOneWay); + bindPatternVarsOneWay); if (!subPatternType) return Type(); @@ -2731,38 +2711,16 @@ namespace { case PatternKind::Tuple: { auto tuplePat = cast(pattern); - // If there's an externally-imposed type, decompose it into element - // types so long as we have the right number of such types. - SmallVector externalEltTypes; - if (externalPatternType) { - decomposeTuple(externalPatternType, externalEltTypes); - - // If we have the wrong number of elements, we may not be able to - // provide more specific types. - if (tuplePat->getNumElements() != externalEltTypes.size()) { - externalEltTypes.clear(); - - // Implicit tupling. - if (tuplePat->getNumElements() == 1) { - externalEltTypes.push_back( - AnyFunctionType::Param(externalPatternType)); - } - } - } - SmallVector tupleTypeElts; tupleTypeElts.reserve(tuplePat->getNumElements()); for (unsigned i = 0, e = tuplePat->getNumElements(); i != e; ++i) { auto &tupleElt = tuplePat->getElement(i); - Type externalEltType; - if (!externalEltTypes.empty()) - externalEltType = externalEltTypes[i].getPlainType(); auto *eltPattern = tupleElt.getPattern(); Type eltTy = getTypeForPattern( eltPattern, locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)), - externalEltType, bindPatternVarsOneWay); + bindPatternVarsOneWay); if (!eltTy) return Type(); @@ -2774,25 +2732,12 @@ namespace { } case PatternKind::OptionalSome: { - // Remove an optional from the object type. - if (externalPatternType) { - Type objVar = CS.createTypeVariable( - CS.getConstraintLocator( - locator.withPathElement(ConstraintLocator::OptionalPayload)), - TVO_CanBindToNoEscape); - CS.addConstraint( - ConstraintKind::OptionalObject, externalPatternType, objVar, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); - - externalPatternType = objVar; - } - auto *subPattern = cast(pattern)->getSubPattern(); // The subpattern must have optional type. Type subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - externalPatternType, bindPatternVarsOneWay); + bindPatternVarsOneWay); if (!subPatternType) return Type(); @@ -2827,7 +2772,7 @@ namespace { auto subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - castType, bindPatternVarsOneWay); + bindPatternVarsOneWay); // NOTE: The order here is important! Pattern matching equality is // not symmetric (we need to fix that either by using a different @@ -2912,18 +2857,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); baseType = parentType; - // Perform member lookup into the external pattern metatype. e.g. - // `case let .test(tuple) as Test`. - } else if (externalPatternType) { - Type externalMetaType = MetatypeType::get(externalPatternType); - - CS.addValueMemberConstraint( - externalMetaType, enumPattern->getName(), memberType, CurDC, - functionRefKind, {}, - CS.getConstraintLocator(locator, - LocatorPathElt::PatternMatch(pattern))); - - baseType = externalPatternType; } else { // Use the pattern type for member lookup. CS.addUnresolvedValueMemberConstraint( @@ -2941,7 +2874,7 @@ namespace { Type subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), - Type(), bindPatternVarsOneWay); + bindPatternVarsOneWay); if (!subPatternType) return Type(); @@ -4846,7 +4779,7 @@ Type ConstraintSystem::generateConstraints( bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding, unsigned patternIndex) { ConstraintGenerator cg(*this, nullptr); - return cg.getTypeForPattern(pattern, locator, Type(), bindPatternVarsOneWay, + return cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay, patternBinding, patternIndex); } diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index 0104bbe045c16..7843575aa18f6 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -564,3 +564,7 @@ struct TestIUOMatchOp { if case self = self {} } } + +struct TestRecursiveVarRef { + lazy var e: () -> Int = {e}() +} From 2ab005a233a7b75f2f29412da1fa16f9656a2bec Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 04/11] [CS] Remove null pattern handling from `getTypeForPattern` Push the only null case that can occur up into the caller. --- lib/Sema/CSGen.cpp | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 5528acecc767b..356e240e8a79d 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2447,12 +2447,7 @@ namespace { bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding = nullptr, unsigned patternBindingIndex = 0) { - // If there's no pattern, then we have an unknown subpattern. Create a - // type variable. - if (!pattern) { - return CS.createTypeVariable(CS.getConstraintLocator(locator), - TVO_CanBindToNoEscape); - } + assert(pattern); // Local function that must be called for each "return" throughout this // function, to set the type of the pattern. @@ -4355,16 +4350,18 @@ bool ConstraintSystem::generateWrappedPropertyTypeConstraints( static bool generateInitPatternConstraints(ConstraintSystem &cs, SyntacticElementTarget target, Expr *initializer) { - auto pattern = target.getInitializationPattern(); auto locator = cs.getConstraintLocator( initializer, LocatorPathElt::ContextualType(CTP_Initialization)); - Type patternType = cs.generateConstraints( - pattern, locator, target.shouldBindPatternVarsOneWay(), - target.getInitializationPatternBindingDecl(), - target.getInitializationPatternBindingIndex()); - if (!patternType) - return true; + Type patternType; + if (auto pattern = target.getInitializationPattern()) { + patternType = cs.generateConstraints( + pattern, locator, target.shouldBindPatternVarsOneWay(), + target.getInitializationPatternBindingDecl(), + target.getInitializationPatternBindingIndex()); + } else { + patternType = cs.createTypeVariable(locator, TVO_CanBindToNoEscape); + } if (auto wrappedVar = target.getInitializationWrappedVar()) return cs.generateWrappedPropertyTypeConstraints( From 6f76cee5a1250f08758080ab0619b05901128435 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 05/11] [CS] Remove null Type handling from `getTypeForPattern` We should never CSGen a null Type for patterns. --- lib/Sema/CSGen.cpp | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 356e240e8a79d..2838219ef1a06 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2472,19 +2472,12 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - if (!underlyingType) - return Type(); - return setType(ParenType::get(CS.getASTContext(), underlyingType)); } case PatternKind::Binding: { auto *subPattern = cast(pattern)->getSubPattern(); auto type = getTypeForPattern(subPattern, locator, bindPatternVarsOneWay); - - if (!type) - return Type(); - // Var doesn't affect the type. return setType(type); } @@ -2666,9 +2659,6 @@ namespace { Type type = TypeChecker::typeCheckPattern(contextualPattern); - if (!type) - return Type(); - // Look through reference storage types. type = type->getReferenceStorageReferent(); @@ -2685,9 +2675,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - if (!subPatternType) - return Type(); - // NOTE: The order here is important! Pattern matching equality is // not symmetric (we need to fix that either by using a different // constraint, or actually making it symmetric). @@ -2717,9 +2704,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)), bindPatternVarsOneWay); - if (!eltTy) - return Type(); - tupleTypeElts.push_back(TupleTypeElt(eltTy, tupleElt.getLabel())); } @@ -2734,9 +2718,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - if (!subPatternType) - return Type(); - return setType(OptionalType::get(subPatternType)); } @@ -2746,7 +2727,6 @@ namespace { const Type castType = resolveTypeReferenceInExpression( isPattern->getCastTypeRepr(), TypeResolverContext::InExpression, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); - if (!castType) return Type(); // Allow `is` pattern to infer type from context which is then going // to be propaged down to its sub-pattern via conversion. This enables @@ -2834,9 +2814,6 @@ namespace { TypeResolverContext::InExpression, patternMatchLoc); }(); - if (!parentType) - return Type(); - // Perform member lookup into the parent's metatype. Type parentMetaType = MetatypeType::get(parentType); CS.addValueMemberConstraint( @@ -2871,9 +2848,6 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - if (!subPatternType) - return Type(); - SmallVector params; decomposeTuple(subPatternType, params); From ae669cee06c9c0c94b813c91c75c5bf8e69d1da4 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 06/11] [Sema] Walk SyntacticElementTarget for completion Instead of walking the single ASTNode from the target, walk all AST nodes associated with the target to find the completion expr. This is needed to find the completion expr in a pattern for an initialization target. --- include/swift/Sema/CompletionContextFinder.h | 13 +++++++------ lib/Sema/BuilderTransform.cpp | 3 ++- lib/Sema/CompletionContextFinder.cpp | 9 +++++++++ lib/Sema/TypeCheckCodeCompletion.cpp | 11 ++++------- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/include/swift/Sema/CompletionContextFinder.h b/include/swift/Sema/CompletionContextFinder.h index 51dc3bfd515e2..dce6572908b7a 100644 --- a/include/swift/Sema/CompletionContextFinder.h +++ b/include/swift/Sema/CompletionContextFinder.h @@ -20,6 +20,10 @@ namespace swift { +namespace constraints { +class SyntacticElementTarget; +} + class CompletionContextFinder : public ASTWalker { enum class ContextKind { FallbackExpression, @@ -53,12 +57,9 @@ class CompletionContextFinder : public ASTWalker { return MacroWalking::Arguments; } - /// Finder for completion contexts within the provided initial expression. - CompletionContextFinder(ASTNode initialNode, DeclContext *DC) - : InitialExpr(initialNode.dyn_cast()), InitialDC(DC) { - assert(DC); - initialNode.walk(*this); - }; + /// Finder for completion contexts within the provided SyntacticElementTarget. + CompletionContextFinder(constraints::SyntacticElementTarget target, + DeclContext *DC); /// Finder for completion contexts within the outermost non-closure context of /// the code completion expression's direct context. diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp index 43fa783a0d786..b6ae98679227c 100644 --- a/lib/Sema/BuilderTransform.cpp +++ b/lib/Sema/BuilderTransform.cpp @@ -984,7 +984,8 @@ Optional TypeChecker::applyResultBuilderBodyTransform( SmallVector solutions; cs.solveForCodeCompletion(solutions); - CompletionContextFinder analyzer(func, func->getDeclContext()); + SyntacticElementTarget funcTarget(func); + CompletionContextFinder analyzer(funcTarget, func->getDeclContext()); if (analyzer.hasCompletion()) { filterSolutionsForCodeCompletion(solutions, analyzer); for (const auto &solution : solutions) { diff --git a/lib/Sema/CompletionContextFinder.cpp b/lib/Sema/CompletionContextFinder.cpp index f4ef0f7a9336f..9279a038d160a 100644 --- a/lib/Sema/CompletionContextFinder.cpp +++ b/lib/Sema/CompletionContextFinder.cpp @@ -12,10 +12,19 @@ #include "swift/Sema/CompletionContextFinder.h" #include "swift/Parse/Lexer.h" +#include "swift/Sema/SyntacticElementTarget.h" using namespace swift; +using namespace constraints; using Fallback = CompletionContextFinder::Fallback; +CompletionContextFinder::CompletionContextFinder( + SyntacticElementTarget target, DeclContext *DC) + : InitialExpr(target.getAsExpr()), InitialDC(DC) { + assert(DC); + target.walk(*this); +} + ASTWalker::PreWalkResult CompletionContextFinder::walkToExprPre(Expr *E) { if (auto *closure = dyn_cast(E)) { diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp index e93d22a6f06ca..58acb0c1e1965 100644 --- a/lib/Sema/TypeCheckCodeCompletion.cpp +++ b/lib/Sema/TypeCheckCodeCompletion.cpp @@ -574,15 +574,12 @@ bool TypeChecker::typeCheckForCodeCompletion( return false; } - auto node = target.getAsASTNode(); - if (!node) - return false; - - if (auto *expr = getAsExpr(node)) { - node = expr->walk(SanitizeExpr(Context)); + if (getAsExpr(target.getAsASTNode())) { + SanitizeExpr sanitizer(Context); + target = *target.walk(sanitizer); } - CompletionContextFinder contextAnalyzer(node, DC); + CompletionContextFinder contextAnalyzer(target, DC); // If there was no completion expr (e.g. if the code completion location was // among tokens that were skipped over during parser error recovery) bail. From 49a85e290c74aa7e5a6d38f7a4b6543018ef8017 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 07/11] [CS] Allow ExprPatterns to be type-checked in the solver Previously we would wait until CSApply, which would trigger their type-checking in `coercePatternToType`. This caused a number of bugs, and hampered solver-based completion, which does not run CSApply. Instead, form a conjunction of all the ExprPatterns present, which preserves some of the previous isolation behavior (though does not provide complete isolation). We can then modify `coercePatternToType` to accept a closure, which allows the solver to take over rewriting the ExprPatterns it has already solved. This then sets the stage for the complete removal of `coercePatternToType`, and doing all pattern type-checking in the solver. --- include/swift/Sema/ConstraintSystem.h | 35 +++++ lib/IDE/TypeCheckCompletionCallback.cpp | 37 +++-- lib/Sema/CSApply.cpp | 128 ++++++++++++------ lib/Sema/CSGen.cpp | 37 ++--- lib/Sema/CSSolver.cpp | 8 ++ lib/Sema/CSSyntacticElement.cpp | 57 +++++++- lib/Sema/TypeCheckPattern.cpp | 119 +++++++++------- lib/Sema/TypeChecker.h | 15 +- test/Constraints/patterns.swift | 80 +++++++++++ test/Constraints/rdar105781521.swift | 1 - test/Constraints/rdar105782480.swift | 18 +++ test/Constraints/rdar106598067.swift | 11 ++ test/Constraints/rdar109419240.swift | 15 ++ test/Constraints/result_builder_diags.swift | 2 - .../result_builder_invalid_stmts.swift | 4 - test/IDE/complete_unresolved_members.swift | 35 +++++ test/SILGen/switch_expr.swift | 11 ++ test/stdlib/StringDiagnostics.swift | 1 - test/stmt/errors.swift | 3 +- .../Sema/SwiftUI/rdar70256351.swift | 1 - .../issue-65360.swift | 16 ++- 21 files changed, 492 insertions(+), 142 deletions(-) create mode 100644 test/Constraints/rdar105782480.swift create mode 100644 test/Constraints/rdar106598067.swift create mode 100644 test/Constraints/rdar109419240.swift diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index fe165d13f8ba2..22143032333fe 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1500,6 +1500,10 @@ class Solution { llvm::SmallMapVector caseLabelItems; + /// A map of expressions to the ExprPatterns that they are being solved as + /// a part of. + llvm::SmallMapVector exprPatterns; + /// The set of parameters that have been inferred to be 'isolated'. llvm::SmallVector isolatedParams; @@ -1685,6 +1689,16 @@ class Solution { : nullptr; } + /// Retrieve the solved ExprPattern that corresponds to provided + /// sub-expression. + NullablePtr getExprPatternFor(Expr *E) const { + auto result = exprPatterns.find(E); + if (result == exprPatterns.end()) + return nullptr; + + return result->second; + } + /// This method implements functionality of `Expr::isTypeReference` /// with data provided by a given solution. bool isTypeReference(Expr *E) const; @@ -2148,6 +2162,10 @@ class ConstraintSystem { llvm::SmallMapVector caseLabelItems; + /// A map of expressions to the ExprPatterns that they are being solved as + /// a part of. + llvm::SmallMapVector exprPatterns; + /// The set of parameters that have been inferred to be 'isolated'. llvm::SmallSetVector isolatedParams; @@ -2745,6 +2763,9 @@ class ConstraintSystem { /// The length of \c caseLabelItems. unsigned numCaseLabelItems; + /// The length of \c exprPatterns. + unsigned numExprPatterns; + /// The length of \c isolatedParams. unsigned numIsolatedParams; @@ -3166,6 +3187,15 @@ class ConstraintSystem { caseLabelItems[item] = info; } + /// Record a given ExprPattern as the parent of its sub-expression. + void setExprPatternFor(Expr *E, ExprPattern *EP) { + assert(E); + assert(EP); + auto inserted = exprPatterns.insert({E, EP}).second; + assert(inserted && "Mapping already defined?"); + (void)inserted; + } + Optional getCaseLabelItemInfo( const CaseLabelItem *item) const { auto known = caseLabelItems.find(item); @@ -4315,6 +4345,11 @@ class ConstraintSystem { /// \returns \c true if constraint generation failed, \c false otherwise bool generateConstraints(SingleValueStmtExpr *E); + /// Generate constraints for an array of ExprPatterns, forming a conjunction + /// that solves each expression in turn. + void generateConstraints(ArrayRef exprPatterns, + ConstraintLocatorBuilder locator); + /// Generate constraints for the given (unchecked) expression. /// /// \returns a possibly-sanitized expression, or null if an error occurred. diff --git a/lib/IDE/TypeCheckCompletionCallback.cpp b/lib/IDE/TypeCheckCompletionCallback.cpp index d2f652bd84a68..9739a53fbd679 100644 --- a/lib/IDE/TypeCheckCompletionCallback.cpp +++ b/lib/IDE/TypeCheckCompletionCallback.cpp @@ -81,7 +81,13 @@ Type swift::ide::getTypeForCompletion(const constraints::Solution &S, /// \endcode /// If the code completion expression occurs in such an AST, return the /// declaration of the \c $match variable, otherwise return \c nullptr. -static VarDecl *getMatchVarIfInPatternMatch(Expr *E, ConstraintSystem &CS) { +static VarDecl *getMatchVarIfInPatternMatch(Expr *E, const Solution &S) { + if (auto EP = S.getExprPatternFor(E)) + return EP.get()->getMatchVar(); + + // TODO: Once ExprPattern type-checking is fully moved into the solver, + // the below can be deleted. + auto &CS = S.getConstraintSystem(); auto &Context = CS.getASTContext(); auto *Binary = dyn_cast_or_null(CS.getParentExpr(E)); @@ -109,20 +115,21 @@ static VarDecl *getMatchVarIfInPatternMatch(Expr *E, ConstraintSystem &CS) { } Type swift::ide::getPatternMatchType(const constraints::Solution &S, Expr *E) { - if (auto MatchVar = getMatchVarIfInPatternMatch(E, S.getConstraintSystem())) { - Type MatchVarType; - // If the MatchVar has an explicit type, it's not part of the solution. But - // we can look it up in the constraint system directly. - if (auto T = S.getConstraintSystem().getVarType(MatchVar)) { - MatchVarType = T; - } else { - MatchVarType = getTypeForCompletion(S, MatchVar); - } - if (MatchVarType) { - return MatchVarType; - } - } - return nullptr; + auto MatchVar = getMatchVarIfInPatternMatch(E, S); + if (!MatchVar) + return nullptr; + + if (S.hasType(MatchVar)) + return S.getResolvedType(MatchVar); + + // If the ExprPattern wasn't solved as part of the constraint system, it's + // not part of the solution. + // TODO: This can be removed once ExprPattern type-checking is fully part + // of the constraint system. + if (auto T = S.getConstraintSystem().getVarType(MatchVar)) + return T; + + return getTypeForCompletion(S, MatchVar); } void swift::ide::getSolutionSpecificVarTypes( diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index c255fbd47887e..6e5dc16fa6782 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8688,6 +8688,9 @@ namespace { return Action::SkipChildren(); } + NullablePtr + rewritePattern(Pattern *pattern, DeclContext *DC); + /// Rewrite the target, producing a new target. Optional rewriteTarget(SyntacticElementTarget target); @@ -8934,12 +8937,68 @@ static Expr *wrapAsyncLetInitializer( return resultInit; } +static Pattern *rewriteExprPattern(const SyntacticElementTarget &matchTarget, + Type patternTy, + RewriteTargetFn rewriteTarget) { + auto *EP = matchTarget.getExprPattern(); + + // See if we can simplify to another kind of pattern. + if (auto simplified = TypeChecker::trySimplifyExprPattern(EP, patternTy)) + return simplified.get(); + + auto resultTarget = rewriteTarget(matchTarget); + if (!resultTarget) + return nullptr; + + EP->setMatchExpr(resultTarget->getAsExpr()); + EP->getMatchVar()->setInterfaceType(patternTy->mapTypeOutOfContext()); + EP->setType(patternTy); + return EP; +} + +/// Attempt to rewrite either an ExprPattern, or a pattern that was solved as +/// an ExprPattern, e.g an EnumElementPattern that could not refer to an enum +/// case. +static Optional +tryRewriteExprPattern(Pattern *P, Solution &solution, Type patternTy, + RewriteTargetFn rewriteTarget) { + // See if we have a match expression target. + auto matchTarget = solution.getTargetFor(P); + if (!matchTarget) + return None; + + return rewriteExprPattern(*matchTarget, patternTy, rewriteTarget); +} + +NullablePtr ExprWalker::rewritePattern(Pattern *pattern, + DeclContext *DC) { + auto &solution = Rewriter.solution; + + // Figure out the pattern type. + auto patternTy = solution.getResolvedType(pattern); + patternTy = patternTy->reconstituteSugar(/*recursive=*/false); + + // Coerce the pattern to its appropriate type. + TypeResolutionOptions patternOptions(TypeResolverContext::InExpression); + patternOptions |= TypeResolutionFlags::OverrideType; + + auto tryRewritePattern = [&](Pattern *EP, Type ty) { + return ::tryRewriteExprPattern( + EP, solution, ty, [&](auto target) { return rewriteTarget(target); }); + }; + + auto contextualPattern = ContextualPattern::forRawPattern(pattern, DC); + return TypeChecker::coercePatternToType(contextualPattern, patternTy, + patternOptions, tryRewritePattern); +} + /// Apply the given solution to the initialization target. /// /// \returns the resulting initialization expression. static Optional applySolutionToInitialization(Solution &solution, SyntacticElementTarget target, - Expr *initializer) { + Expr *initializer, + RewriteTargetFn rewriteTarget) { auto wrappedVar = target.getInitializationWrappedVar(); Type initType; if (wrappedVar) { @@ -9004,10 +9063,14 @@ applySolutionToInitialization(Solution &solution, SyntacticElementTarget target, finalPatternType = finalPatternType->reconstituteSugar(/*recursive =*/false); + auto tryRewritePattern = [&](Pattern *EP, Type ty) { + return ::tryRewriteExprPattern(EP, solution, ty, rewriteTarget); + }; + // Apply the solution to the pattern as well. auto contextualPattern = target.getContextualPattern(); if (auto coercedPattern = TypeChecker::coercePatternToType( - contextualPattern, finalPatternType, options)) { + contextualPattern, finalPatternType, options, tryRewritePattern)) { resultTarget.setPattern(coercedPattern); } else { return None; @@ -9154,10 +9217,15 @@ static Optional applySolutionToForEachStmt( TypeResolutionOptions options(TypeResolverContext::ForEachStmt); options |= TypeResolutionFlags::OverrideType; + auto tryRewritePattern = [&](Pattern *EP, Type ty) { + return ::tryRewriteExprPattern(EP, solution, ty, rewriteTarget); + }; + // Apply the solution to the pattern as well. auto contextualPattern = target.getContextualPattern(); auto coercedPattern = TypeChecker::coercePatternToType( - contextualPattern, forEachStmtInfo.initType, options); + contextualPattern, forEachStmtInfo.initType, options, + tryRewritePattern); if (!coercedPattern) return None; @@ -9245,7 +9313,8 @@ ExprWalker::rewriteTarget(SyntacticElementTarget target) { switch (target.getExprContextualTypePurpose()) { case CTP_Initialization: { auto initResultTarget = applySolutionToInitialization( - solution, target, rewrittenExpr); + solution, target, rewrittenExpr, + [&](auto target) { return rewriteTarget(target); }); if (!initResultTarget) return None; @@ -9336,47 +9405,11 @@ ExprWalker::rewriteTarget(SyntacticElementTarget target) { ConstraintSystem &cs = solution.getConstraintSystem(); auto info = *cs.getCaseLabelItemInfo(*caseLabelItem); - // Figure out the pattern type. - Type patternType = solution.simplifyType(solution.getType(info.pattern)); - patternType = patternType->reconstituteSugar(/*recursive=*/false); - - // Check whether this enum element is resolved via ~= application. - if (auto *enumElement = dyn_cast(info.pattern)) { - if (auto target = cs.getTargetFor(enumElement)) { - auto *EP = target->getExprPattern(); - auto enumType = solution.getResolvedType(EP); - - auto *matchCall = target->getAsExpr(); - - auto *result = matchCall->walk(*this); - if (!result) - return None; - - { - auto *matchVar = EP->getMatchVar(); - matchVar->setInterfaceType(enumType->mapTypeOutOfContext()); - } - - EP->setMatchExpr(result); - EP->setType(enumType); - - (*caseLabelItem)->setPattern(EP, /*resolved=*/true); - return target; - } - } - - // Coerce the pattern to its appropriate type. - TypeResolutionOptions patternOptions(TypeResolverContext::InExpression); - patternOptions |= TypeResolutionFlags::OverrideType; - auto contextualPattern = - ContextualPattern::forRawPattern(info.pattern, - target.getDeclContext()); - if (auto coercedPattern = TypeChecker::coercePatternToType( - contextualPattern, patternType, patternOptions)) { - (*caseLabelItem)->setPattern(coercedPattern, /*resolved=*/true); - } else { + auto pattern = rewritePattern(info.pattern, target.getDeclContext()); + if (!pattern) return None; - } + + (*caseLabelItem)->setPattern(pattern.get(), /*resolved=*/true); // If there is a guard expression, coerce that. if (auto *guardExpr = info.guardExpr) { @@ -9444,8 +9477,13 @@ ExprWalker::rewriteTarget(SyntacticElementTarget target) { options |= TypeResolutionFlags::OverrideType; } + auto tryRewritePattern = [&](Pattern *EP, Type ty) { + return ::tryRewriteExprPattern( + EP, solution, ty, [&](auto target) { return rewriteTarget(target); }); + }; + if (auto coercedPattern = TypeChecker::coercePatternToType( - contextualPattern, patternType, options)) { + contextualPattern, patternType, options, tryRewritePattern)) { auto resultTarget = target; resultTarget.setPattern(coercedPattern); return resultTarget; diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 2838219ef1a06..80fe6f42c2b8a 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2453,12 +2453,6 @@ namespace { // function, to set the type of the pattern. auto setType = [&](Type type) { CS.setType(pattern, type); - if (auto PE = dyn_cast(pattern)) { - // Set the type of the pattern's sub-expression as well, so code - // completion can retrieve the expression's type in case it is a code - // completion token. - CS.setType(PE->getSubExpr(), type); - } return type; }; @@ -2883,15 +2877,12 @@ namespace { return setType(patternType); } - // Refutable patterns occur when checking the PatternBindingDecls in an - // if/let or while/let condition. They always require an initial value, - // so they always allow unspecified types. - case PatternKind::Expr: - // TODO: we could try harder here, e.g. for enum elements to provide the - // enum type. - return setType( - CS.createTypeVariable(CS.getConstraintLocator(locator), - TVO_CanBindToNoEscape | TVO_CanBindToHole)); + case PatternKind::Expr: { + // We generate constraints for ExprPatterns in a separate pass. For + // now, just create a type variable. + return setType(CS.createTypeVariable(CS.getConstraintLocator(locator), + TVO_CanBindToNoEscape)); + } } llvm_unreachable("Unhandled pattern kind"); @@ -4750,8 +4741,20 @@ Type ConstraintSystem::generateConstraints( bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding, unsigned patternIndex) { ConstraintGenerator cg(*this, nullptr); - return cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay, - patternBinding, patternIndex); + auto ty = cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay, + patternBinding, patternIndex); + assert(ty); + + // Gather the ExprPatterns, and form a conjunction for their expressions. + SmallVector exprPatterns; + pattern->forEachNode([&](Pattern *P) { + if (auto *EP = dyn_cast(P)) + exprPatterns.push_back(EP); + }); + if (!exprPatterns.empty()) + generateConstraints(exprPatterns, getConstraintLocator(pattern)); + + return ty; } bool ConstraintSystem::generateConstraints(StmtCondition condition, diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index e80def2f385f1..97dcdef70744b 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -195,6 +195,7 @@ Solution ConstraintSystem::finalize() { solution.targets = targets; solution.caseLabelItems = caseLabelItems; + solution.exprPatterns = exprPatterns; solution.isolatedParams.append(isolatedParams.begin(), isolatedParams.end()); solution.preconcurrencyClosures.append(preconcurrencyClosures.begin(), preconcurrencyClosures.end()); @@ -327,6 +328,9 @@ void ConstraintSystem::applySolution(const Solution &solution) { isolatedParams.insert(param); } + for (auto &pair : solution.exprPatterns) + exprPatterns.insert(pair); + for (auto closure : solution.preconcurrencyClosures) { preconcurrencyClosures.insert(closure); } @@ -621,6 +625,7 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numContextualTypes = cs.contextualTypes.size(); numTargets = cs.targets.size(); numCaseLabelItems = cs.caseLabelItems.size(); + numExprPatterns = cs.exprPatterns.size(); numIsolatedParams = cs.isolatedParams.size(); numPreconcurrencyClosures = cs.preconcurrencyClosures.size(); numImplicitValueConversions = cs.ImplicitValueConversions.size(); @@ -737,6 +742,9 @@ ConstraintSystem::SolverScope::~SolverScope() { // Remove any case label item infos. truncate(cs.caseLabelItems, numCaseLabelItems); + // Remove any ExprPattern mappings. + truncate(cs.exprPatterns, numExprPatterns); + // Remove any isolated parameters. truncate(cs.isolatedParams, numIsolatedParams); diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 43e0ac33a50c9..36529ffad526a 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -410,7 +410,7 @@ ElementInfo makeJoinElement(ConstraintSystem &cs, TypeJoinExpr *join, struct SyntacticElementContext : public llvm::PointerUnion { + SingleValueStmtExpr *, ExprPattern *> { // Inherit the constructors from PointerUnion. using PointerUnion::PointerUnion; @@ -441,6 +441,10 @@ struct SyntacticElementContext return context; } + static SyntacticElementContext forExprPattern(ExprPattern *EP) { + return SyntacticElementContext{EP}; + } + DeclContext *getAsDeclContext() const { if (auto *fn = this->dyn_cast()) { return fn; @@ -448,6 +452,8 @@ struct SyntacticElementContext return closure; } else if (auto *SVE = dyn_cast()) { return SVE->getDeclContext(); + } else if (auto *EP = dyn_cast()) { + return EP->getDeclContext(); } else { llvm_unreachable("unsupported kind"); } @@ -519,7 +525,32 @@ class SyntacticElementConstraintGenerator ConstraintLocator *locator) : cs(cs), context(context), locator(locator) {} - void visitPattern(Pattern *pattern, ContextualTypeInfo context) { + void visitExprPattern(ExprPattern *EP) { + auto target = SyntacticElementTarget::forExprPattern(EP); + + if (cs.preCheckTarget(target, /*replaceInvalidRefWithErrors=*/true, + /*leaveClosureBodyUnchecked=*/false)) { + hadError = true; + return; + } + cs.setType(EP->getMatchVar(), cs.getType(EP)); + + if (cs.generateConstraints(target)) { + hadError = true; + return; + } + cs.setTargetFor(EP, target); + cs.setExprPatternFor(EP->getSubExpr(), EP); + } + + void visitPattern(Pattern *pattern, ContextualTypeInfo contextInfo) { + if (context.is()) { + // This is for an ExprPattern conjunction, go ahead and generate + // constraints for the match expression. + visitExprPattern(cast(pattern)); + return; + } + auto parentElement = locator->getLastElementAs(); @@ -535,7 +566,7 @@ class SyntacticElementConstraintGenerator } if (isa(stmt)) { - visitCaseItemPattern(pattern, context); + visitCaseItemPattern(pattern, contextInfo); return; } } @@ -1438,6 +1469,24 @@ bool ConstraintSystem::generateConstraints(SingleValueStmtExpr *E) { return generator.hadError; } +void ConstraintSystem::generateConstraints(ArrayRef exprPatterns, + ConstraintLocatorBuilder locator) { + // Form a conjunction of ExprPattern elements, isolated from the rest of the + // pattern. + SmallVector elements; + SmallVector referencedTypeVars; + for (auto *EP : exprPatterns) { + auto ty = getType(EP)->castTo(); + referencedTypeVars.push_back(ty); + + ContextualTypeInfo context(ty, CTP_ExprPattern); + elements.push_back(makeElement(EP, getConstraintLocator(EP), context)); + } + auto *loc = getConstraintLocator(locator); + createConjunction(*this, elements, loc, /*isIsolated*/ true, + referencedTypeVars); +} + bool ConstraintSystem::isInResultBuilderContext(ClosureExpr *closure) const { if (!closure->hasSingleExpressionBody()) { auto *DC = closure->getParent(); @@ -1488,6 +1537,8 @@ ConstraintSystem::simplifySyntacticElementConstraint( context = SyntacticElementContext::forFunction(fn); } else if (auto *SVE = getAsExpr(anchor)) { context = SyntacticElementContext::forSingleValueStmtExpr(SVE); + } else if (auto *EP = getAsPattern(anchor)) { + context = SyntacticElementContext::forExprPattern(EP); } else { return SolutionKind::Error; } diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 946e86da0d858..9b42c5c8896a5 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -1034,15 +1034,61 @@ void repairTupleOrAssociatedValuePatternIfApplicable( enumCase->getName()); } +NullablePtr TypeChecker::trySimplifyExprPattern(ExprPattern *EP, + Type patternTy) { + auto *subExpr = EP->getSubExpr(); + auto &ctx = EP->getDeclContext()->getASTContext(); + + if (patternTy->isBool()) { + // The type is Bool. + // Check if the pattern is a Bool literal + auto *semanticSubExpr = subExpr->getSemanticsProvidingExpr(); + if (auto *BLE = dyn_cast(semanticSubExpr)) { + auto *BP = new (ctx) BoolPattern(BLE->getLoc(), BLE->getValue()); + BP->setType(patternTy); + return BP; + } + } + + // case nil is equivalent to .none when switching on Optionals. + if (auto *NLE = dyn_cast(EP->getSubExpr())) { + if (patternTy->getOptionalObjectType()) { + auto *NoneEnumElement = ctx.getOptionalNoneDecl(); + auto *BaseTE = TypeExpr::createImplicit(patternTy, ctx); + auto *EEP = new (ctx) + EnumElementPattern(BaseTE, NLE->getLoc(), DeclNameLoc(NLE->getLoc()), + NoneEnumElement->createNameRef(), NoneEnumElement, + nullptr, EP->getDeclContext()); + EEP->setType(patternTy); + return EEP; + } else { + // ...but for non-optional types it can never match! Diagnose it. + ctx.Diags + .diagnose(NLE->getLoc(), diag::value_type_comparison_with_nil_illegal, + patternTy) + .warnUntilSwiftVersion(6); + + if (ctx.isSwiftVersionAtLeast(6)) + return nullptr; + } + } + return nullptr; +} + /// Perform top-down type coercion on the given pattern. -Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, - Type type, - TypeResolutionOptions options) { +Pattern *TypeChecker::coercePatternToType( + ContextualPattern pattern, Type type, TypeResolutionOptions options, + llvm::function_ref(Pattern *, Type)> + tryRewritePattern) { auto P = pattern.getPattern(); auto dc = pattern.getDeclContext(); auto &Context = dc->getASTContext(); auto &diags = Context.Diags; + // See if we can rewrite this using the constraint system. + if (auto result = tryRewritePattern(P, type)) + return *result; + options = applyContextualPatternOptions(options, pattern); auto subOptions = options; subOptions.setContext(None); @@ -1061,8 +1107,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, if (tupleType->getNumElements() == 1) { auto element = tupleType->getElement(0); sub = coercePatternToType( - pattern.forSubPattern(sub, /*retainTopLevel=*/true), element.getType(), - subOptions); + pattern.forSubPattern(sub, /*retainTopLevel=*/true), + element.getType(), subOptions, tryRewritePattern); if (!sub) return nullptr; TuplePatternElt elt(element.getName(), SourceLoc(), sub); @@ -1077,7 +1123,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, } sub = coercePatternToType( - pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, subOptions); + pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, subOptions, + tryRewritePattern); if (!sub) return nullptr; @@ -1090,7 +1137,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, Pattern *sub = VP->getSubPattern(); sub = coercePatternToType( - pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, subOptions); + pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, subOptions, + tryRewritePattern); if (!sub) return nullptr; VP->setSubPattern(sub); @@ -1123,7 +1171,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, Pattern *sub = TP->getSubPattern(); sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, - subOptions | TypeResolutionFlags::FromNonInferredPattern); + subOptions | TypeResolutionFlags::FromNonInferredPattern, + tryRewritePattern); if (!sub) return nullptr; @@ -1212,9 +1261,9 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, auto decayToParen = [&]() -> Pattern * { assert(canDecayToParen); Pattern *sub = TP->getElement(0).getPattern(); - sub = TypeChecker::coercePatternToType( + sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), type, - subOptions); + subOptions, tryRewritePattern); if (!sub) return nullptr; @@ -1271,7 +1320,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, auto sub = coercePatternToType( pattern.forSubPattern(elt.getPattern(), /*retainTopLevel=*/false), - CoercionType, subOptions); + CoercionType, subOptions, tryRewritePattern); if (!sub) return nullptr; @@ -1291,37 +1340,9 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, assert(cast(P)->isResolved() && "coercing unresolved expr pattern!"); auto *EP = cast(P); - if (type->isBool()) { - // The type is Bool. - // Check if the pattern is a Bool literal - if (auto *BLE = dyn_cast( - EP->getSubExpr()->getSemanticsProvidingExpr())) { - P = new (Context) BoolPattern(BLE->getLoc(), BLE->getValue()); - P->setType(type); - return P; - } - } - // case nil is equivalent to .none when switching on Optionals. - if (auto *NLE = dyn_cast(EP->getSubExpr())) { - if (type->getOptionalObjectType()) { - auto *NoneEnumElement = Context.getOptionalNoneDecl(); - auto *BaseTE = TypeExpr::createImplicit(type, Context); - P = new (Context) EnumElementPattern( - BaseTE, NLE->getLoc(), DeclNameLoc(NLE->getLoc()), - NoneEnumElement->createNameRef(), NoneEnumElement, nullptr, dc); - return TypeChecker::coercePatternToType( - pattern.forSubPattern(P, /*retainTopLevel=*/true), type, options); - } else { - // ...but for non-optional types it can never match! Diagnose it. - diags.diagnose(NLE->getLoc(), - diag::value_type_comparison_with_nil_illegal, type) - .warnUntilSwiftVersion(6); - - if (type->getASTContext().isSwiftVersionAtLeast(6)) - return nullptr; - } - } + if (auto P = trySimplifyExprPattern(EP, type)) + return P.get(); if (TypeChecker::typeCheckExprPattern(EP, dc, type)) return nullptr; @@ -1370,7 +1391,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, P = sub; return coercePatternToType( - pattern.forSubPattern(P, /*retainTopLevel=*/true), type, options); + pattern.forSubPattern(P, /*retainTopLevel=*/true), type, options, + tryRewritePattern); } CheckedCastKind castKind = TypeChecker::typeCheckCheckedCast( @@ -1419,7 +1441,8 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), IP->getCastType(), - subOptions | TypeResolutionFlags::FromNonInferredPattern); + subOptions | TypeResolutionFlags::FromNonInferredPattern, + tryRewritePattern); if (!sub) return nullptr; @@ -1457,7 +1480,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, EEP->getEndLoc()); return coercePatternToType( pattern.forSubPattern(P, /*retainTopLevel=*/true), type, - options); + options, tryRewritePattern); } else { diags.diagnose(EEP->getLoc(), diag::enum_element_pattern_member_not_found, @@ -1472,7 +1495,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, Context, EEP->getUnresolvedOriginalExpr(), dc); return coercePatternToType( pattern.forSubPattern(P, /*retainTopLevel=*/true), type, - options); + options, tryRewritePattern); } } } @@ -1595,7 +1618,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), elementType, - newSubOptions); + newSubOptions, tryRewritePattern); if (!sub) return nullptr; @@ -1630,7 +1653,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, newSubOptions |= TypeResolutionFlags::FromNonInferredPattern; sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), elementType, - newSubOptions); + newSubOptions, tryRewritePattern); if (!sub) return nullptr; EEP->setSubPattern(sub); @@ -1674,7 +1697,7 @@ Pattern *TypeChecker::coercePatternToType(ContextualPattern pattern, newSubOptions |= TypeResolutionFlags::FromNonInferredPattern; sub = coercePatternToType( pattern.forSubPattern(sub, /*retainTopLevel=*/false), elementType, - newSubOptions); + newSubOptions, tryRewritePattern); if (!sub) return nullptr; diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index b5a9651e43f2d..d1ecf6bf92711 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -729,15 +729,26 @@ Pattern *resolvePattern(Pattern *P, DeclContext *dc, bool isStmtCondition); /// unbound generic types. Type typeCheckPattern(ContextualPattern pattern); +/// Attempt to simplify an ExprPattern into a BoolPattern or +/// OptionalSomePattern. Returns \c nullptr if the pattern could not be +/// simplified. +NullablePtr trySimplifyExprPattern(ExprPattern *EP, Type patternTy); + /// Coerce a pattern to the given type. /// /// \param pattern The contextual pattern. /// \param type the type to coerce the pattern to. /// \param options Options that control the coercion. +/// \param tryRewritePattern A function that attempts to externally rewrite +/// the given pattern. This is used by the constraint system to take over +/// rewriting for ExprPatterns. /// /// \returns the coerced pattern, or nullptr if the coercion failed. -Pattern *coercePatternToType(ContextualPattern pattern, Type type, - TypeResolutionOptions options); +Pattern *coercePatternToType( + ContextualPattern pattern, Type type, TypeResolutionOptions options, + llvm::function_ref(Pattern *, Type)> tryRewritePattern = + [](Pattern *, Type) { return None; }); + bool typeCheckExprPattern(ExprPattern *EP, DeclContext *DC, Type type); /// Coerce the specified parameter list of a ClosureExpr to the specified diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index 7843575aa18f6..c1ee5f5d0378b 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -568,3 +568,83 @@ struct TestIUOMatchOp { struct TestRecursiveVarRef { lazy var e: () -> Int = {e}() } + +func testMultiStmtClosureExprPattern(_ x: Int) { + if case { (); return x }() = x {} +} + +func testExprPatternIsolation() { + // We type-check ExprPatterns separately, so these are illegal. + if case 0 = nil {} // expected-error {{'nil' requires a contextual type}} + let _ = { + if case 0 = nil {} // expected-error {{'nil' requires a contextual type}} + } + for case 0 in nil {} // expected-error {{'nil' requires a contextual type}} + for case 0 in [nil] {} + // expected-error@-1 {{type 'Any' cannot conform to 'Equatable'}} + // expected-note@-2 {{only concrete types such as structs, enums and classes can conform to protocols}} + // expected-note@-3 {{requirement from conditional conformance of 'Any?' to 'Equatable'}} + + // Though we will try Double for an integer literal... + let d: Double = 0 + if case d = 0 {} + let _ = { + if case d = 0 {} + } + for case d in [0] {} + + // But not Float + let f: Float = 0 + if case f = 0 {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + let _ = { + if case f = 0 {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + } + for case f in [0] {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + + enum MultiPayload: Equatable { + case e(T, T) + static func f(_ x: T, _ y: T) -> Self { .e(x, y) } + } + enum E: Equatable { + case a, b + static var c: E { .a } + static var d: E { .b } + } + + func produceMultiPayload() -> MultiPayload { fatalError() } + + // We type-check ExprPatterns left to right, so only one of these works. + if case .e(0.0, 0) = produceMultiPayload() {} + if case .e(0, 0.0) = produceMultiPayload() {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + + for case .e(0.0, 0) in [produceMultiPayload()] {} + for case .e(0, 0.0) in [produceMultiPayload()] {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + + // Same, because although it's a top-level ExprPattern, we don't resolve + // that until during solving. + if case .f(0.0, 0) = produceMultiPayload() {} + if case .f(0, 0.0) = produceMultiPayload() {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + + if case .e(5, nil) = produceMultiPayload() {} // expected-warning {{type 'Int' is not optional, value can never be nil; this is an error in Swift 6}} + + // FIXME: Bad error (https://github.com/apple/swift/issues/64279) + if case .e(nil, 0) = produceMultiPayload() {} + // expected-error@-1 {{expression pattern of type 'String' cannot match values of type 'Substring'}} + // expected-note@-2 {{overloads for '~=' exist with these partially matching parameter lists}} + + if case .e(5, nil) = produceMultiPayload() as MultiPayload {} + if case .e(nil, 0) = produceMultiPayload() as MultiPayload {} + + // Enum patterns are solved together. + if case .e(E.a, .b) = produceMultiPayload() {} + if case .e(.a, E.b) = produceMultiPayload() {} + + // These also work because they start life as EnumPatterns. + if case .e(E.c, .d) = produceMultiPayload() {} + if case .e(.c, E.d) = produceMultiPayload() {} + for case .e(E.c, .d) in [produceMultiPayload()] {} + for case .e(.c, E.d) in [produceMultiPayload()] {} + + // Silly, but allowed. + if case 0: Int? = 0 {} // expected-warning {{non-optional expression of type 'Int' used in a check for optionals}} +} diff --git a/test/Constraints/rdar105781521.swift b/test/Constraints/rdar105781521.swift index ec76bf061d662..6e5f6eebe3092 100644 --- a/test/Constraints/rdar105781521.swift +++ b/test/Constraints/rdar105781521.swift @@ -12,7 +12,6 @@ func test(value: MyEnum) { switch value { case .first(true): // expected-error@-1 {{expression pattern of type 'Bool' cannot match values of type 'String'}} - // expected-note@-2 {{overloads for '~=' exist with these partially matching parameter lists: (Substring, String)}} break default: break diff --git a/test/Constraints/rdar105782480.swift b/test/Constraints/rdar105782480.swift new file mode 100644 index 0000000000000..581f7b3d0db3b --- /dev/null +++ b/test/Constraints/rdar105782480.swift @@ -0,0 +1,18 @@ +// RUN: %target-typecheck-verify-swift + +// rdar://105782480 +enum MyEnum { + case second(Int?) +} + +func takeClosure(_ x: () -> Void) {} + +func foo(value: MyEnum) { + takeClosure { + switch value { + case .second(let drag).invalid: + // expected-error@-1 {{value of type 'MyEnum' has no member 'invalid'}} + break + } + } +} diff --git a/test/Constraints/rdar106598067.swift b/test/Constraints/rdar106598067.swift new file mode 100644 index 0000000000000..941bbae2f2a2e --- /dev/null +++ b/test/Constraints/rdar106598067.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift + +enum E: Error { case e } + +// rdar://106598067 – Make sure we don't crash. +// FIXME: Bad diagnostic (the issue is that it should be written 'as', not 'as?') +let fn = { + // expected-error@-1 {{unable to infer closure type in the current context}} + do {} catch let x as? E {} + // expected-warning@-1 {{'catch' block is unreachable because no errors are thrown in 'do' block}} +} diff --git a/test/Constraints/rdar109419240.swift b/test/Constraints/rdar109419240.swift new file mode 100644 index 0000000000000..9d78a4c6cf831 --- /dev/null +++ b/test/Constraints/rdar109419240.swift @@ -0,0 +1,15 @@ +// RUN: %target-typecheck-verify-swift + +// rdar://109419240 – Make sure we don't crash +enum E { // expected-note {{'E' declared here}} + case e(Int) +} + +func foo(_ arr: [E]) -> Int { + return arr.reduce(0) { (total, elem) -> Int in + switch elem { + case let e(x): // expected-error {{cannot find 'e' in scope; did you mean 'E'?}} + return total + x + } + } +} diff --git a/test/Constraints/result_builder_diags.swift b/test/Constraints/result_builder_diags.swift index 660e719a0e569..5cd49c0cc771a 100644 --- a/test/Constraints/result_builder_diags.swift +++ b/test/Constraints/result_builder_diags.swift @@ -659,8 +659,6 @@ struct MyView { } @TupleBuilder var invalidCaseWithoutDot: some P { - // expected-error@-1 {{return type of property 'invalidCaseWithoutDot' requires that 'Either' conform to 'P'}} - // expected-note@-2 {{opaque return type declared here}} switch Optional.some(1) { case none: 42 // expected-error {{cannot find 'none' in scope}} case .some(let x): diff --git a/test/Constraints/result_builder_invalid_stmts.swift b/test/Constraints/result_builder_invalid_stmts.swift index 5de3ccc668b1f..5f8dc47268ee4 100644 --- a/test/Constraints/result_builder_invalid_stmts.swift +++ b/test/Constraints/result_builder_invalid_stmts.swift @@ -15,7 +15,6 @@ func foo(_ x: String) -> Int { if .random() { switch x { case 1: // expected-error {{expression pattern of type 'Int' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists}} 0 default: 1 @@ -29,7 +28,6 @@ func bar(_ x: String) -> Int { case 0: switch x { case 1: // expected-error {{expression pattern of type 'Int' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists}} 0 default: 1 @@ -44,7 +42,6 @@ func baz(_ x: String) -> Int { do { switch x { case 1: // expected-error {{expression pattern of type 'Int' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists}} 0 default: 1 @@ -57,7 +54,6 @@ func qux(_ x: String) -> Int { for _ in 0 ... 0 { switch x { case 1: // expected-error {{expression pattern of type 'Int' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists}} 0 default: 1 diff --git a/test/IDE/complete_unresolved_members.swift b/test/IDE/complete_unresolved_members.swift index 0e29f7935297e..f81f9a6c31ade 100644 --- a/test/IDE/complete_unresolved_members.swift +++ b/test/IDE/complete_unresolved_members.swift @@ -710,3 +710,38 @@ let _: DispatchTime = .#^UNRESOLVED_FUNCTION_CALL^#now() + 0.2 // UNRESOLVED_FUNCTION_CALL: Begin completions, 2 items // UNRESOLVED_FUNCTION_CALL-DAG: Decl[StaticMethod]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: now()[#DispatchTime#]; // UNRESOLVED_FUNCTION_CALL-DAG: Decl[Constructor]/CurrNominal/TypeRelation[Convertible]: init()[#DispatchTime#]; + +func id(_ x: T) -> T { x } + +func testNestedExprPatternCompletion(_ x: SomeEnum1) { + // Multi-statement closures have different type-checking code paths, + // so we need to test both. + let fn = { + switch x { + case id(.#^UNRESOLVED_NESTED1^#): + // UNRESOLVED_NESTED1: Begin completions, 3 items + // UNRESOLVED_NESTED1: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: South[#SomeEnum1#]; name=South + // UNRESOLVED_NESTED1: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: North[#SomeEnum1#]; name=North + // UNRESOLVED_NESTED1: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): SomeEnum1#})[#(into: inout Hasher) -> Void#]; name=hash(:) + break + } + if case id(.#^UNRESOLVED_NESTED2^#) = x {} + // UNRESOLVED_NESTED2: Begin completions, 3 items + // UNRESOLVED_NESTED2: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: South[#SomeEnum1#]; name=South + // UNRESOLVED_NESTED2: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: North[#SomeEnum1#]; name=North + // UNRESOLVED_NESTED2: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): SomeEnum1#})[#(into: inout Hasher) -> Void#]; name=hash(:) + } + switch x { + case id(.#^UNRESOLVED_NESTED3^#): + // UNRESOLVED_NESTED3: Begin completions, 3 items + // UNRESOLVED_NESTED3: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: South[#SomeEnum1#]; name=South + // UNRESOLVED_NESTED3: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: North[#SomeEnum1#]; name=North + // UNRESOLVED_NESTED3: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): SomeEnum1#})[#(into: inout Hasher) -> Void#]; name=hash(:) + break + } + if case id(.#^UNRESOLVED_NESTED4^#) = x {} + // UNRESOLVED_NESTED4: Begin completions, 3 items + // UNRESOLVED_NESTED4: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: South[#SomeEnum1#]; name=South + // UNRESOLVED_NESTED4: Decl[EnumElement]/CurrNominal/Flair[ExprSpecific]/TypeRelation[Convertible]: North[#SomeEnum1#]; name=North + // UNRESOLVED_NESTED4: Decl[InstanceMethod]/CurrNominal/TypeRelation[Invalid]: hash({#(self): SomeEnum1#})[#(into: inout Hasher) -> Void#]; name=hash(:) +} diff --git a/test/SILGen/switch_expr.swift b/test/SILGen/switch_expr.swift index 7275aa1bb534d..310dc90329ab3 100644 --- a/test/SILGen/switch_expr.swift +++ b/test/SILGen/switch_expr.swift @@ -605,3 +605,14 @@ struct TestLValues { opt![keyPath: kp] = switch Bool.random() { case true: 1 case false: throw Err() } } } + +func exprPatternInClosure() { + let f: (Int) -> Void = { i in + switch i { + case i: + () + default: + () + } + } +} diff --git a/test/stdlib/StringDiagnostics.swift b/test/stdlib/StringDiagnostics.swift index c3fde8ff884f5..3a53a26c69ee0 100644 --- a/test/stdlib/StringDiagnostics.swift +++ b/test/stdlib/StringDiagnostics.swift @@ -48,7 +48,6 @@ func testAmbiguousStringComparisons(s: String) { // Shouldn't suggest 'as' in a pattern-matching context, as opposed to all these other situations if case nsString = "" {} // expected-error{{expression pattern of type 'NSString' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists: (Substring, String)}} } func testStringDeprecation(hello: String) { diff --git a/test/stmt/errors.swift b/test/stmt/errors.swift index d71da201b4247..506b2230f775a 100644 --- a/test/stmt/errors.swift +++ b/test/stmt/errors.swift @@ -169,7 +169,8 @@ func thirteen() { thirteen_helper { (a) in // expected-error {{invalid conversion from throwing function of type '(Thirteen) throws -> ()' to non-throwing function type '(Thirteen) -> ()'}} do { try thrower() - } catch a { + // FIXME: Bad diagnostic (https://github.com/apple/swift/issues/63459) + } catch a { // expected-error {{binary operator '~=' cannot be applied to two 'any Error' operands}} } } } diff --git a/validation-test/Sema/SwiftUI/rdar70256351.swift b/validation-test/Sema/SwiftUI/rdar70256351.swift index 0881009bd7457..1aa7ad723e977 100644 --- a/validation-test/Sema/SwiftUI/rdar70256351.swift +++ b/validation-test/Sema/SwiftUI/rdar70256351.swift @@ -10,7 +10,6 @@ struct ContentView: View { var body: some View { switch currentPage { case 1: // expected-error {{expression pattern of type 'Int' cannot match values of type 'String'}} - // expected-note@-1 {{overloads for '~=' exist with these partially matching parameter lists: (Substring, String)}} Text("1") default: Text("default") diff --git a/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift b/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift index 2e7e10322c932..fd6652a85f4e4 100644 --- a/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift +++ b/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift @@ -9,12 +9,24 @@ let _: () -> Void = { let _: () -> Void = { for case (0)? in [a] {} + // expected-error@-1 {{pattern cannot match values of type 'Any?'}} if case (0, 0) = a {} - // expected-error@-1 {{cannot convert value of type 'Any?' to specified type '(_, _)}} } let _: () -> Void = { for case (0)? in [a] {} + // expected-error@-1 {{pattern cannot match values of type 'Any?'}} for case (0, 0) in [a] {} - // expected-error@-1 {{cannot convert value of type 'Any?' to expected element type '(_, _)'}} +} + +let _: () -> Void = { + if case (0, 0) = a {} + // expected-error@-1 {{cannot convert value of type 'Any?' to specified type '(Int, Int)'}} + for case (0)? in [a] {} +} + +let _: () -> Void = { + for case (0, 0) in [a] {} + // expected-error@-1 {{cannot convert value of type 'Any?' to expected element type '(Int, Int)'}} + for case (0)? in [a] {} } From 1d4a0f3c3f7bf3b12fb63b0183d44e101fbeaf70 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:38 +0100 Subject: [PATCH 08/11] [CS] Don't form conversion between switch subject and pattern This is wrong because there's nowhere to put any conversion that is introduced, meaning that we'll likely crash in SILGen. Change the constraint to equality, which matches what we do outside of the constraint system. rdar://107709341 --- include/swift/AST/DiagnosticsSema.def | 3 +++ lib/Sema/CSDiagnostics.cpp | 2 ++ lib/Sema/CSSyntacticElement.cpp | 7 +++++-- test/Constraints/rdar107709341.swift | 14 ++++++++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/Constraints/rdar107709341.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 165a78aaf2104..7c2e1734af7b7 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -197,6 +197,9 @@ ERROR(cannot_match_expr_tuple_pattern_with_nontuple_value,none, ERROR(cannot_match_unresolved_expr_pattern_with_value,none, "pattern cannot match values of type %0", (Type)) +ERROR(cannot_match_value_with_pattern,none, + "pattern of type %1 cannot match %0", + (Type, Type)) ERROR(cannot_reference_compare_types,none, "cannot check reference equality of functions; operands here have types " diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 8bf6079b6155d..73de17b18c75c 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -3549,6 +3549,8 @@ ContextualFailure::getDiagnosticFor(ContextualTypePurpose context, return diag::cannot_convert_discard_value; case CTP_CaseStmt: + return diag::cannot_match_value_with_pattern; + case CTP_ThrowStmt: case CTP_ForEachStmt: case CTP_ForEachSequence: diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 36529ffad526a..8fdec6b319a5d 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -657,8 +657,11 @@ class SyntacticElementConstraintGenerator // Convert the contextual type to the pattern, which establishes the // bindings. - cs.addConstraint(ConstraintKind::Conversion, context.getType(), patternType, - locator); + auto *loc = cs.getConstraintLocator( + locator, {LocatorPathElt::PatternMatch(pattern), + LocatorPathElt::ContextualType(context.purpose)}); + cs.addConstraint(ConstraintKind::Equal, context.getType(), patternType, + loc); // For any pattern variable that has a parent variable (i.e., another // pattern variable with the same name in the same case), require that diff --git a/test/Constraints/rdar107709341.swift b/test/Constraints/rdar107709341.swift new file mode 100644 index 0000000000000..3358bf86eda05 --- /dev/null +++ b/test/Constraints/rdar107709341.swift @@ -0,0 +1,14 @@ +// RUN: %target-typecheck-verify-swift + +// rdar://107709341 – Make sure we don't crash. +func foo(_ x: Int) { + // FIXME: We ought to have a better diagnostic + let _ = { // expected-error {{unable to infer closure type in the current context}} + switch x { + case Optional.some(x): + break + default: + break + } + } +} From 2dadb52dd6fc0e12993b9431487438b60e03d1cb Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:39 +0100 Subject: [PATCH 09/11] [CS] Improve diagnostics a bit for pattern mismatch There's still plenty of more work to do here for pattern diagnostics, including introducing a bunch of new locator elements, and handling things like argument list mismatches. This at least lets us fall back to a generic mismatch diagnostic. --- include/swift/Sema/ConstraintLocator.h | 3 +++ .../swift/Sema/ConstraintLocatorPathElts.def | 4 +++ lib/Sema/CSDiagnostics.cpp | 19 +++++++++++-- lib/Sema/CSGen.cpp | 27 +++++++++---------- lib/Sema/CSSimplify.cpp | 18 +++++++++++-- lib/Sema/ConstraintLocator.cpp | 9 +++++++ lib/Sema/ConstraintSystem.cpp | 5 ++++ test/Constraints/rdar107709341.swift | 5 ++-- 8 files changed, 69 insertions(+), 21 deletions(-) diff --git a/include/swift/Sema/ConstraintLocator.h b/include/swift/Sema/ConstraintLocator.h index 71fd781385d97..981b70188ebfc 100644 --- a/include/swift/Sema/ConstraintLocator.h +++ b/include/swift/Sema/ConstraintLocator.h @@ -318,6 +318,9 @@ class ConstraintLocator : public llvm::FoldingSetNode { /// otherwise \c nullptr. NullablePtr getPatternMatch() const; + /// Whether the locator in question is for a pattern match. + bool isForPatternMatch() const; + /// Returns true if \p locator is ending with either of the following /// - Member /// - Member -> KeyPathDynamicMember diff --git a/include/swift/Sema/ConstraintLocatorPathElts.def b/include/swift/Sema/ConstraintLocatorPathElts.def index 62649588386f0..c60640e114cd9 100644 --- a/include/swift/Sema/ConstraintLocatorPathElts.def +++ b/include/swift/Sema/ConstraintLocatorPathElts.def @@ -225,6 +225,10 @@ CUSTOM_LOCATOR_PATH_ELT(TernaryBranch) /// Performing a pattern patch. CUSTOM_LOCATOR_PATH_ELT(PatternMatch) +/// The constraint that models the allowed implicit casts for +/// an EnumElementPattern. +SIMPLE_LOCATOR_PATH_ELT(EnumPatternImplicitCastMatch) + /// Points to a particular attribute associated with one of /// the arguments e.g. `inout` or its type e.g. `@escaping`. /// diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 73de17b18c75c..43231228d83be 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2715,6 +2715,19 @@ bool ContextualFailure::diagnoseAsError() { return false; } + case ConstraintLocator::EnumPatternImplicitCastMatch: { + // In this case, the types are reversed, as we are checking whether we + // can convert the pattern type to the context type. + std::swap(fromType, toType); + diagnostic = diag::cannot_match_value_with_pattern; + break; + } + + case ConstraintLocator::PatternMatch: { + diagnostic = diag::cannot_match_value_with_pattern; + break; + } + default: return false; } @@ -2927,9 +2940,11 @@ bool ContextualFailure::diagnoseConversionToNil() const { void ContextualFailure::tryFixIts(InFlightDiagnostic &diagnostic) const { auto *locator = getLocator(); // Can't apply any of the fix-its below if this failure - // is related to `inout` argument. - if (locator->isLastElement()) + // is related to `inout` argument, or a pattern mismatch. + if (locator->isLastElement() || + locator->isForPatternMatch()) { return; + } if (trySequenceSubsequenceFixIts(diagnostic)) return; diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 80fe6f42c2b8a..9818749ae32e1 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2783,6 +2783,9 @@ namespace { if (dyn_cast_or_null(enumPattern->getSubPattern())) functionRefKind = FunctionRefKind::Compound; + auto patternLocator = + locator.withPathElement(LocatorPathElt::PatternMatch(pattern)); + if (enumPattern->getParentType() || enumPattern->getParentTypeRepr()) { // Resolve the parent type. const auto parentType = [&] { @@ -2810,25 +2813,23 @@ namespace { // Perform member lookup into the parent's metatype. Type parentMetaType = MetatypeType::get(parentType); - CS.addValueMemberConstraint( - parentMetaType, enumPattern->getName(), memberType, CurDC, - functionRefKind, {}, - CS.getConstraintLocator(locator, - LocatorPathElt::PatternMatch(pattern))); + CS.addValueMemberConstraint(parentMetaType, enumPattern->getName(), + memberType, CurDC, functionRefKind, {}, + patternLocator); // Parent type needs to be convertible to the pattern type; this // accounts for cases where the pattern type is existential. CS.addConstraint( ConstraintKind::Conversion, parentType, patternType, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + patternLocator.withPathElement( + ConstraintLocator::EnumPatternImplicitCastMatch)); baseType = parentType; } else { // Use the pattern type for member lookup. CS.addUnresolvedValueMemberConstraint( MetatypeType::get(patternType), enumPattern->getName(), - memberType, CurDC, functionRefKind, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + memberType, CurDC, functionRefKind, patternLocator); baseType = patternType; } @@ -2865,13 +2866,11 @@ namespace { // NOTE: The order here is important! Pattern matching equality is // not symmetric (we need to fix that either by using a different // constraint, or actually making it symmetric). - CS.addConstraint( - ConstraintKind::Equal, functionType, memberType, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + CS.addConstraint(ConstraintKind::Equal, functionType, memberType, + patternLocator); - CS.addConstraint( - ConstraintKind::Conversion, outputType, baseType, - locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); + CS.addConstraint(ConstraintKind::Conversion, outputType, baseType, + patternLocator); } return setType(patternType); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index f5d4e5f965c39..9be88e5527f57 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -6352,8 +6352,20 @@ bool ConstraintSystem::repairFailures( break; } + case ConstraintLocator::EnumPatternImplicitCastMatch: { + // If either type is a placeholder, consider this fixed. + if (lhs->isPlaceholder() || rhs->isPlaceholder()) + return true; + + conversionsOrFixes.push_back(ContextualMismatch::create( + *this, lhs, rhs, getConstraintLocator(locator))); + break; + } + case ConstraintLocator::PatternMatch: { auto *pattern = elt.castTo().getPattern(); + + // TODO: We ought to introduce a new locator element for this. bool isMemberMatch = lhs->is() && isa(pattern); @@ -6368,13 +6380,12 @@ bool ConstraintSystem::repairFailures( if (lhs->isPlaceholder() || rhs->isPlaceholder()) return true; - // If member reference didn't match expected pattern, - // let's consider that a contextual mismatch. if (isMemberMatch) { recordAnyTypeVarAsPotentialHole(lhs); recordAnyTypeVarAsPotentialHole(rhs); conversionsOrFixes.push_back(AllowAssociatedValueMismatch::create( *this, lhs, rhs, getConstraintLocator(locator))); + break; } // `weak` declaration with an explicit non-optional type e.g. @@ -6396,6 +6407,9 @@ bool ConstraintSystem::repairFailures( } } + conversionsOrFixes.push_back(ContextualMismatch::create( + *this, lhs, rhs, getConstraintLocator(locator))); + break; } diff --git a/lib/Sema/ConstraintLocator.cpp b/lib/Sema/ConstraintLocator.cpp index e3fe38458601b..4987c3a47cb21 100644 --- a/lib/Sema/ConstraintLocator.cpp +++ b/lib/Sema/ConstraintLocator.cpp @@ -90,6 +90,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const { case ConstraintLocator::ImplicitCallAsFunction: case ConstraintLocator::TernaryBranch: case ConstraintLocator::PatternMatch: + case ConstraintLocator::EnumPatternImplicitCastMatch: case ConstraintLocator::ArgumentAttribute: case ConstraintLocator::UnresolvedMemberChainResult: case ConstraintLocator::PlaceholderType: @@ -403,6 +404,10 @@ void LocatorPathElt::dump(raw_ostream &out) const { out << "pattern match"; break; + case ConstraintLocator::EnumPatternImplicitCastMatch: + out << "enum pattern implicit cast match"; + break; + case ConstraintLocator::ArgumentAttribute: { using AttrLoc = LocatorPathElt::ArgumentAttribute; @@ -677,6 +682,10 @@ NullablePtr ConstraintLocator::getPatternMatch() const { return matchElt->getPattern(); } +bool ConstraintLocator::isForPatternMatch() const { + return getPatternMatch() != nullptr; +} + bool ConstraintLocator::isMemberRef() const { if (isLastElement()) { return true; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 0a2a64c48ccb1..244e9ccdc72cc 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -5824,6 +5824,11 @@ void constraints::simplifyLocator(ASTNode &anchor, continue; } + case ConstraintLocator::EnumPatternImplicitCastMatch: { + path = path.slice(1); + continue; + } + case ConstraintLocator::PackType: case ConstraintLocator::ParentType: case ConstraintLocator::KeyPathType: diff --git a/test/Constraints/rdar107709341.swift b/test/Constraints/rdar107709341.swift index 3358bf86eda05..cc38962ec4f12 100644 --- a/test/Constraints/rdar107709341.swift +++ b/test/Constraints/rdar107709341.swift @@ -2,10 +2,9 @@ // rdar://107709341 – Make sure we don't crash. func foo(_ x: Int) { - // FIXME: We ought to have a better diagnostic - let _ = { // expected-error {{unable to infer closure type in the current context}} + let _ = { switch x { - case Optional.some(x): + case Optional.some(x): // expected-error {{pattern of type 'Optional' cannot match 'Int'}} {{none}} break default: break From 052be3902b472232178cd9e3bdcd3907c9ac2895 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Wed, 24 May 2023 16:40:39 +0100 Subject: [PATCH 10/11] [CS] Fix `coercePatternToType` enum cast handling Previously if the cast was unresolved, we would emit a warning and bail with `nullptr`. This is wrong, because the caller expects a `nullptr` return to mean we emitted an error. Change the diagnostic to an error to fix this. This may appear source breaking, but in reality previously we were failing to add the cast at all in this case, which lead to a crash in SILGen. We really do want to reject these cases as errors, as this will give us a better opportunity to fall back to type-checking as ExprPatterns, and better matches the constraint solver type-checking. Also while we're here, change the diagnostic for the case where we don't have an existential context type from the confusing "enum doesn't have member" diagnostic to the pattern mismatch diagnostic. rdar://107420031 --- lib/Sema/TypeCheckPattern.cpp | 11 +++++------ test/Constraints/patterns.swift | 2 +- test/Constraints/rdar107420031.swift | 18 ++++++++++++++++++ test/Parse/matching_patterns.swift | 2 +- .../matching_patterns_reference_bindings.swift | 2 +- 5 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 test/Constraints/rdar107420031.swift diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 9b42c5c8896a5..69260e9a0975e 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -1570,9 +1570,8 @@ Pattern *TypeChecker::coercePatternToType( type, parentTy, CheckedCastContextKind::EnumElementPattern, dc); // If the cast failed, we can't resolve the pattern. if (foundCastKind < CheckedCastKind::First_Resolved) { - diags - .diagnose(EEP->getLoc(), diag::downcast_to_unrelated, type, - parentTy) + diags.diagnose(EEP->getLoc(), diag::cannot_match_value_with_pattern, + type, parentTy) .highlight(EEP->getSourceRange()); return nullptr; } @@ -1582,9 +1581,9 @@ Pattern *TypeChecker::coercePatternToType( castKind = foundCastKind; enumTy = parentTy; } else { - diags.diagnose(EEP->getLoc(), - diag::enum_element_pattern_not_member_of_enum, - EEP->getName(), type); + diags.diagnose(EEP->getLoc(), diag::cannot_match_value_with_pattern, + type, parentTy) + .highlight(EEP->getSourceRange()); return nullptr; } } diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index c1ee5f5d0378b..e20d93be43897 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -120,7 +120,7 @@ case iPadHair.HairForceOne: () case iPadHair.HairForceOne: // expected-error{{generic enum type 'iPadHair' is ambiguous without explicit generic parameters when matching value of type 'any HairType'}} () -case Watch.Edition: // expected-warning {{cast from 'any HairType' to unrelated type 'Watch' always fails}} +case Watch.Edition: // expected-error {{pattern of type 'Watch' cannot match 'any HairType'}} () case .HairForceOne: // expected-error{{type 'any HairType' has no member 'HairForceOne'}} () diff --git a/test/Constraints/rdar107420031.swift b/test/Constraints/rdar107420031.swift new file mode 100644 index 0000000000000..2f5687eb6fe31 --- /dev/null +++ b/test/Constraints/rdar107420031.swift @@ -0,0 +1,18 @@ +// RUN: %target-typecheck-verify-swift + +enum E { + case e +} + +func ~= (lhs: any Error, rhs: E) -> Bool { true } + +// rdar://107420031 – Make sure we don't crash. +// TODO: This ought to compile. +func foo(_ error: any Error) { + switch error { + case E.e: // expected-error {{pattern of type 'E' cannot match 'any Error'}} + break + default: + break + } +} diff --git a/test/Parse/matching_patterns.swift b/test/Parse/matching_patterns.swift index 1fbec74a906e8..050916fac0574 100644 --- a/test/Parse/matching_patterns.swift +++ b/test/Parse/matching_patterns.swift @@ -122,7 +122,7 @@ if case let .Naught(value1, value2, value3) = n {} // expected-error{{pattern wi switch n { -case Foo.A: // expected-error{{enum case 'A' is not a member of type 'Voluntary'}} +case Foo.A: // expected-error{{pattern of type 'Foo' cannot match 'Voluntary'}} () case Voluntary.Naught, Voluntary.Naught(), // expected-error {{pattern with associated values does not match enum case 'Naught'}} diff --git a/test/Parse/matching_patterns_reference_bindings.swift b/test/Parse/matching_patterns_reference_bindings.swift index fadfd53ac9821..fbf6b9840b1b2 100644 --- a/test/Parse/matching_patterns_reference_bindings.swift +++ b/test/Parse/matching_patterns_reference_bindings.swift @@ -140,7 +140,7 @@ if case inout .Naught(value1, value2, value3) = n {} // expected-error{{pattern switch n { -case Foo.A: // expected-error{{enum case 'A' is not a member of type 'Voluntary'}} +case Foo.A: // expected-error{{pattern of type 'Foo' cannot match 'Voluntary'}} () case Voluntary.Naught, Voluntary.Naught(), // expected-error {{pattern with associated values does not match enum case 'Naught'}} From 07ef3904a6a28d118ee1f15b0fedb3be199512a2 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Tue, 16 May 2023 19:01:14 +0100 Subject: [PATCH 11/11] [CS] Allow bidirectional inference for ExprPatterns Rather than using a conjunction, just form the constraints for the match expression right away. --- include/swift/AST/Pattern.h | 36 ++++-- include/swift/Sema/ConstraintSystem.h | 19 ++- lib/Sema/CSGen.cpp | 118 ++++++++++++------ lib/Sema/CSSyntacticElement.cpp | 61 +-------- test/Constraints/patterns.swift | 35 ++---- test/Misc/misc_diagnostics.swift | 5 +- test/stdlib/StringDiagnostics.swift | 6 +- .../issue-65360.swift | 4 +- 8 files changed, 134 insertions(+), 150 deletions(-) diff --git a/include/swift/AST/Pattern.h b/include/swift/AST/Pattern.h index f37e73b7dce68..7e47e095b9f78 100644 --- a/include/swift/AST/Pattern.h +++ b/include/swift/AST/Pattern.h @@ -140,9 +140,11 @@ class alignas(8) Pattern : public ASTAllocated { /// equivalent to matching this pattern. /// /// Looks through ParenPattern, BindingPattern, and TypedPattern. - Pattern *getSemanticsProvidingPattern(); - const Pattern *getSemanticsProvidingPattern() const { - return const_cast(this)->getSemanticsProvidingPattern(); + Pattern *getSemanticsProvidingPattern(bool allowTypedPattern = true); + const Pattern * + getSemanticsProvidingPattern(bool allowTypedPattern = true) const { + return const_cast(this)->getSemanticsProvidingPattern( + allowTypedPattern); } /// Returns whether this pattern has been type-checked yet. @@ -799,14 +801,26 @@ class BindingPattern : public Pattern { } }; -inline Pattern *Pattern::getSemanticsProvidingPattern() { - if (auto *pp = dyn_cast(this)) - return pp->getSubPattern()->getSemanticsProvidingPattern(); - if (auto *tp = dyn_cast(this)) - return tp->getSubPattern()->getSemanticsProvidingPattern(); - if (auto *vp = dyn_cast(this)) - return vp->getSubPattern()->getSemanticsProvidingPattern(); - return this; +inline Pattern *Pattern::getSemanticsProvidingPattern(bool allowTypedPattern) { + auto *P = this; + while (true) { + if (auto *PP = dyn_cast(P)) { + P = PP->getSubPattern(); + continue; + } + if (auto *BP = dyn_cast(P)) { + P = BP->getSubPattern(); + continue; + } + if (allowTypedPattern) { + if (auto *TP = dyn_cast(P)) { + P = TP->getSubPattern(); + continue; + } + } + break; + } + return P; } /// Describes a pattern and the context in which it occurs. diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 22143032333fe..a6292967a3e2d 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -4345,11 +4345,6 @@ class ConstraintSystem { /// \returns \c true if constraint generation failed, \c false otherwise bool generateConstraints(SingleValueStmtExpr *E); - /// Generate constraints for an array of ExprPatterns, forming a conjunction - /// that solves each expression in turn. - void generateConstraints(ArrayRef exprPatterns, - ConstraintLocatorBuilder locator); - /// Generate constraints for the given (unchecked) expression. /// /// \returns a possibly-sanitized expression, or null if an error occurred. @@ -4357,15 +4352,15 @@ class ConstraintSystem { Expr *generateConstraints(Expr *E, DeclContext *dc, bool isInputExpression = true); - /// Generate constraints for binding the given pattern to the - /// value of the given expression. + /// Generate constraints for a given pattern. /// - /// \returns a possibly-sanitized initializer, or null if an error occurred. + /// \returns The type of the pattern, or \c None if a failure occured. [[nodiscard]] - Type generateConstraints(Pattern *P, ConstraintLocatorBuilder locator, - bool bindPatternVarsOneWay, - PatternBindingDecl *patternBinding, - unsigned patternIndex); + Optional generateConstraints(Pattern *P, + ConstraintLocatorBuilder locator, + bool bindPatternVarsOneWay, + PatternBindingDecl *patternBinding, + unsigned patternIndex); /// Generate constraints for a statement condition. /// diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 9818749ae32e1..e8cbcaebe0e50 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -2442,7 +2442,7 @@ namespace { /// for the types of each variable declared within the pattern, along /// with a one-way constraint binding that to the type to which the /// variable will be ascribed or inferred. - Type getTypeForPattern( + Optional getTypeForPattern( Pattern *pattern, ConstraintLocatorBuilder locator, bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding = nullptr, @@ -2466,14 +2466,21 @@ namespace { locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - return setType(ParenType::get(CS.getASTContext(), underlyingType)); + if (!underlyingType) + return None; + + return setType(ParenType::get(CS.getASTContext(), *underlyingType)); } case PatternKind::Binding: { auto *subPattern = cast(pattern)->getSubPattern(); auto type = getTypeForPattern(subPattern, locator, bindPatternVarsOneWay); + + if (!type) + return None; + // Var doesn't affect the type. - return setType(type); + return setType(*type); } case PatternKind::Any: { Type type; @@ -2653,6 +2660,9 @@ namespace { Type type = TypeChecker::typeCheckPattern(contextualPattern); + if (!type) + return None; + // Look through reference storage types. type = type->getReferenceStorageReferent(); @@ -2664,16 +2674,19 @@ namespace { auto *subPattern = cast(pattern)->getSubPattern(); // Determine the subpattern type. It will be convertible to the // ascribed type. - Type subPatternType = getTypeForPattern( + auto subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); + if (!subPatternType) + return None; + // NOTE: The order here is important! Pattern matching equality is // not symmetric (we need to fix that either by using a different // constraint, or actually making it symmetric). CS.addConstraint( - ConstraintKind::Equal, openedType, subPatternType, + ConstraintKind::Equal, openedType, *subPatternType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); // FIXME [OPAQUE SUPPORT]: the distinction between where we want opaque @@ -2693,12 +2706,15 @@ namespace { auto &tupleElt = tuplePat->getElement(i); auto *eltPattern = tupleElt.getPattern(); - Type eltTy = getTypeForPattern( + auto eltTy = getTypeForPattern( eltPattern, locator.withPathElement(LocatorPathElt::PatternMatch(eltPattern)), bindPatternVarsOneWay); - tupleTypeElts.push_back(TupleTypeElt(eltTy, tupleElt.getLabel())); + if (!eltTy) + return None; + + tupleTypeElts.push_back(TupleTypeElt(*eltTy, tupleElt.getLabel())); } return setType(TupleType::get(tupleTypeElts, CS.getASTContext())); @@ -2707,12 +2723,15 @@ namespace { case PatternKind::OptionalSome: { auto *subPattern = cast(pattern)->getSubPattern(); // The subpattern must have optional type. - Type subPatternType = getTypeForPattern( + auto subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); - return setType(OptionalType::get(subPatternType)); + if (!subPatternType) + return None; + + return setType(OptionalType::get(*subPatternType)); } case PatternKind::Is: { @@ -2742,12 +2761,14 @@ namespace { subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); + if (!subPatternType) + return None; // NOTE: The order here is important! Pattern matching equality is // not symmetric (we need to fix that either by using a different // constraint, or actually making it symmetric). CS.addConstraint( - ConstraintKind::Equal, castType, subPatternType, + ConstraintKind::Equal, castType, *subPatternType, locator.withPathElement(LocatorPathElt::PatternMatch(pattern))); } return setType(isType); @@ -2811,6 +2832,9 @@ namespace { TypeResolverContext::InExpression, patternMatchLoc); }(); + if (!parentType) + return None; + // Perform member lookup into the parent's metatype. Type parentMetaType = MetatypeType::get(parentType); CS.addValueMemberConstraint(parentMetaType, enumPattern->getName(), @@ -2838,13 +2862,13 @@ namespace { // When there is a subpattern, the member will have function type, // and we're matching the type of that subpattern to the parameter // types. - Type subPatternType = getTypeForPattern( + auto subPatternType = getTypeForPattern( subPattern, locator.withPathElement(LocatorPathElt::PatternMatch(subPattern)), bindPatternVarsOneWay); SmallVector params; - decomposeTuple(subPatternType, params); + decomposeTuple(*subPatternType, params); // Remove parameter labels; they aren't used when matching cases, // but outright conflicts will be checked during coercion. @@ -2877,10 +2901,24 @@ namespace { } case PatternKind::Expr: { - // We generate constraints for ExprPatterns in a separate pass. For - // now, just create a type variable. - return setType(CS.createTypeVariable(CS.getConstraintLocator(locator), - TVO_CanBindToNoEscape)); + auto *EP = cast(pattern); + Type patternTy = CS.createTypeVariable(CS.getConstraintLocator(locator), + TVO_CanBindToNoEscape); + + auto target = SyntacticElementTarget::forExprPattern(EP); + + if (CS.preCheckTarget(target, /*replaceInvalidRefWithErrors=*/true, + /*leaveClosureBodyUnchecked=*/false)) { + return None; + } + CS.setType(EP->getMatchVar(), patternTy); + + if (CS.generateConstraints(target)) + return None; + + CS.setTargetFor(EP, target); + CS.setExprPatternFor(EP->getSubExpr(), EP); + return setType(patternTy); } } @@ -4318,11 +4356,19 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs, initializer, LocatorPathElt::ContextualType(CTP_Initialization)); Type patternType; + bool forExprPattern = false; if (auto pattern = target.getInitializationPattern()) { - patternType = cs.generateConstraints( + auto *semanticPattern = + pattern->getSemanticsProvidingPattern(/*allowTypedPattern*/ false); + forExprPattern = isa(semanticPattern); + auto ty = cs.generateConstraints( pattern, locator, target.shouldBindPatternVarsOneWay(), target.getInitializationPatternBindingDecl(), target.getInitializationPatternBindingIndex()); + if (!ty) + return true; + + patternType = *ty; } else { patternType = cs.createTypeVariable(locator, TVO_CanBindToNoEscape); } @@ -4331,9 +4377,15 @@ static bool generateInitPatternConstraints(ConstraintSystem &cs, return cs.generateWrappedPropertyTypeConstraints( wrappedVar, cs.getType(target.getAsExpr()), patternType); - // Add a conversion constraint between the types. - cs.addConstraint(ConstraintKind::Conversion, cs.getType(target.getAsExpr()), - patternType, locator, /*isFavored*/true); + // Add a constraint between the types. For ExprPatterns, we want an equality + // constraint, because we want to propagate the type of the initializer directly + // into the implicit '~=' call. We'll then allow conversions when matching that as + // an argument. This avoids producing bad diagnostics where we try and apply fixes + // to the conversion outside of the call. + auto kind = forExprPattern ? ConstraintKind::Equal + : ConstraintKind::Conversion; + cs.addConstraint(kind, cs.getType(target.getAsExpr()), + patternType, locator, /*isFavored*/true); return false; } @@ -4481,7 +4533,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, // Collect constraints from the element pattern. auto elementLocator = cs.getConstraintLocator( sequenceExpr, ConstraintLocator::SequenceElementType); - Type initType = + auto initType = cs.generateConstraints(pattern, elementLocator, target.shouldBindPatternVarsOneWay(), nullptr, 0); if (!initType) @@ -4500,7 +4552,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, // resolving `optional object` constraint which is sometimes too eager. cs.addConstraint(ConstraintKind::Conversion, nextType, OptionalType::get(elementType), elementTypeLoc); - cs.addConstraint(ConstraintKind::Conversion, elementType, initType, + cs.addConstraint(ConstraintKind::Conversion, elementType, *initType, elementLocator); } @@ -4526,7 +4578,7 @@ generateForEachStmtConstraints(ConstraintSystem &cs, // Populate all of the information for a for-each loop. forEachStmtInfo.elementType = elementType; - forEachStmtInfo.initType = initType; + forEachStmtInfo.initType = *initType; target.setPattern(pattern); target.getForEachStmtInfo() = forEachStmtInfo; return target; @@ -4706,7 +4758,7 @@ bool ConstraintSystem::generateConstraints( // Generate constraints to bind all of the internal declarations // and verify the pattern. - Type patternType = generateConstraints( + auto patternType = generateConstraints( pattern, locator, /*shouldBindPatternVarsOneWay*/ true, target.getPatternBindingOfUninitializedVar(), target.getIndexOfUninitializedVar()); @@ -4735,25 +4787,13 @@ Expr *ConstraintSystem::generateConstraints( return generateConstraintsFor(*this, expr, dc); } -Type ConstraintSystem::generateConstraints( +Optional ConstraintSystem::generateConstraints( Pattern *pattern, ConstraintLocatorBuilder locator, bool bindPatternVarsOneWay, PatternBindingDecl *patternBinding, unsigned patternIndex) { ConstraintGenerator cg(*this, nullptr); - auto ty = cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay, - patternBinding, patternIndex); - assert(ty); - - // Gather the ExprPatterns, and form a conjunction for their expressions. - SmallVector exprPatterns; - pattern->forEachNode([&](Pattern *P) { - if (auto *EP = dyn_cast(P)) - exprPatterns.push_back(EP); - }); - if (!exprPatterns.empty()) - generateConstraints(exprPatterns, getConstraintLocator(pattern)); - - return ty; + return cg.getTypeForPattern(pattern, locator, bindPatternVarsOneWay, + patternBinding, patternIndex); } bool ConstraintSystem::generateConstraints(StmtCondition condition, diff --git a/lib/Sema/CSSyntacticElement.cpp b/lib/Sema/CSSyntacticElement.cpp index 8fdec6b319a5d..0c44c3093187b 100644 --- a/lib/Sema/CSSyntacticElement.cpp +++ b/lib/Sema/CSSyntacticElement.cpp @@ -410,7 +410,7 @@ ElementInfo makeJoinElement(ConstraintSystem &cs, TypeJoinExpr *join, struct SyntacticElementContext : public llvm::PointerUnion { + SingleValueStmtExpr *> { // Inherit the constructors from PointerUnion. using PointerUnion::PointerUnion; @@ -441,10 +441,6 @@ struct SyntacticElementContext return context; } - static SyntacticElementContext forExprPattern(ExprPattern *EP) { - return SyntacticElementContext{EP}; - } - DeclContext *getAsDeclContext() const { if (auto *fn = this->dyn_cast()) { return fn; @@ -452,8 +448,6 @@ struct SyntacticElementContext return closure; } else if (auto *SVE = dyn_cast()) { return SVE->getDeclContext(); - } else if (auto *EP = dyn_cast()) { - return EP->getDeclContext(); } else { llvm_unreachable("unsupported kind"); } @@ -525,32 +519,7 @@ class SyntacticElementConstraintGenerator ConstraintLocator *locator) : cs(cs), context(context), locator(locator) {} - void visitExprPattern(ExprPattern *EP) { - auto target = SyntacticElementTarget::forExprPattern(EP); - - if (cs.preCheckTarget(target, /*replaceInvalidRefWithErrors=*/true, - /*leaveClosureBodyUnchecked=*/false)) { - hadError = true; - return; - } - cs.setType(EP->getMatchVar(), cs.getType(EP)); - - if (cs.generateConstraints(target)) { - hadError = true; - return; - } - cs.setTargetFor(EP, target); - cs.setExprPatternFor(EP->getSubExpr(), EP); - } - - void visitPattern(Pattern *pattern, ContextualTypeInfo contextInfo) { - if (context.is()) { - // This is for an ExprPattern conjunction, go ahead and generate - // constraints for the match expression. - visitExprPattern(cast(pattern)); - return; - } - + void visitPattern(Pattern *pattern, ContextualTypeInfo context) { auto parentElement = locator->getLastElementAs(); @@ -566,7 +535,7 @@ class SyntacticElementConstraintGenerator } if (isa(stmt)) { - visitCaseItemPattern(pattern, contextInfo); + visitCaseItemPattern(pattern, context); return; } } @@ -646,7 +615,7 @@ class SyntacticElementConstraintGenerator } void visitCaseItemPattern(Pattern *pattern, ContextualTypeInfo context) { - Type patternType = cs.generateConstraints( + auto patternType = cs.generateConstraints( pattern, locator, /*bindPatternVarsOneWay=*/false, /*patternBinding=*/nullptr, /*patternIndex=*/0); @@ -660,7 +629,7 @@ class SyntacticElementConstraintGenerator auto *loc = cs.getConstraintLocator( locator, {LocatorPathElt::PatternMatch(pattern), LocatorPathElt::ContextualType(context.purpose)}); - cs.addConstraint(ConstraintKind::Equal, context.getType(), patternType, + cs.addConstraint(ConstraintKind::Equal, context.getType(), *patternType, loc); // For any pattern variable that has a parent variable (i.e., another @@ -1472,24 +1441,6 @@ bool ConstraintSystem::generateConstraints(SingleValueStmtExpr *E) { return generator.hadError; } -void ConstraintSystem::generateConstraints(ArrayRef exprPatterns, - ConstraintLocatorBuilder locator) { - // Form a conjunction of ExprPattern elements, isolated from the rest of the - // pattern. - SmallVector elements; - SmallVector referencedTypeVars; - for (auto *EP : exprPatterns) { - auto ty = getType(EP)->castTo(); - referencedTypeVars.push_back(ty); - - ContextualTypeInfo context(ty, CTP_ExprPattern); - elements.push_back(makeElement(EP, getConstraintLocator(EP), context)); - } - auto *loc = getConstraintLocator(locator); - createConjunction(*this, elements, loc, /*isIsolated*/ true, - referencedTypeVars); -} - bool ConstraintSystem::isInResultBuilderContext(ClosureExpr *closure) const { if (!closure->hasSingleExpressionBody()) { auto *DC = closure->getParent(); @@ -1540,8 +1491,6 @@ ConstraintSystem::simplifySyntacticElementConstraint( context = SyntacticElementContext::forFunction(fn); } else if (auto *SVE = getAsExpr(anchor)) { context = SyntacticElementContext::forSingleValueStmtExpr(SVE); - } else if (auto *EP = getAsPattern(anchor)) { - context = SyntacticElementContext::forExprPattern(EP); } else { return SolutionKind::Error; } diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index e20d93be43897..b5c2b8ed09ee6 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -573,19 +573,14 @@ func testMultiStmtClosureExprPattern(_ x: Int) { if case { (); return x }() = x {} } -func testExprPatternIsolation() { - // We type-check ExprPatterns separately, so these are illegal. - if case 0 = nil {} // expected-error {{'nil' requires a contextual type}} +func testExprPatternInference() { + if case 0 = nil {} let _ = { - if case 0 = nil {} // expected-error {{'nil' requires a contextual type}} + if case 0 = nil {} } for case 0 in nil {} // expected-error {{'nil' requires a contextual type}} for case 0 in [nil] {} - // expected-error@-1 {{type 'Any' cannot conform to 'Equatable'}} - // expected-note@-2 {{only concrete types such as structs, enums and classes can conform to protocols}} - // expected-note@-3 {{requirement from conditional conformance of 'Any?' to 'Equatable'}} - // Though we will try Double for an integer literal... let d: Double = 0 if case d = 0 {} let _ = { @@ -593,13 +588,12 @@ func testExprPatternIsolation() { } for case d in [0] {} - // But not Float let f: Float = 0 - if case f = 0 {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + if case f = 0 {} let _ = { - if case f = 0 {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + if case f = 0 {} } - for case f in [0] {} // expected-error {{expression pattern of type 'Float' cannot match values of type 'Int'}} + for case f in [0] {} enum MultiPayload: Equatable { case e(T, T) @@ -613,33 +607,24 @@ func testExprPatternIsolation() { func produceMultiPayload() -> MultiPayload { fatalError() } - // We type-check ExprPatterns left to right, so only one of these works. if case .e(0.0, 0) = produceMultiPayload() {} - if case .e(0, 0.0) = produceMultiPayload() {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + if case .e(0, 0.0) = produceMultiPayload() {} for case .e(0.0, 0) in [produceMultiPayload()] {} - for case .e(0, 0.0) in [produceMultiPayload()] {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + for case .e(0, 0.0) in [produceMultiPayload()] {} - // Same, because although it's a top-level ExprPattern, we don't resolve - // that until during solving. if case .f(0.0, 0) = produceMultiPayload() {} - if case .f(0, 0.0) = produceMultiPayload() {} // expected-error {{expression pattern of type 'Double' cannot match values of type 'Int'}} + if case .f(0, 0.0) = produceMultiPayload() {} - if case .e(5, nil) = produceMultiPayload() {} // expected-warning {{type 'Int' is not optional, value can never be nil; this is an error in Swift 6}} - - // FIXME: Bad error (https://github.com/apple/swift/issues/64279) + if case .e(5, nil) = produceMultiPayload() {} if case .e(nil, 0) = produceMultiPayload() {} - // expected-error@-1 {{expression pattern of type 'String' cannot match values of type 'Substring'}} - // expected-note@-2 {{overloads for '~=' exist with these partially matching parameter lists}} if case .e(5, nil) = produceMultiPayload() as MultiPayload {} if case .e(nil, 0) = produceMultiPayload() as MultiPayload {} - // Enum patterns are solved together. if case .e(E.a, .b) = produceMultiPayload() {} if case .e(.a, E.b) = produceMultiPayload() {} - // These also work because they start life as EnumPatterns. if case .e(E.c, .d) = produceMultiPayload() {} if case .e(.c, E.d) = produceMultiPayload() {} for case .e(E.c, .d) in [produceMultiPayload()] {} diff --git a/test/Misc/misc_diagnostics.swift b/test/Misc/misc_diagnostics.swift index 6d27d411adcc5..91648a6e55c9c 100644 --- a/test/Misc/misc_diagnostics.swift +++ b/test/Misc/misc_diagnostics.swift @@ -145,10 +145,11 @@ func test20770032() { if case let 1...10 = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} // expected-error@-1 {{expression pattern of type 'ClosedRange' cannot match values of type '(Int, Int)'}} } + if case let (1...10) = (1, 1) { // expected-warning{{'let' pattern has no effect; sub-pattern didn't bind any variables}} {{11-15=}} + // expected-error@-1 {{expression pattern of type 'ClosedRange' cannot match values of type '(Int, Int)'}} + } } - - func tuple_splat1(_ a : Int, _ b : Int) { // expected-note 2 {{'tuple_splat1' declared here}} let x = (1,2) tuple_splat1(x) // expected-error {{global function 'tuple_splat1' expects 2 separate arguments}} diff --git a/test/stdlib/StringDiagnostics.swift b/test/stdlib/StringDiagnostics.swift index 3a53a26c69ee0..3072983562ef7 100644 --- a/test/stdlib/StringDiagnostics.swift +++ b/test/stdlib/StringDiagnostics.swift @@ -45,9 +45,9 @@ func testAmbiguousStringComparisons(s: String) { let a10 = nsString <= s // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{21-21= as String}} let a11 = nsString >= s // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{21-21= as String}} let a12 = nsString > s // expected-error{{'NSString' is not implicitly convertible to 'String'; did you mean to use 'as' to explicitly convert?}} {{21-21= as String}} - - // Shouldn't suggest 'as' in a pattern-matching context, as opposed to all these other situations - if case nsString = "" {} // expected-error{{expression pattern of type 'NSString' cannot match values of type 'String'}} + + // Okay, we can infer the string literal as NSString. + if case nsString = "" {} } func testStringDeprecation(hello: String) { diff --git a/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift b/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift index fd6652a85f4e4..63d4ccca5e205 100644 --- a/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift +++ b/validation-test/Sema/type_checker_crashers_fixed/issue-65360.swift @@ -9,13 +9,13 @@ let _: () -> Void = { let _: () -> Void = { for case (0)? in [a] {} - // expected-error@-1 {{pattern cannot match values of type 'Any?'}} + // expected-error@-1 {{cannot convert sequence element type 'Any?' to expected type '(Int)?'}} if case (0, 0) = a {} } let _: () -> Void = { for case (0)? in [a] {} - // expected-error@-1 {{pattern cannot match values of type 'Any?'}} + // expected-error@-1 {{cannot convert sequence element type 'Any?' to expected type '(Int)?'}} for case (0, 0) in [a] {} }