From 607c2984939dab5395ded1dcf41ede12de261d66 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 6 Feb 2020 12:16:28 -0800 Subject: [PATCH 01/13] [Diagnostics] Refactor diagnoseAmbigutiyWithFixes to use the differences between solutions in order to figure out the source of ambiguity. --- lib/Sema/ConstraintSystem.cpp | 167 +++++++++------------ lib/Sema/ConstraintSystem.h | 14 +- test/Constraints/closures.swift | 2 +- test/Constraints/one_way_constraints.swift | 2 +- 4 files changed, 85 insertions(+), 100 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 531f51cdb9916..99dd5898f6266 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2749,13 +2749,66 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( return false; } - // Problems related to fixes forming ambiguous solution set - // could only be diagnosed (at the moment), if all of the fixes - // have the same callee locator, which means they fix different - // overloads of the same declaration. - ConstraintLocator *commonCalleeLocator = nullptr; + // Collect aggregated fixes from all solutions + llvm::SmallMapVector, + llvm::SmallVector, 4> aggregatedFixes; + for (const auto &solution: solutions) { + for (auto fixesPerLocator: solution.getAggregatedFixes()) { + for (auto kindAndFixes: fixesPerLocator.second) { + aggregatedFixes[{fixesPerLocator.first, kindAndFixes.first}] + .push_back(kindAndFixes.second.front()); + } + } + } + + // If there are no overload differences, diagnose the common fixes. + SolutionDiff solutionDiff(solutions); + if (solutionDiff.overloads.size() == 0) { + bool diagnosed = false; + ConstraintSystem::SolverScope scope(*this); + applySolution(solutions.front()); + for (auto fixes: aggregatedFixes) { + // A common fix must appear in all solutions + if (fixes.second.size() < solutions.size()) continue; + diagnosed |= fixes.second.front()->diagnose(); + } + return diagnosed; + } + + // If there is an overload difference, let's see if there's a common callee + // locator for all of the fixes. + auto ambiguousOverload = llvm::find_if(solutionDiff.overloads, + [&](const auto &overloadDiff) { + return llvm::all_of(aggregatedFixes, [&](const auto &aggregatedFix) { + auto *locator = aggregatedFix.first.first; + auto *anchor = locator->getAnchor(); + // Assignment failures are all about the source expression, because + // they treat destination as a contextual type. + if (auto *assignExpr = dyn_cast(anchor)) + anchor = assignExpr->getSrc(); + + if (auto *callExpr = dyn_cast(anchor)) { + if (!isa(callExpr->getDirectCallee())) + anchor = callExpr->getDirectCallee(); + } else if (auto *applyExpr = dyn_cast(anchor)) { + anchor = applyExpr->getFn(); + } + + return overloadDiff.locator->getAnchor() == anchor; + }); + }); + + if (ambiguousOverload == solutionDiff.overloads.end()) + return false; + + ConstraintLocator* commonCalleeLocator = ambiguousOverload->locator; SmallPtrSet distinctChoices; SmallVector viableSolutions; + assert(ambiguousOverload->choices.size() == solutions.size()); + for (unsigned i = 0; i < solutions.size(); ++i) { + if (distinctChoices.insert(ambiguousOverload->choices[i].getDecl()).second) + viableSolutions.push_back(&solutions[i]); + } enum class AmbiguityKind { /// There is exactly one fix associated with each candidate. @@ -2771,103 +2824,23 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( }; auto ambiguityKind = AmbiguityKind::CloseMatch; - bool diagnosable = llvm::all_of(solutions, [&](const Solution &solution) { - ArrayRef fixes = solution.Fixes; - - if (fixes.empty()) + // FIXME: Can this be re-written in a cleaner way? + for (const auto &solution: solutions) { + if (solution.Fixes.empty()) return false; - if (fixes.size() > 1) { - // Attempt to disambiguite in cases where the argument matches - // involves tuple mismatches. e.g. - // func t(_: (T, U), _: (U, T)) {} - // func useTuples(_ x: Int, y: Float) { - // t((x, y), (x, y)) - // } - // So fixes are ambiguous in all solutions. - if ((ambiguityKind == AmbiguityKind::CloseMatch || - ambiguityKind == AmbiguityKind::ArgumentMismatch) && - llvm::all_of(fixes, [](const ConstraintFix *fix) -> bool { - return fix->getKind() == FixKind::AllowTupleTypeMismatch; - })) { - ambiguityKind = AmbiguityKind::ArgumentMismatch; - } else { - ambiguityKind = - (ambiguityKind == AmbiguityKind::CloseMatch || - ambiguityKind == AmbiguityKind::ArgumentMismatch || - ambiguityKind == AmbiguityKind::ParameterList) && - llvm::all_of( - fixes, - [](const ConstraintFix *fix) -> bool { - auto *locator = fix->getLocator(); - return locator - ->findLast() - .hasValue(); - }) - ? AmbiguityKind::ParameterList - : AmbiguityKind::General; - } + if (solution.Fixes.size() > 1) { + ambiguityKind = (ambiguityKind == AmbiguityKind::CloseMatch || + ambiguityKind == AmbiguityKind::ParameterList) && + llvm::all_of(solution.Fixes, [](const ConstraintFix *fix) -> bool { + auto *locator = fix->getLocator(); + return locator->findLast().hasValue(); + }) ? AmbiguityKind::ParameterList + : AmbiguityKind::General; } - - if (fixes.size() == 1) { - // Attempt to disambiguite in cases where all the solutions - // produces the same fixes for different generic arguments e.g. - // func f(_: T, _: T) {} - // f(Int(1), Float(1)) - // - ambiguityKind = - ((ambiguityKind == AmbiguityKind::CloseMatch || - ambiguityKind == AmbiguityKind::ArgumentMismatch) && - fixes.front()->getKind() == FixKind::AllowArgumentTypeMismatch) - ? AmbiguityKind::ArgumentMismatch - : AmbiguityKind::CloseMatch; - } - - for (const auto *fix : fixes) { - auto *locator = fix->getLocator(); - // Assignment failures are all about the source expression, - // because they treat destination as a contextual type. - if (auto *anchor = locator->getAnchor()) { - if (auto *assignExpr = dyn_cast(anchor)) - locator = getConstraintLocator(assignExpr->getSrc()); - } - - auto *calleeLocator = solution.getCalleeLocator(locator); - if (!commonCalleeLocator) - commonCalleeLocator = calleeLocator; - else if (commonCalleeLocator != calleeLocator) - return false; - } - - auto overload = solution.getOverloadChoiceIfAvailable(commonCalleeLocator); - if (!overload) - return false; - - auto *decl = overload->choice.getDeclOrNull(); - if (!decl) - return false; - - // If this declaration is distinct, let's record this solution - // as viable, otherwise we'd produce the same diagnostic multiple - // times, which means that actual problem is elsewhere. - if (distinctChoices.insert(decl).second) - viableSolutions.push_back(&solution); - return true; - }); - - if (ambiguityKind == AmbiguityKind::ArgumentMismatch && - viableSolutions.size() == 1) { - // Let's apply the solution so the contextual generic types - // are available in the system for diagnostics. - applySolution(*viableSolutions[0]); - solutions.front().Fixes.front()->diagnose(/*asNote*/ false); - return true; } - if (!diagnosable || viableSolutions.size() < 2) - return false; - - auto *decl = *distinctChoices.begin(); + auto *decl = ambiguousOverload->choices.front().getDecl(); assert(solverState); bool diagnosed = true; diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index aa8e3ca3febd0..d4f4055c759f1 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -837,9 +837,21 @@ class Solution { llvm::DenseMap, ConversionRestrictionKind> ConstraintRestrictions; + using ConstraintFixVector = llvm::SmallVector; /// The list of fixes that need to be applied to the initial expression /// to make the solution work. - llvm::SmallVector Fixes; + ConstraintFixVector Fixes; + + using AggregatedFixesMap = llvm::SmallMapVector, 4>; + + AggregatedFixesMap getAggregatedFixes() const { + AggregatedFixesMap aggregatedFixes; + for (auto *fix: Fixes) { + aggregatedFixes[fix->getLocator()][fix->getKind()].push_back(fix); + } + return aggregatedFixes; + } /// The set of disjunction choices used to arrive at this solution, /// which informs constraint application. diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift index 42299524f07ed..876b5a1b76260 100644 --- a/test/Constraints/closures.swift +++ b/test/Constraints/closures.swift @@ -371,7 +371,7 @@ func someGeneric19997471(_ x: T) { func rdar21078316() { var foo : [String : String]? var bar : [(String, String)]? - bar = foo.map { ($0, $1) } // expected-error {{contextual closure type '(Dictionary) throws -> (Dictionary, _)' expects 1 argument, but 2 were used in closure body}} + bar = foo.map { ($0, $1) } // expected-error {{contextual closure type '([String : String]) throws -> [(String, String)]' expects 1 argument, but 2 were used in closure body}} } diff --git a/test/Constraints/one_way_constraints.swift b/test/Constraints/one_way_constraints.swift index d830367d35d78..4546880149afe 100644 --- a/test/Constraints/one_way_constraints.swift +++ b/test/Constraints/one_way_constraints.swift @@ -10,7 +10,7 @@ func testTernaryOneWay(b: Bool) { let _: Float = b ? 3.14159 : 17 // Errors due to one-way inference. - let _: Float = b ? Builtin.one_way(3.14159) // expected-error{{result values in '? :' expression have mismatching types 'Double' and 'Int'}} + let _: Float = b ? Builtin.one_way(3.14159) // expected-error{{cannot convert value of type 'Double' to specified type 'Float'}} : 17 let _: Float = b ? 3.14159 // expected-error {{cannot convert value of type 'Int' to specified type 'Float'}} : Builtin.one_way(17) From 54706ba79b84fbdf9c753e8ca08a62cb229d7ec6 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 6 Feb 2020 16:50:30 -0800 Subject: [PATCH 02/13] [Diagnostics] Simplify diagnoseAmbiguityWithFixes by removing AmbiguityKind. --- lib/Sema/ConstraintSystem.cpp | 192 ++++++++----------------- test/Constraints/diagnostics.swift | 2 +- test/decl/subscript/subscripting.swift | 4 +- 3 files changed, 64 insertions(+), 134 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 99dd5898f6266..ab019e79e3722 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2801,45 +2801,7 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( if (ambiguousOverload == solutionDiff.overloads.end()) return false; - ConstraintLocator* commonCalleeLocator = ambiguousOverload->locator; - SmallPtrSet distinctChoices; - SmallVector viableSolutions; - assert(ambiguousOverload->choices.size() == solutions.size()); - for (unsigned i = 0; i < solutions.size(); ++i) { - if (distinctChoices.insert(ambiguousOverload->choices[i].getDecl()).second) - viableSolutions.push_back(&solutions[i]); - } - - enum class AmbiguityKind { - /// There is exactly one fix associated with each candidate. - CloseMatch, - /// Solution is ambiguous because all candidates had partially matching - /// parameter lists. - ParameterList, - /// General ambiguity failure. - General, - /// Argument mismatch ambiguity where each solution has the same - /// argument mismatch fixes for the same call. - ArgumentMismatch - }; - auto ambiguityKind = AmbiguityKind::CloseMatch; - - // FIXME: Can this be re-written in a cleaner way? - for (const auto &solution: solutions) { - if (solution.Fixes.empty()) - return false; - - if (solution.Fixes.size() > 1) { - ambiguityKind = (ambiguityKind == AmbiguityKind::CloseMatch || - ambiguityKind == AmbiguityKind::ParameterList) && - llvm::all_of(solution.Fixes, [](const ConstraintFix *fix) -> bool { - auto *locator = fix->getLocator(); - return locator->findLast().hasValue(); - }) ? AmbiguityKind::ParameterList - : AmbiguityKind::General; - } - } - + auto *commonCalleeLocator = ambiguousOverload->locator; auto *decl = ambiguousOverload->choices.front().getDecl(); assert(solverState); @@ -2851,66 +2813,71 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( auto &DE = getASTContext().Diags; auto name = decl->getFullName(); - auto emitGeneralAmbiguityFailure = [&]() { - // Three choices here: - // 1. If this is a special name avoid printing it because - // printing kind is sufficient; - // 2. If all of the labels match, print a full name; - // 3. If labels in different choices are different, it means - // that we can only print a base name. - if (name.isSpecial()) { - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call_special, - decl->getDescriptiveKind()); - } else if (name.isOperator()) { - auto operatorId = name.getBaseIdentifier(); - diagnoseOperatorAmbiguity(*this, operatorId, solutions, - commonCalleeLocator); - } else if (llvm::all_of(distinctChoices, - [&name](const ValueDecl *choice) { - return choice->getFullName() == name; - })) { - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call, - decl->getDescriptiveKind(), name); - } else { - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call_no_labels, - decl->getDescriptiveKind(), name.getBaseName()); - } - }; + if (name.isOperator()) { + diagnoseOperatorAmbiguity(*this, name.getBaseIdentifier(), solutions, + commonCalleeLocator); + return true; + } - switch (ambiguityKind) { - case AmbiguityKind::ArgumentMismatch: - case AmbiguityKind::CloseMatch: - // Handled below - break; - case AmbiguityKind::ParameterList: { - emitGeneralAmbiguityFailure(); + // Emit an error message for the ambiguity. + if (aggregatedFixes.size() == 1 && + aggregatedFixes.front().first.first + ->getLastElementAs()) { + auto *anchor = aggregatedFixes.front().first.first->getAnchor(); + auto baseName = name.getBaseName(); + DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type, + baseName.userFacingName(), getContextualType(anchor)); + } else if (name.isSpecial()) { + // If this is a special name, avoid printing it because printing the + // decl kind is sufficient. + DE.diagnose(commonAnchor->getLoc(), + diag::no_overloads_match_exactly_in_call_special, + decl->getDescriptiveKind()); + } else if (llvm::all_of(ambiguousOverload->choices, + [&name](const OverloadChoice &choice) { + return choice.getDecl()->getFullName() == name; + })) { + // If the labels match in each overload, print a full name. + DE.diagnose(commonAnchor->getLoc(), + diag::no_overloads_match_exactly_in_call, + decl->getDescriptiveKind(), name); + } else { + // If the labels are different, we can only print a base name. + DE.diagnose(commonAnchor->getLoc(), + diag::no_overloads_match_exactly_in_call_no_labels, + decl->getDescriptiveKind(), name.getBaseName()); + } + + // Produce candidate notes + SmallPtrSet distinctChoices; + llvm::SmallSet candidateTypes; + for (const auto &solution: solutions) { + auto overload = solution.getOverloadChoice(commonCalleeLocator); + auto *decl = overload.choice.getDecl(); + auto type = solution.simplifyType(overload.openedType); + // Skip if we've already produced a note for this overload + if (!distinctChoices.insert(decl).second) + continue; - for (const auto &viable: viableSolutions) { - auto overload = viable->getOverloadChoice(commonCalleeLocator); - auto *fn = overload.openedType->getAs(); + if (solution.Fixes.size() == 1) { + // Create scope so each applied solution is rolled back. + ConstraintSystem::SolverScope scope(*this); + applySolution(solution); + // All of the solutions supposed to produce a "candidate" note. + diagnosed &= solution.Fixes.front()->diagnose(/*asNote*/ true); + } else if (llvm::all_of(solution.Fixes, + [&](ConstraintFix *fix) { + return fix->getLocator() + ->findLast().hasValue(); + })) { + // All fixes have to do with arguments, so let's show the parameter lists. + auto *fn = type->getAs(); assert(fn); - DE.diagnose(overload.choice.getDecl()->getLoc(), + DE.diagnose(decl->getLoc(), diag::candidate_partial_match, fn->getParamListAsString(fn->getParams())); - } - - return true; - } - case AmbiguityKind::General: { - emitGeneralAmbiguityFailure(); - - // Notes for operators are diagnosed through emitGeneralAmbiguityFailure - if (name.isOperator()) - return true; - - llvm::SmallSet candidateTypes; - for (const auto &viable: viableSolutions) { - auto overload = viable->getOverloadChoice(commonCalleeLocator); - auto *decl = overload.choice.getDecl(); - auto type = viable->simplifyType(overload.openedType); + } else { + // Emit a general "found candidate" note if (decl->getLoc().isInvalid()) { if (candidateTypes.insert(type->getCanonicalType()).second) DE.diagnose(commonAnchor->getLoc(), diag::found_candidate_type, type); @@ -2918,43 +2885,6 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( DE.diagnose(decl->getLoc(), diag::found_candidate); } } - - return true; - } - } - - auto *fix = viableSolutions.front()->Fixes.front(); - if (fix->getKind() == FixKind::UseSubscriptOperator) { - auto *UDE = cast(commonAnchor); - DE.diagnose(commonAnchor->getLoc(), - diag::could_not_find_subscript_member_did_you_mean, - getType(UDE->getBase())); - } else if (fix->getKind() == FixKind::TreatRValueAsLValue) { - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_assignment, - decl->getBaseName()); - } else if (llvm::all_of( - viableSolutions, - [](const Solution *viable) { - auto *locator = viable->Fixes.front()->getLocator(); - return locator - ->isLastElement(); - })) { - auto anchor = - viableSolutions.front()->Fixes.front()->getLocator()->getAnchor(); - auto baseName = name.getBaseName(); - DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type, - baseName.userFacingName(), getContextualType(anchor)); - } else { - emitGeneralAmbiguityFailure(); - } - - for (const auto &viable : viableSolutions) { - // Create scope so each applied solution is rolled back. - ConstraintSystem::SolverScope scope(*this); - applySolution(*viable); - // All of the solutions supposed to produce a "candidate" note. - diagnosed &= viable->Fixes.front()->diagnose(/*asNote*/ true); } // If not all of the fixes produced a note, we can't diagnose this. diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index b9ffb4a484c80..9c6397c2cd276 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -1085,7 +1085,7 @@ func SR_6272_c() { struct SR_6272_D: ExpressibleByIntegerLiteral { typealias IntegerLiteralType = Int init(integerLiteral: Int) {} - static func +(lhs: SR_6272_D, rhs: Int) -> Float { return 42.0 } // expected-note 2 {{candidate expects value of type 'Int' for parameter #2}} + static func +(lhs: SR_6272_D, rhs: Int) -> Float { return 42.0 } } func SR_6272_d() { diff --git a/test/decl/subscript/subscripting.swift b/test/decl/subscript/subscripting.swift index c7b92dc292d9c..1065c0e69d31e 100644 --- a/test/decl/subscript/subscripting.swift +++ b/test/decl/subscript/subscripting.swift @@ -385,9 +385,9 @@ func testSubscript1(_ s1 : SubscriptTest1) { _ = s1.subscript(SuperClass()) // expected-error@-1 {{no exact matches in call to subscript}} _ = s1.subscript("hello") - // expected-error@-1 {{value of type 'SubscriptTest1' has no property or method named 'subscript'; did you mean to use the subscript operator?}} + // expected-error@-1 {{no exact matches in call to subscript}} _ = s1.subscript("hello" - // expected-error@-1 {{value of type 'SubscriptTest1' has no property or method named 'subscript'; did you mean to use the subscript operator?}} + // expected-error@-1 {{no exact matches in call to subscript}} // expected-note@-2 {{to match this opening '('}} let _ = s1["hello"] From 651c27b50baafad32a4dc8149759a8d01dba1cc0 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 7 Feb 2020 16:25:46 -0800 Subject: [PATCH 03/13] [Diagnostics] Add CSFix::diagnoseForAmbiguity for diagnosing common fixes that appear in all solutions. --- lib/Sema/CSFix.cpp | 18 +++++++++++++++ lib/Sema/CSFix.h | 20 ++++++++++++++++ lib/Sema/ConstraintSystem.cpp | 29 +++++++++++------------- test/Constraints/closures.swift | 2 +- test/Misc/misc_diagnostics.swift | 6 ++--- test/Sema/diag_ambiguous_overloads.swift | 4 ++-- test/type/protocol_composition.swift | 4 +--- 7 files changed, 58 insertions(+), 25 deletions(-) diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 19f553667c775..c127f3cb014dc 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -479,6 +479,24 @@ bool DefineMemberBasedOnUse::diagnose(bool asNote) const { return AlreadyDiagnosed || failure.diagnose(asNote); } +bool +DefineMemberBasedOnUse::diagnoseForAmbiguity(ArrayRef solutions) const { + Type concreteBaseType; + for (const auto &solution: solutions) { + auto baseType = solution.simplifyType(BaseType); + if (!concreteBaseType) + concreteBaseType = baseType; + + if (concreteBaseType->getCanonicalType() != baseType->getCanonicalType()) { + getConstraintSystem().getASTContext().Diags.diagnose(getAnchor()->getLoc(), + diag::unresolved_member_no_inference, Name); + return true; + } + } + + return diagnose(); +} + DefineMemberBasedOnUse * DefineMemberBasedOnUse::create(ConstraintSystem &cs, Type baseType, DeclNameRef member, bool alreadyDiagnosed, diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h index d4c453070228d..e2b6121ff0594 100644 --- a/lib/Sema/CSFix.h +++ b/lib/Sema/CSFix.h @@ -289,6 +289,8 @@ class ConstraintFix { /// Diagnose a failure associated with this fix. virtual bool diagnose(bool asNote = false) const = 0; + virtual bool diagnoseForAmbiguity(ArrayRef solutions) const { return false; } + void print(llvm::raw_ostream &Out) const; SWIFT_DEBUG_DUMP; @@ -843,6 +845,8 @@ class DefineMemberBasedOnUse final : public ConstraintFix { bool diagnose(bool asNote = false) const override; + bool diagnoseForAmbiguity(ArrayRef solutions) const override; + static DefineMemberBasedOnUse *create(ConstraintSystem &cs, Type baseType, DeclNameRef member, bool alreadyDiagnosed, ConstraintLocator *locator); @@ -1109,6 +1113,10 @@ class AddMissingArguments final bool diagnose(bool asNote = false) const override; + bool diagnoseForAmbiguity(ArrayRef solutions) const override { + return diagnose(); + } + static AddMissingArguments *create(ConstraintSystem &cs, llvm::ArrayRef synthesizedArgs, ConstraintLocator *locator); @@ -1149,6 +1157,10 @@ class RemoveExtraneousArguments final bool diagnose(bool asNote = false) const override; + bool diagnoseForAmbiguity(ArrayRef solutions) const override { + return diagnose(); + } + /// FIXME(diagnostics): Once `resolveDeclRefExpr` is gone this /// logic would be obsolete. /// @@ -1349,6 +1361,10 @@ class DefaultGenericArgument final : public ConstraintFix { bool diagnose(bool asNote = false) const override; + bool diagnoseForAmbiguity(ArrayRef solutions) const override { + return diagnose(); + } + static DefaultGenericArgument *create(ConstraintSystem &cs, GenericTypeParamType *param, ConstraintLocator *locator); @@ -1420,6 +1436,10 @@ class IgnoreContextualType : public ContextualMismatch { bool diagnose(bool asNote = false) const override; + bool diagnoseForAmbiguity(ArrayRef solutions) const override { + return diagnose(); + } + static IgnoreContextualType *create(ConstraintSystem &cs, Type resultTy, Type specifiedTy, ConstraintLocator *locator); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index ab019e79e3722..b3e2a81430f3b 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2761,22 +2761,9 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( } } - // If there are no overload differences, diagnose the common fixes. - SolutionDiff solutionDiff(solutions); - if (solutionDiff.overloads.size() == 0) { - bool diagnosed = false; - ConstraintSystem::SolverScope scope(*this); - applySolution(solutions.front()); - for (auto fixes: aggregatedFixes) { - // A common fix must appear in all solutions - if (fixes.second.size() < solutions.size()) continue; - diagnosed |= fixes.second.front()->diagnose(); - } - return diagnosed; - } - // If there is an overload difference, let's see if there's a common callee // locator for all of the fixes. + SolutionDiff solutionDiff(solutions); auto ambiguousOverload = llvm::find_if(solutionDiff.overloads, [&](const auto &overloadDiff) { return llvm::all_of(aggregatedFixes, [&](const auto &aggregatedFix) { @@ -2798,8 +2785,18 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( }); }); - if (ambiguousOverload == solutionDiff.overloads.end()) - return false; + // If we didn't find an ambiguous overload, diagnose the common fixes. + if (ambiguousOverload == solutionDiff.overloads.end()) { + bool diagnosed = false; + ConstraintSystem::SolverScope scope(*this); + applySolution(solutions.front()); + for (auto fixes: aggregatedFixes) { + // A common fix must appear in all solutions + if (fixes.second.size() < solutions.size()) continue; + diagnosed |= fixes.second.front()->diagnoseForAmbiguity(solutions); + } + return diagnosed; + } auto *commonCalleeLocator = ambiguousOverload->locator; auto *decl = ambiguousOverload->choices.front().getDecl(); diff --git a/test/Constraints/closures.swift b/test/Constraints/closures.swift index 876b5a1b76260..67d87f40203ca 100644 --- a/test/Constraints/closures.swift +++ b/test/Constraints/closures.swift @@ -186,7 +186,7 @@ func testMap() { } // "UnresolvedDot" "in wrong phase" assertion from verifier -[].reduce { $0 + $1 } // expected-error {{cannot invoke 'reduce' with an argument list of type '(_)'}} +[].reduce { $0 + $1 } // expected-error {{missing argument for parameter #1 in call}} diff --git a/test/Misc/misc_diagnostics.swift b/test/Misc/misc_diagnostics.swift index 4449414e68fa5..b26c78b0aa66a 100644 --- a/test/Misc/misc_diagnostics.swift +++ b/test/Misc/misc_diagnostics.swift @@ -47,12 +47,12 @@ takesInt(noParams(1)) // expected-error{{argument passed to call that takes no a takesInt(takesAndReturnsInt("")) // expected-error{{cannot convert value of type 'String' to expected argument type 'Int'}} // Test error recovery for type expressions. -struct MyArray {} +struct MyArray {} // expected-note {{'Element' declared as parameter to type 'MyArray'}} class A { var a: MyArray init() { - a = MyArray.Type' and 'Int.Type'}} - // expected-note @-1 {{overloads for '<' exist with these partially matching parameter lists:}} + a = MyArray(_ f: (T) -> T) -> Void {} // expected-note {{in call to function 'fg'}} fg({x in x}) // expected-error {{generic parameter 'T' could not be inferred}} diff --git a/test/type/protocol_composition.swift b/test/type/protocol_composition.swift index a75b0414b5aa5..b78a9af3a5fd0 100644 --- a/test/type/protocol_composition.swift +++ b/test/type/protocol_composition.swift @@ -173,9 +173,7 @@ 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'}} -// 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}} +takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}} typealias T08 = P1 & inout P2 // expected-error {{'inout' may only be used on parameters}} typealias T09 = P1 & __shared P2 // expected-error {{'__shared' may only be used on parameters}} From 1a628f31077ca60f9f5a415b0ed7b8c3b4729212 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 7 Feb 2020 16:36:56 -0800 Subject: [PATCH 04/13] [Diagnostics] Simplify logic in diagnoseAmbiguityWithFixes for emitting the ambiguity diagnostic by combining a few special errors into one. --- include/swift/AST/DiagnosticsSema.def | 10 ++-------- lib/Sema/ConstraintSystem.cpp | 20 +++----------------- test/Sema/diag_non_ephemeral.swift | 2 +- test/decl/ext/protocol.swift | 4 ++-- 4 files changed, 8 insertions(+), 28 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 610c9c897fd28..7c06b1fe1e8da 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -61,14 +61,8 @@ ERROR(ambiguous_member_overload_set,none, ERROR(ambiguous_reference_to_decl,none, "ambiguous reference to %0 %1", (DescriptiveDeclKind, DeclName)) ERROR(no_overloads_match_exactly_in_call,none, - "no exact matches in call to %0 %1", - (DescriptiveDeclKind, DeclName)) -ERROR(no_overloads_match_exactly_in_call_no_labels,none, - "no exact matches in call to %0 %1", - (DescriptiveDeclKind, DeclBaseName)) -ERROR(no_overloads_match_exactly_in_call_special,none, - "no exact matches in call to %0", - (DescriptiveDeclKind)) + "no exact matches in call to %0 %select{%2|}1", + (DescriptiveDeclKind, bool, DeclBaseName)) ERROR(no_overloads_match_exactly_in_assignment,none, "no exact matches in assignment to %0", (DeclBaseName)) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index b3e2a81430f3b..b75d211a1178b 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2824,25 +2824,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( auto baseName = name.getBaseName(); DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type, baseName.userFacingName(), getContextualType(anchor)); - } else if (name.isSpecial()) { - // If this is a special name, avoid printing it because printing the - // decl kind is sufficient. - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call_special, - decl->getDescriptiveKind()); - } else if (llvm::all_of(ambiguousOverload->choices, - [&name](const OverloadChoice &choice) { - return choice.getDecl()->getFullName() == name; - })) { - // If the labels match in each overload, print a full name. - DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call, - decl->getDescriptiveKind(), name); } else { - // If the labels are different, we can only print a base name. DE.diagnose(commonAnchor->getLoc(), - diag::no_overloads_match_exactly_in_call_no_labels, - decl->getDescriptiveKind(), name.getBaseName()); + diag::no_overloads_match_exactly_in_call, + decl->getDescriptiveKind(), name.isSpecial(), + name.getBaseName()); } // Produce candidate notes diff --git a/test/Sema/diag_non_ephemeral.swift b/test/Sema/diag_non_ephemeral.swift index 451d625989865..8868afa5ea051 100644 --- a/test/Sema/diag_non_ephemeral.swift +++ b/test/Sema/diag_non_ephemeral.swift @@ -489,7 +489,7 @@ func takesPointerOverload(x: Int = 0, @_nonEphemeral _ ptr: UnsafeMutablePointer func testAmbiguity() { var arr = [1, 2, 3] - takesPointerOverload(&arr) // expected-error {{no exact matches in call to global function 'takesPointerOverload(x:_:)'}} + takesPointerOverload(&arr) // expected-error {{no exact matches in call to global function 'takesPointerOverload'}} } func takesPointerOverload2(@_nonEphemeral _ ptr: UnsafePointer) {} diff --git a/test/decl/ext/protocol.swift b/test/decl/ext/protocol.swift index 70e6a1cd65abd..2628bbc8aab11 100644 --- a/test/decl/ext/protocol.swift +++ b/test/decl/ext/protocol.swift @@ -236,9 +236,9 @@ extension P4 where Self.AssocP4 == Bool { } func testP4(_ s4a: S4a, s4b: S4b, s4c: S4c, s4d: S4d) { - s4a.extP4a() // expected-error{{no exact matches in call to instance method 'extP4a()'}} + s4a.extP4a() // expected-error{{no exact matches in call to instance method 'extP4a'}} s4b.extP4a() // ok - s4c.extP4a() // expected-error{{no exact matches in call to instance method 'extP4a()'}} + s4c.extP4a() // expected-error{{no exact matches in call to instance method 'extP4a'}} s4c.extP4Int() // okay var b1 = s4d.extP4a() // okay, "Bool" version b1 = true // checks type above From d1f6b3e2efc536b6cf2c5610c8113fe757989f02 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Mon, 10 Feb 2020 12:12:37 -0800 Subject: [PATCH 05/13] [Diagnostics] When diagnosing an ambiguous overload, don't mention "call" if the expression is not a function application. --- include/swift/AST/DiagnosticsSema.def | 7 ++----- lib/Sema/ConstraintSystem.cpp | 7 +++++++ test/Constraints/diagnostics.swift | 2 +- test/Constraints/patterns.swift | 2 +- test/NameBinding/import-resolution-overload.swift | 2 +- test/Parse/super.swift | 2 +- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 7c06b1fe1e8da..7fbe23b1c17a7 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -61,11 +61,8 @@ ERROR(ambiguous_member_overload_set,none, ERROR(ambiguous_reference_to_decl,none, "ambiguous reference to %0 %1", (DescriptiveDeclKind, DeclName)) ERROR(no_overloads_match_exactly_in_call,none, - "no exact matches in call to %0 %select{%2|}1", - (DescriptiveDeclKind, bool, DeclBaseName)) -ERROR(no_overloads_match_exactly_in_assignment,none, - "no exact matches in assignment to %0", - (DeclBaseName)) + "no exact matches in %select{reference|call}0 to %1 %select{%3|}2", + (bool, DescriptiveDeclKind, bool, DeclBaseName)) NOTE(candidate_partial_match,none, "candidate has partially matching parameter list %0", diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index b75d211a1178b..94518ff045190 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2807,6 +2807,8 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( DiagnosticTransaction transaction(getASTContext().Diags); auto *commonAnchor = commonCalleeLocator->getAnchor(); + if (auto *callExpr = dyn_cast(commonAnchor)) + commonAnchor = callExpr->getDirectCallee(); auto &DE = getASTContext().Diags; auto name = decl->getFullName(); @@ -2825,8 +2827,13 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type, baseName.userFacingName(), getContextualType(anchor)); } else { + bool isApplication = llvm::find_if(ArgumentInfos, [&](const auto argInfo) { + return argInfo.first->getAnchor() == commonAnchor; + }) != ArgumentInfos.end(); + DE.diagnose(commonAnchor->getLoc(), diag::no_overloads_match_exactly_in_call, + isApplication, decl->getDescriptiveKind(), name.isSpecial(), name.getBaseName()); } diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index 9c6397c2cd276..6ba8223922fce 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -935,7 +935,7 @@ class CacheValue { func valueForKey(_ key: K) -> CacheValue? { let cache = NSCache() - return cache.object(forKey: key)?.value // expected-error {{no exact matches in call to instance method 'value'}} + return cache.object(forKey: key)?.value // expected-error {{no exact matches in reference to instance method 'value'}} } // SR-2242: poor diagnostic when argument label is omitted diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index ef03365f1ce4a..401fbf542f2d6 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -301,7 +301,7 @@ switch staticMembers { // TODO: repeated error message case .optProp: break // expected-error* {{not unwrapped}} - case .method: break // expected-error{{no exact matches in call to static method 'method'}} + case .method: break // expected-error{{no exact matches in reference to static method 'method'}} case .method(0): break case .method(_): break // expected-error{{'_' can only appear in a pattern}} case .method(let x): break // expected-error{{cannot appear in an expression}} diff --git a/test/NameBinding/import-resolution-overload.swift b/test/NameBinding/import-resolution-overload.swift index 9f5a89eb13247..ce9c493560811 100644 --- a/test/NameBinding/import-resolution-overload.swift +++ b/test/NameBinding/import-resolution-overload.swift @@ -46,7 +46,7 @@ scopedFunction = 42 // FIXME: Should be an error -- a type name and a function cannot overload. var _ : Int = TypeNameWins(42) -TypeNameWins = 42 // expected-error {{no exact matches in assignment to 'TypeNameWins'}} +TypeNameWins = 42 // expected-error {{no exact matches in reference to global function 'TypeNameWins'}} var _ : TypeNameWins // no-warning // rdar://problem/21739333 diff --git a/test/Parse/super.swift b/test/Parse/super.swift index c4317a4bf18b8..a7acb18b2ad76 100644 --- a/test/Parse/super.swift +++ b/test/Parse/super.swift @@ -39,7 +39,7 @@ class D : B { super.bar // expected-error {{expression resolves to an unused function}} super.bar() // FIXME: should also say "'super.init' cannot be referenced outside of an initializer" - super.init // expected-error{{no exact matches in call to initializer}} + super.init // expected-error{{no exact matches in reference to initializer}} super.init() // expected-error{{'super.init' cannot be called outside of an initializer}} super.init(0) // expected-error{{'super.init' cannot be called outside of an initializer}} // expected-error {{missing argument label 'x:' in call}} super[0] // expected-error {{expression resolves to an unused subscript}} From ab59032ebbe36bf2cb72cc46412688f0315d2152 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Mon, 10 Feb 2020 12:51:07 -0800 Subject: [PATCH 06/13] [CSDiag] Remove obsolete ambiguity code. --- include/swift/AST/DiagnosticsSema.def | 13 ------------- lib/Sema/CSDiag.cpp | 11 ----------- 2 files changed, 24 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 7fbe23b1c17a7..87bcef6d6ce8a 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -68,9 +68,6 @@ NOTE(candidate_partial_match,none, "candidate has partially matching parameter list %0", (StringRef)) -ERROR(ambiguous_subscript,none, - "ambiguous subscript with base type %0 and index type %1", - (Type, Type)) ERROR(could_not_find_value_subscript,none, "value of type %0 has no subscripts", (Type)) @@ -273,16 +270,6 @@ ERROR(no_candidates_match_result_type,none, "no '%0' candidates produce the expected contextual result type %1", (StringRef, Type)) -ERROR(candidates_no_match_result_type,none, - "'%0' produces %1, not the expected contextual result type %2", - (StringRef, Type, Type)) - - - -ERROR(invalid_callee_result_type,none, - "cannot convert call result type %0 to expected type %1", - (Type, Type)) - ERROR(cannot_invoke_closure,none, "cannot invoke closure expression with an argument list of type '%0'", diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index b46e4dd281c65..bfd91aea71859 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -1247,17 +1247,6 @@ void FailureDiagnosis::diagnoseAmbiguity(Expr *E) { return; } - // Diagnose ".foo" expressions that lack context specifically. - if (auto UME = - dyn_cast(E->getSemanticsProvidingExpr())) { - if (!CS.getContextualType(E)) { - diagnose(E->getLoc(), diag::unresolved_member_no_inference,UME->getName()) - .highlight(SourceRange(UME->getDotLoc(), - UME->getNameLoc().getSourceRange().End)); - return; - } - } - // Attempt to re-type-check the entire expression, allowing ambiguity, but // ignoring a contextual type. if (expr == E) { From 5437622d2da556b23ab459452d2d4bcfb0dd0d13 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 12 Feb 2020 17:23:07 -0800 Subject: [PATCH 07/13] [Diagnostics] Diagnose ambiguity with conflicting arguments to generic parameters It's done by first retrieving all generic parameters from each solution, filtering boundings into distrinct set and diagnosing any differences. For example: ```swift func foo(_: T, _: T) {} func bar(x: Int, y: Float) { foo(x, y) } ``` --- include/swift/AST/DiagnosticsSema.def | 4 + lib/Sema/CSDiagnostics.cpp | 34 ---- lib/Sema/CSFix.cpp | 6 + lib/Sema/CSFix.h | 6 + lib/Sema/ConstraintSystem.cpp | 154 +++++++++++++++++- lib/Sema/ConstraintSystem.h | 2 + test/Generics/deduction.swift | 10 +- .../suppress-argument-labels-in-types.swift | 2 +- test/Sema/type_join.swift | 4 +- test/decl/func/default-values.swift | 2 +- test/type/opaque.swift | 9 +- 11 files changed, 187 insertions(+), 46 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 87bcef6d6ce8a..cb18f72f48422 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -385,6 +385,10 @@ ERROR(cannot_convert_argument_value_generic,none, "cannot convert value of type %0 (%1) to expected argument type %2 (%3)", (Type, StringRef, Type, StringRef)) +ERROR(conflicting_arguments_for_generic_parameter,none, + "conflicting arguments to generic parameter %0 (%1)", + (Type, StringRef)) + // @_nonEphemeral conversion diagnostics ERROR(cannot_pass_type_to_non_ephemeral,none, "cannot pass %0 to parameter; argument %1 must be a pointer that " diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index f853f146d9919..6471c45973e1b 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -5585,40 +5585,6 @@ bool ArgumentMismatchFailure::diagnoseArchetypeMismatch() const { if (!(paramDecl && argDecl)) return false; - auto describeGenericType = [&](ValueDecl *genericParam, - bool includeName = false) -> std::string { - if (!genericParam) - return ""; - - Decl *parent = nullptr; - if (auto *AT = dyn_cast(genericParam)) { - parent = AT->getProtocol(); - } else { - auto *dc = genericParam->getDeclContext(); - parent = dc->getInnermostDeclarationDeclContext(); - } - - if (!parent) - return ""; - - llvm::SmallString<64> result; - llvm::raw_svector_ostream OS(result); - - OS << Decl::getDescriptiveKindName(genericParam->getDescriptiveKind()); - - if (includeName && genericParam->hasName()) - OS << " '" << genericParam->getBaseName() << "'"; - - OS << " of "; - OS << Decl::getDescriptiveKindName(parent->getDescriptiveKind()); - if (auto *decl = dyn_cast(parent)) { - if (decl->hasName()) - OS << " '" << decl->getFullName() << "'"; - } - - return OS.str(); - }; - emitDiagnostic( getAnchor()->getLoc(), diag::cannot_convert_argument_value_generic, argTy, describeGenericType(argDecl), paramTy, describeGenericType(paramDecl)); diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index c127f3cb014dc..67bbe57cbfdad 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -1106,6 +1106,12 @@ UseValueTypeOfRawRepresentative::attempt(ConstraintSystem &cs, Type argType, return nullptr; } +unsigned AllowArgumentMismatch::getParamIdx() const { + const auto *locator = getLocator(); + auto elt = locator->castLastElementTo(); + return elt.getParamIdx(); +} + bool AllowArgumentMismatch::diagnose(bool asNote) const { auto &cs = getConstraintSystem(); ArgumentMismatchFailure failure(cs, getFromType(), getToType(), diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h index e2b6121ff0594..15d0378ef98b2 100644 --- a/lib/Sema/CSFix.h +++ b/lib/Sema/CSFix.h @@ -1499,11 +1499,17 @@ class AllowArgumentMismatch : public ContextualMismatch { return "allow argument to parameter type conversion mismatch"; } + unsigned getParamIdx() const; + bool diagnose(bool asNote = false) const override; static AllowArgumentMismatch *create(ConstraintSystem &cs, Type argType, Type paramType, ConstraintLocator *locator); + + static bool classof(const ConstraintFix *fix) { + return fix->getKind() == FixKind::AllowArgumentTypeMismatch; + } }; class ExpandArrayIntoVarargs final : public AllowArgumentMismatch { diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 94518ff045190..e42e6cc1deaca 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2729,6 +2729,154 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs, llvm::join(parameters, ", ")); } +std::string swift::describeGenericType(ValueDecl *GP, bool includeName) { + if (!GP) + return ""; + + Decl *parent = nullptr; + if (auto *AT = dyn_cast(GP)) { + parent = AT->getProtocol(); + } else { + auto *dc = GP->getDeclContext(); + parent = dc->getInnermostDeclarationDeclContext(); + } + + if (!parent) + return ""; + + llvm::SmallString<64> result; + llvm::raw_svector_ostream OS(result); + + OS << Decl::getDescriptiveKindName(GP->getDescriptiveKind()); + + if (includeName && GP->hasName()) + OS << " '" << GP->getBaseName() << "'"; + + OS << " of "; + OS << Decl::getDescriptiveKindName(parent->getDescriptiveKind()); + if (auto *decl = dyn_cast(parent)) { + if (decl->hasName()) + OS << " '" << decl->getFullName() << "'"; + } + + return OS.str(); +} + +/// Special handling of conflicts associated with generic arguments. +/// +/// func foo(_: T, _: T) {} +/// func bar(x: Int, y: Float) { +/// foo(x, y) +/// } +/// +/// It's done by first retrieving all generic parameters from each solution, +/// filtering boundings into distrinct set and diagnosing any differences. +static bool diagnoseConflictingArguments(ConstraintSystem &cs, + const SolutionDiff &diff, + ArrayRef solutions) { + if (!diff.overloads.empty()) + return false; + + if (!llvm::all_of(solutions, [](const Solution &solution) -> bool { + return llvm::all_of( + solution.Fixes, [](const ConstraintFix *fix) -> bool { + return fix->getKind() == FixKind::AllowArgumentTypeMismatch || + fix->getKind() == FixKind::AllowFunctionTypeMismatch || + fix->getKind() == FixKind::AllowTupleTypeMismatch; + }); + })) + return false; + + auto &DE = cs.getASTContext().Diags; + + llvm::SmallDenseMap> conflicts; + + for (const auto &binding : solutions[0].typeBindings) { + auto *typeVar = binding.first; + + if (!typeVar->getImpl().getGenericParameter()) + continue; + + llvm::SmallSetVector arguments; + arguments.insert(binding.second); + + if (!llvm::all_of(solutions.slice(1), [&](const Solution &solution) { + auto binding = solution.typeBindings.find(typeVar); + if (binding == solution.typeBindings.end()) + return false; + + if (auto *opaque = + binding->second->getAs()) { + auto *decl = opaque->getDecl(); + arguments.remove_if([&](Type argType) -> bool { + if (auto *otherOpaque = + argType->getAs()) { + return decl == otherOpaque->getDecl(); + } + return false; + }); + } + + arguments.insert(binding->second); + return true; + })) + continue; + + if (arguments.size() > 1) { + conflicts[typeVar].append(arguments.begin(), arguments.end()); + } + } + + auto getGenericTypeDecl = [&](ArchetypeType *archetype) -> ValueDecl * { + auto type = archetype->getInterfaceType(); + + if (auto *GTPT = type->getAs()) + return GTPT->getDecl(); + + if (auto *DMT = type->getAs()) + return DMT->getAssocType(); + + return nullptr; + }; + + bool diagnosed = false; + for (auto &conflict : conflicts) { + auto *typeVar = conflict.first; + auto *locator = typeVar->getImpl().getLocator(); + auto conflictingArguments = conflict.second; + + llvm::SmallString<64> arguments; + llvm::raw_svector_ostream OS(arguments); + + interleave( + conflictingArguments, + [&](Type argType) { + OS << "'" << argType << "'"; + + if (auto *opaque = argType->getAs()) { + auto *decl = opaque->getDecl()->getNamingDecl(); + OS << " (result type of '" << decl->getBaseName().userFacingName() + << "')"; + return; + } + + if (auto archetype = argType->getAs()) { + if (auto *GTD = getGenericTypeDecl(archetype)) + OS << " (" << describeGenericType(GTD) << ")"; + } + }, + [&OS] { OS << " vs. "; }); + + auto *anchor = locator->getAnchor(); + DE.diagnose(anchor->getLoc(), + diag::conflicting_arguments_for_generic_parameter, + typeVar->getImpl().getGenericParameter(), OS.str()); + diagnosed = true; + } + + return diagnosed; +} + bool ConstraintSystem::diagnoseAmbiguityWithFixes( SmallVectorImpl &solutions) { if (solutions.empty()) @@ -2749,6 +2897,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( return false; } + SolutionDiff solutionDiff(solutions); + + if (diagnoseConflictingArguments(*this, solutionDiff, solutions)) + return true; + // Collect aggregated fixes from all solutions llvm::SmallMapVector, llvm::SmallVector, 4> aggregatedFixes; @@ -2763,7 +2916,6 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( // If there is an overload difference, let's see if there's a common callee // locator for all of the fixes. - SolutionDiff solutionDiff(solutions); auto ambiguousOverload = llvm::find_if(solutionDiff.overloads, [&](const auto &overloadDiff) { return llvm::all_of(aggregatedFixes, [&](const auto &aggregatedFix) { diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index d4f4055c759f1..f08c1a6afcae3 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -5115,6 +5115,8 @@ bool exprNeedsParensOutsideFollowingOperator( /// Determine whether this is a SIMD operator. bool isSIMDOperator(ValueDecl *value); +std::string describeGenericType(ValueDecl *GP, bool includeName = false); + /// Apply the given function builder transform within a specific solution /// to produce the rewritten body. /// diff --git a/test/Generics/deduction.swift b/test/Generics/deduction.swift index 9ce7ff1f3419a..2a5ad2cfe60f7 100644 --- a/test/Generics/deduction.swift +++ b/test/Generics/deduction.swift @@ -28,7 +28,6 @@ func useIdentity(_ x: Int, y: Float, i32: Int32) { xx = identity2(yy) // expected-error{{no exact matches in call to global function 'identity2'}} } -// FIXME: Crummy diagnostic! func twoIdentical(_ x: T, _ y: T) -> T {} func useTwoIdentical(_ xi: Int, yi: Float) { @@ -40,7 +39,7 @@ func useTwoIdentical(_ xi: Int, yi: Float) { y = twoIdentical(1.0, y) y = twoIdentical(y, 1.0) - twoIdentical(x, y) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}} + twoIdentical(x, y) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}} } func mySwap(_ x: inout T, @@ -67,8 +66,9 @@ func takeTuples(_: (T, U), _: (U, T)) { func useTuples(_ x: Int, y: Float, z: (Float, Int)) { takeTuples((x, y), (y, x)) - takeTuples((x, y), (x, y)) // expected-error{{cannot convert value of type '(Int, Float)' to expected argument type '(Float, Int)'}} - + takeTuples((x, y), (x, y)) + // expected-error@-1 {{conflicting arguments to generic parameter 'U' ('Float' vs. 'Int')}} + // expected-error@-2 {{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}} // FIXME: Use 'z', which requires us to fix our tuple-conversion // representation. } @@ -315,7 +315,7 @@ struct A {} func foo() { for i in min(1,2) { // expected-error{{for-in loop requires 'Int' to conform to 'Sequence'}} } - let j = min(Int(3), Float(2.5)) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}} + let j = min(Int(3), Float(2.5)) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}} let k = min(A(), A()) // expected-error{{global function 'min' requires that 'A' conform to 'Comparable'}} let oi : Int? = 5 let l = min(3, oi) // expected-error{{global function 'min' requires that 'Int?' conform to 'Comparable'}} diff --git a/test/Sema/suppress-argument-labels-in-types.swift b/test/Sema/suppress-argument-labels-in-types.swift index adff0d0d7a1d9..7945ea146fe89 100644 --- a/test/Sema/suppress-argument-labels-in-types.swift +++ b/test/Sema/suppress-argument-labels-in-types.swift @@ -203,7 +203,7 @@ class C0 { } // Check diagnostics changes. -let _ = min(Int(3), Float(2.5)) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}} +let _ = min(Int(3), Float(2.5)) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}} // SR-11429 func testIntermediateCoercions() { diff --git a/test/Sema/type_join.swift b/test/Sema/type_join.swift index ad6d2b78cdc27..29509a43e8838 100644 --- a/test/Sema/type_join.swift +++ b/test/Sema/type_join.swift @@ -98,9 +98,9 @@ func joinFunctions( // Any to be inferred for the generic type. That's pretty // arbitrary. _ = commonSupertype(escaping, x) - // expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '() -> ()'}} + // expected-error@-1 {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. 'Int')}} _ = commonSupertype(x, escaping) - // expected-error@-1 {{cannot convert value of type '() -> ()' to expected argument type 'Int'}} + // expected-error@-1 {{conflicting arguments to generic parameter 'T' ('Int' vs. '() -> ()')}} let a: Any = 1 _ = commonSupertype(nonescaping, a) diff --git a/test/decl/func/default-values.swift b/test/decl/func/default-values.swift index 7a90c2587d9f3..a389c6691203b 100644 --- a/test/decl/func/default-values.swift +++ b/test/decl/func/default-values.swift @@ -92,7 +92,7 @@ defaultArgTuplesNotMaterializable(identity(5)) // QoI: Propagate contextual information in a call to operands defaultArgTuplesNotMaterializable(identity((5, y: 10))) -// expected-error@-1 {{cannot convert value of type '(Int, y: Int)' to expected argument type 'Int'}} +// expected-error@-1 {{conflicting arguments to generic parameter 'T' ('(Int, y: Int)' vs. 'Int')}} // rdar://problem/21799331 diff --git a/test/type/opaque.swift b/test/type/opaque.swift index 9d0b7bdb95e6b..9471164136377 100644 --- a/test/type/opaque.swift +++ b/test/type/opaque.swift @@ -270,10 +270,15 @@ func associatedTypeIdentity() { sameType(cr, c.r_out()) sameType(dr, d.r_out()) - sameType(cr, dr) // expected-error {{cannot convert value of type '(some opaque.R).S' (associated type of protocol 'R') to expected argument type '(some opaque.R).S' (associated type of protocol 'R')}} + sameType(cr, dr) // expected-error {{conflicting arguments to generic parameter 'T' ('(some R).S' (associated type of protocol 'R') vs. '(some R).S' (associated type of protocol 'R'))}} sameType(gary(candace()).r_out(), gary(candace()).r_out()) sameType(gary(doug()).r_out(), gary(doug()).r_out()) - sameType(gary(doug()).r_out(), gary(candace()).r_out()) // expected-error {{cannot convert value of type 'some R' (result of 'candace()') to expected argument type 'some R' (result of 'doug()')}} + // TODO(diagnostics): This is not great but we the problem comes from the way solver discovers and attempts bindings, if we could detect that + // `(some R).S` from first reference to `gary()` in incosistent with the second one based on the parent type of `S` it would be much easier to diagnose. + sameType(gary(doug()).r_out(), gary(candace()).r_out()) + // expected-error@-1:3 {{conflicting arguments to generic parameter 'T' ('(some R).S' (associated type of protocol 'R') vs. '(some R).S' (associated type of protocol 'R'))}} + // expected-error@-2:12 {{conflicting arguments to generic parameter 'T' ('some R' (result type of 'doug') vs. 'some R' (result type of 'candace'))}} + // expected-error@-3:34 {{conflicting arguments to generic parameter 'T' ('some R' (result type of 'doug') vs. 'some R' (result type of 'candace'))}} } func redeclaration() -> some P { return 0 } // expected-note 2{{previously declared}} From af65c0e2f360be7d99ec44d51470fc609250b6b9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 12 Feb 2020 17:42:11 -0800 Subject: [PATCH 08/13] [Diagnostics] Prioritize contextual type mismatch other ambiguities --- lib/Sema/ConstraintSystem.cpp | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e42e6cc1deaca..91cbef6682624 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2904,14 +2904,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( // Collect aggregated fixes from all solutions llvm::SmallMapVector, - llvm::SmallVector, 4> aggregatedFixes; - for (const auto &solution: solutions) { - for (auto fixesPerLocator: solution.getAggregatedFixes()) { - for (auto kindAndFixes: fixesPerLocator.second) { - aggregatedFixes[{fixesPerLocator.first, kindAndFixes.first}] - .push_back(kindAndFixes.second.front()); - } - } + llvm::TinyPtrVector, 4> + aggregatedFixes; + for (const auto &solution : solutions) { + for (auto *fix : solution.Fixes) + aggregatedFixes[{fix->getLocator(), fix->getKind()}].push_back(fix); } // If there is an overload difference, let's see if there's a common callee @@ -2964,20 +2961,17 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( auto &DE = getASTContext().Diags; auto name = decl->getFullName(); - if (name.isOperator()) { - diagnoseOperatorAmbiguity(*this, name.getBaseIdentifier(), solutions, - commonCalleeLocator); - return true; - } - // Emit an error message for the ambiguity. if (aggregatedFixes.size() == 1 && - aggregatedFixes.front().first.first - ->getLastElementAs()) { + aggregatedFixes.front().first.first->isForContextualType()) { auto *anchor = aggregatedFixes.front().first.first->getAnchor(); auto baseName = name.getBaseName(); DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type, baseName.userFacingName(), getContextualType(anchor)); + } else if (name.isOperator()) { + diagnoseOperatorAmbiguity(*this, name.getBaseIdentifier(), solutions, + commonCalleeLocator); + return true; } else { bool isApplication = llvm::find_if(ArgumentInfos, [&](const auto argInfo) { return argInfo.first->getAnchor() == commonAnchor; From 4d5c676ca9e897765f3382e0c66fb663d60f2ae4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 12 Feb 2020 18:01:27 -0800 Subject: [PATCH 09/13] [ConstraintSystem] NFC: Remove obsolete code related to fixes from Solution --- lib/Sema/ConstraintSystem.h | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index f08c1a6afcae3..218d9e5d9e7d7 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -837,21 +837,9 @@ class Solution { llvm::DenseMap, ConversionRestrictionKind> ConstraintRestrictions; - using ConstraintFixVector = llvm::SmallVector; /// The list of fixes that need to be applied to the initial expression /// to make the solution work. - ConstraintFixVector Fixes; - - using AggregatedFixesMap = llvm::SmallMapVector, 4>; - - AggregatedFixesMap getAggregatedFixes() const { - AggregatedFixesMap aggregatedFixes; - for (auto *fix: Fixes) { - aggregatedFixes[fix->getLocator()][fix->getKind()].push_back(fix); - } - return aggregatedFixes; - } + llvm::SmallVector Fixes; /// The set of disjunction choices used to arrive at this solution, /// which informs constraint application. From 3f10bfa74e4a946d6bb6650141d667e994e377a4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 12 Feb 2020 18:17:04 -0800 Subject: [PATCH 10/13] [Diagnostics] NFC: Rename `diagnoseConflictingArguments` into `diagnoseConflictingGenericArguments` --- lib/Sema/ConstraintSystem.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 91cbef6682624..54b60ba298d5b 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2771,9 +2771,9 @@ std::string swift::describeGenericType(ValueDecl *GP, bool includeName) { /// /// It's done by first retrieving all generic parameters from each solution, /// filtering boundings into distrinct set and diagnosing any differences. -static bool diagnoseConflictingArguments(ConstraintSystem &cs, - const SolutionDiff &diff, - ArrayRef solutions) { +static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs, + const SolutionDiff &diff, + ArrayRef solutions) { if (!diff.overloads.empty()) return false; @@ -2899,7 +2899,7 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( SolutionDiff solutionDiff(solutions); - if (diagnoseConflictingArguments(*this, solutionDiff, solutions)) + if (diagnoseConflictingGenericArguments(*this, solutionDiff, solutions)) return true; // Collect aggregated fixes from all solutions From 60a210b6575d41470a454099940edc5f8b03a72f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 13 Feb 2020 00:44:13 -0800 Subject: [PATCH 11/13] [ConstraintSystem] NFC: Fix a couple of typos in comments --- lib/Sema/ConstraintSystem.cpp | 2 +- test/type/opaque.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 54b60ba298d5b..d0f29a3420c75 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2770,7 +2770,7 @@ std::string swift::describeGenericType(ValueDecl *GP, bool includeName) { /// } /// /// It's done by first retrieving all generic parameters from each solution, -/// filtering boundings into distrinct set and diagnosing any differences. +/// filtering bindings into a distinct set and diagnosing any differences. static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs, const SolutionDiff &diff, ArrayRef solutions) { diff --git a/test/type/opaque.swift b/test/type/opaque.swift index 9471164136377..276933e611580 100644 --- a/test/type/opaque.swift +++ b/test/type/opaque.swift @@ -273,7 +273,7 @@ func associatedTypeIdentity() { sameType(cr, dr) // expected-error {{conflicting arguments to generic parameter 'T' ('(some R).S' (associated type of protocol 'R') vs. '(some R).S' (associated type of protocol 'R'))}} sameType(gary(candace()).r_out(), gary(candace()).r_out()) sameType(gary(doug()).r_out(), gary(doug()).r_out()) - // TODO(diagnostics): This is not great but we the problem comes from the way solver discovers and attempts bindings, if we could detect that + // TODO(diagnostics): This is not great but the problem comes from the way solver discovers and attempts bindings, if we could detect that // `(some R).S` from first reference to `gary()` in incosistent with the second one based on the parent type of `S` it would be much easier to diagnose. sameType(gary(doug()).r_out(), gary(candace()).r_out()) // expected-error@-1:3 {{conflicting arguments to generic parameter 'T' ('(some R).S' (associated type of protocol 'R') vs. '(some R).S' (associated type of protocol 'R'))}} From b872bdfca1be8a86735c2ab13226097d5072c972 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 13 Feb 2020 00:45:44 -0800 Subject: [PATCH 12/13] [Diagnostics] Replace `llvm::find_if` with `llvm::any_of` in `diagnoseAmbiguityWithFixes` --- lib/Sema/ConstraintSystem.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index d0f29a3420c75..ef17b29af6efc 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2973,9 +2973,10 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes( commonCalleeLocator); return true; } else { - bool isApplication = llvm::find_if(ArgumentInfos, [&](const auto argInfo) { - return argInfo.first->getAnchor() == commonAnchor; - }) != ArgumentInfos.end(); + bool isApplication = + llvm::any_of(ArgumentInfos, [&](const auto &argInfo) { + return argInfo.first->getAnchor() == commonAnchor; + }); DE.diagnose(commonAnchor->getLoc(), diag::no_overloads_match_exactly_in_call, From 172c2003ea5bbafa5ccd267bcbd83ea72f4f20d8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 13 Feb 2020 00:54:51 -0800 Subject: [PATCH 13/13] [Diagnostics] NFC: Add a comment about special handling of opaque result types --- lib/Sema/ConstraintSystem.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index ef17b29af6efc..37b52e68c627b 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2805,6 +2805,9 @@ static bool diagnoseConflictingGenericArguments(ConstraintSystem &cs, if (binding == solution.typeBindings.end()) return false; + // Contextual opaque result type is uniquely identified by + // declaration it's associated with, so we have to compare + // declarations instead of using pointer equality on such types. if (auto *opaque = binding->second->getAs()) { auto *decl = opaque->getDecl();