diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index 8acbf7fde33..3ea4b0b4c3a 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -2228,48 +2228,6 @@ static bool tryRawRepresentableFixIts(InFlightDiagnostic &diag, return false; } -/// Try to add a fix-it when converting between a collection and its slice type, -/// such as String <-> Substring or (eventually) Array <-> ArraySlice -static bool trySequenceSubsequenceConversionFixIts(InFlightDiagnostic &diag, - ConstraintSystem &CS, - Type fromType, Type toType, - Expr *expr) { - if (CS.TC.Context.getStdlibModule() == nullptr) - return false; - - auto String = CS.TC.getStringType(CS.DC); - auto Substring = CS.TC.getSubstringType(CS.DC); - - if (!String || !Substring) - return false; - - /// FIXME: Remove this flag when void subscripts are implemented. - /// Make this unconditional and remove the if statement. - if (CS.TC.getLangOpts().FixStringToSubstringConversions) { - // String -> Substring conversion - // Add '[]' void subscript call to turn the whole String into a Substring - if (fromType->isEqual(String)) { - if (toType->isEqual(Substring)) { - diag.fixItInsertAfter(expr->getEndLoc (), "[]"); - return true; - } - } - } - - // Substring -> String conversion - // Wrap in String.init - if (fromType->isEqual(Substring)) { - if (toType->isEqual(String)) { - auto range = expr->getSourceRange(); - diag.fixItInsert(range.Start, "String("); - diag.fixItInsertAfter(range.End, ")"); - return true; - } - } - - return false; -} - /// Attempts to add fix-its for these two mistakes: /// /// - Passing an integer with the right type but which is getting wrapped with a @@ -2738,10 +2696,9 @@ bool FailureDiagnosis::diagnoseContextualConversionError( // Try to convert between a sequence and its subsequence, notably // String <-> Substring. - if (trySequenceSubsequenceConversionFixIts(diag, CS, exprType, contextualType, - expr)) { + if (ContextualFailure::trySequenceSubsequenceFixIts(diag, CS, exprType, + contextualType, expr)) return true; - } // Attempt to add a fixit for the error. switch (CTP) { diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index be248a93281..5e0aa0cbf59 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -1106,3 +1106,90 @@ Diag AssignmentFailure::findDeclDiagonstic(ASTContext &ctx, return diag::assignment_lhs_is_immutable_variable; } + +bool ContextualFailure::diagnoseAsError() { + auto *anchor = getAnchor(); + auto path = getLocator()->getPath(); + + assert(!path.empty()); + + if (diagnoseMissingFunctionCall()) + return true; + + Diag diagnostic; + switch (path.back().getKind()) { + case ConstraintLocator::ClosureResult: { + diagnostic = diag::cannot_convert_closure_result; + break; + } + + default: + return false; + } + + auto diag = emitDiagnostic(anchor->getLoc(), diagnostic, FromType, ToType); + diag.highlight(anchor->getSourceRange()); + + (void)trySequenceSubsequenceFixIts(diag, getConstraintSystem(), FromType, + ToType, anchor); + return true; +} + +bool ContextualFailure::diagnoseMissingFunctionCall() const { + auto &TC = getTypeChecker(); + + auto *srcFT = FromType->getAs(); + if (!srcFT || !srcFT->getParams().empty()) + return false; + + if (ToType->is() || + !TC.isConvertibleTo(srcFT->getResult(), ToType, getDC())) + return false; + + auto *anchor = getAnchor(); + emitDiagnostic(anchor->getLoc(), diag::missing_nullary_call, + srcFT->getResult()) + .highlight(anchor->getSourceRange()) + .fixItInsertAfter(anchor->getEndLoc(), "()"); + return true; +} + +bool ContextualFailure::trySequenceSubsequenceFixIts(InFlightDiagnostic &diag, + ConstraintSystem &CS, + Type fromType, Type toType, + Expr *expr) { + if (!CS.TC.Context.getStdlibModule()) + return false; + + auto String = CS.TC.getStringType(CS.DC); + auto Substring = CS.TC.getSubstringType(CS.DC); + + if (!String || !Substring) + return false; + + /// FIXME: Remove this flag when void subscripts are implemented. + /// Make this unconditional and remove the if statement. + if (CS.TC.getLangOpts().FixStringToSubstringConversions) { + // String -> Substring conversion + // Add '[]' void subscript call to turn the whole String into a Substring + if (fromType->isEqual(String)) { + if (toType->isEqual(Substring)) { + diag.fixItInsertAfter(expr->getEndLoc(), "[]"); + return true; + } + } + } + + // Substring -> String conversion + // Wrap in String.init + if (fromType->isEqual(Substring)) { + if (toType->isEqual(String)) { + auto range = expr->getSourceRange(); + diag.fixItInsert(range.Start, "String("); + diag.fixItInsertAfter(range.End, ")"); + return true; + } + } + + return false; +} diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index eb8e751d803..b14b31e93a5 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -558,6 +558,40 @@ private: } }; +/// Intended to diagnose any possible contextual failure +/// e.g. argument/parameter, closure result, conversions etc. +class ContextualFailure final : public FailureDiagnostic { + Type FromType, ToType; + +public: + ContextualFailure(Expr *root, ConstraintSystem &cs, Type lhs, Type rhs, + ConstraintLocator *locator) + : FailureDiagnostic(root, cs, locator), FromType(resolve(lhs)), + ToType(resolve(rhs)) {} + + bool diagnoseAsError() override; + + // If we're trying to convert something of type "() -> T" to T, + // then we probably meant to call the value. + bool diagnoseMissingFunctionCall() const; + + /// Try to add a fix-it when converting between a collection and its slice + /// type, such as String <-> Substring or (eventually) Array <-> ArraySlice + static bool trySequenceSubsequenceFixIts(InFlightDiagnostic &diag, + ConstraintSystem &CS, Type fromType, + Type toType, Expr *expr); + +private: + Type resolve(Type rawType) { + auto type = resolveType(rawType)->getWithoutSpecifierType(); + if (auto *BGT = type->getAs()) { + if (BGT->hasUnresolvedType()) + return BGT->getDecl()->getDeclaredInterfaceType(); + } + return type; + } +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 311265eadb9..3772bebec61 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -190,3 +190,15 @@ SkipSuperclassRequirement::create(ConstraintSystem &cs, Type lhs, Type rhs, return new (cs.getAllocator()) SkipSuperclassRequirement(cs, lhs, rhs, locator); } + +bool ContextualMismatch::diagnose(Expr *root, bool asNote) const { + auto failure = ContextualFailure(root, getConstraintSystem(), getFromType(), + getToType(), getLocator()); + return failure.diagnose(asNote); +} + +ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs, + Type rhs, + ConstraintLocator *locator) { + return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator); +} diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h index d90feb50ac5..3033e5410f0 100644 --- a/lib/Sema/CSFix.h +++ b/lib/Sema/CSFix.h @@ -80,6 +80,9 @@ enum class FixKind : uint8_t { /// and assume that types are related. SkipSuperclassRequirement, + /// Fix up one of the sides of conversion to make it seem + /// like the types are aligned. + ContextualMismatch, }; class ConstraintFix { @@ -347,6 +350,35 @@ public: create(ConstraintSystem &cs, Type lhs, Type rhs, ConstraintLocator *locator); }; +/// For example: Sometimes type returned from the body of the +/// closure doesn't match expected contextual type: +/// +/// func foo(_: () -> Int) {} +/// foo { "ultimate question" } +/// +/// Body of the closure produces `String` type when `Int` is expected +/// by the context. +class ContextualMismatch : public ConstraintFix { + Type LHS, RHS; + +protected: + ContextualMismatch(ConstraintSystem &cs, Type lhs, Type rhs, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::ContextualMismatch, locator), LHS(lhs), + RHS(rhs) {} + +public: + std::string getName() const override { return "fix contextual mismatch"; } + + Type getFromType() const { return LHS; } + Type getToType() const { return RHS; } + + bool diagnose(Expr *root, bool asNote = false) const override; + + static ContextualMismatch *create(ConstraintSystem &cs, Type lhs, Type rhs, + ConstraintLocator *locator); +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 0f1eb562ab2..f83c5dc4d84 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1573,21 +1573,42 @@ ConstraintSystem::matchTypesBindTypeVar( return getTypeMatchSuccess(); } -static void attemptToFixRequirementFailure( - ConstraintSystem &cs, Type type1, Type type2, - SmallVectorImpl &conversionsOrFixes, - ConstraintLocatorBuilder locator) { - using LocatorPathEltKind = ConstraintLocator::PathElementKind; - +static ConstraintFix *fixRequirementFailure(ConstraintSystem &cs, Type type1, + Type type2, Expr *anchor, + LocatorPathElt &req) { // Can't fix not yet properly resolved types. if (type1->hasTypeVariable() || type2->hasTypeVariable()) - return; + return nullptr; // If dependent members are present here it's because // base doesn't conform to associated type's protocol. if (type1->hasDependentMember() || type2->hasDependentMember()) - return; + return nullptr; + // Build simplified locator which only contains anchor and requirement info. + ConstraintLocatorBuilder requirement(cs.getConstraintLocator(anchor)); + auto *reqLoc = cs.getConstraintLocator(requirement.withPathElement(req)); + + auto reqKind = static_cast(req.getValue2()); + switch (reqKind) { + case RequirementKind::SameType: { + return SkipSameTypeRequirement::create(cs, type1, type2, reqLoc); + } + + case RequirementKind::Superclass: { + return SkipSuperclassRequirement::create(cs, type1, type2, reqLoc); + } + + case RequirementKind::Conformance: + case RequirementKind::Layout: + llvm_unreachable("conformance requirements are handled elsewhere"); + } +} + +static void +repairFailures(ConstraintSystem &cs, Type lhs, Type rhs, + SmallVectorImpl &conversionsOrFixes, + ConstraintLocatorBuilder locator) { SmallVector path; auto *anchor = locator.getLocatorParts(path); @@ -1595,30 +1616,22 @@ static void attemptToFixRequirementFailure( return; auto &elt = path.back(); - if (elt.getKind() != LocatorPathEltKind::TypeParameterRequirement) - return; - - // Build simplified locator which only contains anchor and requirement info. - ConstraintLocatorBuilder requirement(cs.getConstraintLocator(anchor)); - auto *reqLoc = cs.getConstraintLocator(requirement.withPathElement(elt)); - - auto reqKind = static_cast(elt.getValue2()); - switch (reqKind) { - case RequirementKind::SameType: { - auto *fix = SkipSameTypeRequirement::create(cs, type1, type2, reqLoc); - conversionsOrFixes.push_back(fix); - return; + switch (elt.getKind()) { + case ConstraintLocator::TypeParameterRequirement: { + if (auto *fix = fixRequirementFailure(cs, lhs, rhs, anchor, elt)) + conversionsOrFixes.push_back(fix); + break; } - case RequirementKind::Superclass: { - auto *fix = SkipSuperclassRequirement::create(cs, type1, type2, reqLoc); + case ConstraintLocator::ClosureResult: { + auto *fix = ContextualMismatch::create(cs, lhs, rhs, + cs.getConstraintLocator(locator)); conversionsOrFixes.push_back(fix); - return; + break; } - case RequirementKind::Conformance: - case RequirementKind::Layout: - llvm_unreachable("conformance requirements are handled elsewhere"); + default: + return; } } @@ -2407,9 +2420,9 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind, TreatRValueAsLValue::create(*this, getConstraintLocator(locator))); } + // Attempt to repair any failures identifiable at this point. if (attemptFixes) - attemptToFixRequirementFailure(*this, type1, type2, conversionsOrFixes, - locator); + repairFailures(*this, type1, type2, conversionsOrFixes, locator); if (conversionsOrFixes.empty()) { // If one of the types is a type variable or member thereof, we leave this @@ -5047,7 +5060,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( } case FixKind::SkipSameTypeRequirement: - case FixKind::SkipSuperclassRequirement: { + case FixKind::SkipSuperclassRequirement: + case FixKind::ContextualMismatch: { return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved; } diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift index 5c7da2e8e5f..263c20cea46 100644 --- a/test/Constraints/closures.swift +++ b/test/Constraints/closures.swift @@ -805,3 +805,29 @@ func rdar_45659733() { _ = (a ..< b).map { i in foo(i, i) } // Ok } } + + +// rdar://problem/40537960 - Misleading diagnostic when using closure with wrong type + +protocol P_40537960 {} +func rdar_40537960() { + struct S { + var v: String + } + + struct L : P_40537960 { + init(_: String) {} + } + + struct R { + init(_: P_40537960) {} + } + + struct A { + typealias Data = T.Element + init(_: T, fn: (Data) -> R

) {} + } + + var arr: [S] = [] + _ = A(arr, fn: { L($0.v) }) // expected-error {{cannot convert value of type 'L' to closure result type 'R'}} +} diff --git a/test/Sema/string_to_substring_conversion.swift b/test/Sema/string_to_substring_conversion.swift index e0e66ef9b9a..99bf3bc3021 100644 --- a/test/Sema/string_to_substring_conversion.swift +++ b/test/Sema/string_to_substring_conversion.swift @@ -5,7 +5,7 @@ let ss = s[s.startIndex.. (Int) -> (Int,Int) = {{ (4, 2) }} // expected-error var closure4 : (Int,Int) -> Int = { $0 + $1 } var closure5 : (Double) -> Int = { $0 + 1.0 - // expected-error@+1 {{cannot convert value of type 'Double' to closure result type 'Int'}} + // expected-error@-1 {{cannot convert value of type 'Double' to closure result type 'Int'}} } var closure6 = $0 // expected-error {{anonymous closure argument not contained in a closure}} diff --git a/validation-test/compiler_crashers_2_fixed/0148-rdar35773761.swift b/validation-test/compiler_crashers_2_fixed/0148-rdar35773761.swift index 55b3a8c738d..5844463d8e5 100644 --- a/validation-test/compiler_crashers_2_fixed/0148-rdar35773761.swift +++ b/validation-test/compiler_crashers_2_fixed/0148-rdar35773761.swift @@ -1,4 +1,4 @@ // RUN: %target-typecheck-verify-swift let b: () -> Void = withoutActuallyEscaping({ print("hello crash") }, do: { $0() }) -// expected-error@-1 {{cannot convert value of type '()' to specified type '() -> Void'}} +// expected-error@-1 {{cannot convert value of type '()' to closure result type '() -> Void'}}