From c7c95103663543081a1aafebc937dba766b5a59c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 24 Jan 2020 17:07:43 -0800 Subject: [PATCH 1/3] [ConstraintSystem] Don't re-attempt to bind type variable if it could be a hole If type variable is allowed to be a hole and it can't be bound to this particular (full resolved) type, just ignore this binding instead of re-trying it later. --- lib/Sema/CSSimplify.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 4568823340ff5..2794e9650a104 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -2249,8 +2249,17 @@ ConstraintSystem::matchTypesBindTypeVar( // Simplify the right-hand type and perform the "occurs" check. typeVar = getRepresentative(typeVar); type = simplifyType(type, flags); - if (!isBindable(typeVar, type)) + if (!isBindable(typeVar, type)) { + if (shouldAttemptFixes()) { + // If type variable is allowed to be a hole and it can't be bound to + // a particular (full resolved) type, just ignore this binding + // instead of re-trying it and failing later. + if (typeVar->getImpl().canBindToHole() && !type->hasTypeVariable()) + return getTypeMatchSuccess(); + } + return formUnsolvedResult(); + } // Since member lookup doesn't check requirements // it might sometimes return types which are not From dea79e09c6d54a4e8a05fa92083538912a39ab08 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 27 Jan 2020 15:27:45 -0800 Subject: [PATCH 2/3] [ConstraintSystem] Detect that function type has failures before applying arguments If a type variable representing "function type" is a hole or it could be bound to some concrete type with a help of a fix, let's propagate holes to the "input" type. Doing so provides more information to upcoming argument and result matching. --- lib/Sema/CSSimplify.cpp | 31 ++++++++++++++++++++++--------- lib/Sema/ConstraintSystem.h | 1 + test/Constraints/bridging.swift | 2 +- test/expr/expressions.swift | 6 ++---- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 2794e9650a104..c89e42065b2f5 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -7518,6 +7518,18 @@ ConstraintSystem::simplifyApplicableFnConstraint( // following: $T1 -> $T2. auto func1 = type1->castTo(); + // If a type variable representing "function type" is a hole + // or it could be bound to some concrete type with a help of + // a fix, let's propagate holes to the "input" type. Doing so + // provides more information to upcoming argument and result matching. + if (shouldAttemptFixes()) { + if (auto *typeVar = type2->getAs()) { + auto *locator = typeVar->getImpl().getLocator(); + if (typeVar->isHole() || hasFixFor(locator)) + recordPotentialHole(func1); + } + } + // Before stripping lvalue-ness and optional types, save the original second // type for handling `func callAsFunction` and `@dynamicCallable` // applications. This supports the following cases: @@ -7730,10 +7742,7 @@ ConstraintSystem::simplifyApplicableFnConstraint( // If there are any type variables associated with arguments/result // they have to be marked as "holes". - type1.visit([&](Type subType) { - if (auto *typeVar = subType->getAs()) - recordPotentialHole(typeVar); - }); + recordPotentialHole(func1); if (desugar2->isHole()) return SolutionKind::Solved; @@ -8007,11 +8016,7 @@ ConstraintSystem::simplifyDynamicCallableApplicableFnConstraint( return SolutionKind::Error; recordPotentialHole(tv); - - Type(func1).visit([&](Type type) { - if (auto *typeVar = type->getAs()) - recordPotentialHole(typeVar); - }); + recordPotentialHole(func1); return SolutionKind::Solved; } @@ -8572,6 +8577,14 @@ void ConstraintSystem::recordPotentialHole(TypeVariableType *typeVar) { typeVar->getImpl().enableCanBindToHole(getSavedBindings()); } +void ConstraintSystem::recordPotentialHole(FunctionType *fnType) { + assert(fnType); + Type(fnType).visit([&](Type type) { + if (auto *typeVar = type->getAs()) + recordPotentialHole(typeVar); + }); +} + ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( ConstraintFix *fix, Type type1, Type type2, ConstraintKind matchKind, TypeMatchOptions flags, ConstraintLocatorBuilder locator) { diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 1cd0c4724e7a4..028de0c16559c 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -2361,6 +2361,7 @@ class ConstraintSystem { bool recordFix(ConstraintFix *fix, unsigned impact = 1); void recordPotentialHole(TypeVariableType *typeVar); + void recordPotentialHole(FunctionType *fnType); /// Determine whether constraint system already has a fix recorded /// for a particular location. diff --git a/test/Constraints/bridging.swift b/test/Constraints/bridging.swift index bafa1dab76723..4cdc52e3e2626 100644 --- a/test/Constraints/bridging.swift +++ b/test/Constraints/bridging.swift @@ -265,7 +265,7 @@ func rdar19831698() { var v71 = true + 1.0 // expected-error{{binary operator '+' cannot be applied to operands of type 'Bool' and 'Double'}} // expected-note@-1{{overloads for '+'}} var v72 = true + true // expected-error{{binary operator '+' cannot be applied to two 'Bool' operands}} - var v73 = true + [] // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'Array'}} + var v73 = true + [] // expected-error@:18 {{binary operator '+' cannot be applied to operands of type 'Bool' and '[Any]'}} var v75 = true + "str" // expected-error@:13 {{cannot convert value of type 'Bool' to expected argument type 'String'}} } diff --git a/test/expr/expressions.swift b/test/expr/expressions.swift index 43af91bfcfebb..e63fe024f7fa4 100644 --- a/test/expr/expressions.swift +++ b/test/expr/expressions.swift @@ -748,10 +748,8 @@ func invalidDictionaryLiteral() { } -[4].joined(separator: [1]) // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}} -// expected-error@-1 {{cannot convert value of type '[Int]' to expected argument type 'String'}} -[4].joined(separator: [[[1]]]) // expected-error {{cannot convert value of type 'Int' to expected element type 'String'}} -// expected-error@-1 {{cannot convert value of type '[[[Int]]]' to expected argument type 'String'}} +[4].joined(separator: [1]) // expected-error {{referencing instance method 'joined(separator:)' on 'Sequence' requires that 'Int' conform to 'Sequence'}} +[4].joined(separator: [[[1]]]) // expected-error {{referencing instance method 'joined(separator:)' on 'Sequence' requires that 'Int' conform to 'Sequence'}} //===----------------------------------------------------------------------===// // nil/metatype comparisons From e50756c10c51e539b8af1b08a51c62bf06608fe8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 27 Jan 2020 17:17:27 -0800 Subject: [PATCH 3/3] [CSDiag] Obsolete function re-typecheck and diagnostics from `visitApplyExpr` --- lib/Sema/CSDiag.cpp | 93 ++-------------------------- test/Constraints/members.swift | 6 +- test/type/protocol_composition.swift | 4 +- 3 files changed, 11 insertions(+), 92 deletions(-) diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index 6860d71f77ef7..3558188f6e2dd 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -1202,32 +1202,6 @@ static Expr *getFailedArgumentExpr(CalleeCandidateInfo CCI, Expr *argExpr) { bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI, Expr *fnExpr, Expr *argExpr, ArrayRef argLabels) { - if (auto *MTT = CS.getType(fnExpr)->getAs()) { - auto instTy = MTT->getInstanceType(); - auto &DE = CS.getASTContext().Diags; - if (instTy->getAnyNominal()) { - // If we are invoking a constructor on a nominal type and there are - // absolutely no candidates, then they must all be private. - if (CCI.empty() || (CCI.size() == 1 && CCI.candidates[0].getDecl() && - isa(CCI.candidates[0].getDecl()))) { - DE.diagnose(fnExpr->getLoc(), diag::no_accessible_initializers, - instTy); - return true; - } - // continue below - } else if (!instTy->is()) { - // If we are invoking a constructor on a non-nominal type, the expression - // is malformed. - SourceRange initExprRange(fnExpr->getSourceRange().Start, - argExpr->getSourceRange().End); - DE.diagnose(fnExpr->getLoc(), instTy->isExistentialType() ? - diag::construct_protocol_by_name : - diag::non_nominal_no_initializers, instTy) - .highlight(initExprRange); - return true; - } - } - // Try to diagnose errors related to the use of implicit self reference. if (diagnoseImplicitSelfErrors(fnExpr, argExpr, CCI, argLabels)) return true; @@ -1268,59 +1242,9 @@ bool FailureDiagnosis::diagnoseParameterErrors(CalleeCandidateInfo &CCI, return false; } -// Check if there is a structural problem in the function expression -// by performing type checking with the option to allow unresolved -// type variables. If that is going to produce a function type with -// unresolved result let's not re-typecheck the function expression, -// because it might produce unrelated diagnostics due to lack of -// contextual information. -static bool shouldTypeCheckFunctionExpr(FailureDiagnosis &FD, DeclContext *DC, - Expr *fnExpr) { - if (!isa(fnExpr)) - return true; - - SmallPtrSet fnTypes; - FD.getPossibleTypesOfExpressionWithoutApplying( - fnExpr, DC, fnTypes, FreeTypeVariableBinding::UnresolvedType); - - if (fnTypes.size() == 1) { - // Some member types depend on the arguments to produce a result type, - // type-checking such expressions without associated arguments is - // going to produce unrelated diagnostics. - if (auto fn = (*fnTypes.begin())->getAs()) { - auto resultType = fn->getResult(); - if (resultType->hasUnresolvedType() || resultType->hasTypeVariable()) - return false; - } - } - - // Might be a structural problem related to the member itself. - return true; -} - bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) { auto *fnExpr = callExpr->getFn(); - - if (shouldTypeCheckFunctionExpr(*this, CS.DC, fnExpr)) { - // Type check the function subexpression to resolve a type for it if - // possible. - fnExpr = typeCheckChildIndependently(callExpr->getFn()); - if (!fnExpr) { - return CS.getASTContext().Diags.hadAnyError(); - } - } - - SWIFT_DEFER { - if (!fnExpr) return; - - // If it's a member operator reference, put the operator back. - if (auto operatorRef = fnExpr->getMemberOperatorRef()) - callExpr->setFn(operatorRef); - }; - - auto getFuncType = [](Type type) -> Type { return type->getRValueType(); }; - - auto fnType = getFuncType(CS.getType(fnExpr)); + auto fnType = CS.getType(fnExpr)->getRValueType(); bool hasTrailingClosure = callArgHasTrailingClosure(callExpr->getArg()); @@ -1489,17 +1413,6 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) { if (CS.getType(argExpr)->hasUnresolvedType()) return false; - SmallVector params; - AnyFunctionType::decomposeInput(CS.getType(argExpr), params); - auto argString = AnyFunctionType::getParamListAsString(params); - - if (auto MTT = fnType->getAs()) { - if (MTT->getInstanceType()->isExistentialType()) { - diagnose(fnExpr->getLoc(), diag::construct_protocol_value, fnType); - return true; - } - } - bool isInitializer = isa(fnExpr); if (isa(argExpr) && cast(argExpr)->getNumElements() == 0) { @@ -1507,6 +1420,10 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) { diagnose(fnExpr->getLoc(), diag::cannot_call_with_no_params, overloadName, isInitializer); } else { + SmallVector params; + AnyFunctionType::decomposeInput(CS.getType(argExpr), params); + auto argString = AnyFunctionType::getParamListAsString(params); + diagnose(fnExpr->getLoc(), diag::cannot_call_with_params, overloadName, argString, isInitializer); } diff --git a/test/Constraints/members.swift b/test/Constraints/members.swift index 7a39dd010c393..c2de930b8b443 100644 --- a/test/Constraints/members.swift +++ b/test/Constraints/members.swift @@ -405,17 +405,17 @@ func bar_32854314() -> Int { extension Array where Element == Int { func foo() { let _ = min(foo_32854314(), bar_32854314()) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}} - // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}} + // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}} } func foo(_ x: Int, _ y: Double) { let _ = min(x, y) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}} - // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}} + // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}} } func bar() { let _ = min(1.0, 2) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}} - // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min()'}} + // expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}} } } diff --git a/test/type/protocol_composition.swift b/test/type/protocol_composition.swift index b78a9af3a5fd0..a75b0414b5aa5 100644 --- a/test/type/protocol_composition.swift +++ b/test/type/protocol_composition.swift @@ -173,7 +173,9 @@ takesP1AndP2([Swift.AnyObject & P1 & P2]()) takesP1AndP2([AnyObject & protocol_composition.P1 & P2]()) takesP1AndP2([AnyObject & P1 & protocol_composition.P2]()) takesP1AndP2([DoesNotExist & P1 & P2]()) // expected-error {{use of unresolved identifier 'DoesNotExist'}} -takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}} +// TODO(diagnostics): The problem here is that `&` is interpreted as a binary operator, we need to re-think +// how "missing member" fix is implemented because currently it finds N solutions with multiple fixes. +takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{cannot invoke '' with no arguments}} typealias T08 = P1 & inout P2 // expected-error {{'inout' may only be used on parameters}} typealias T09 = P1 & __shared P2 // expected-error {{'__shared' may only be used on parameters}}