Merge pull request #28979 from hborla/ambiguous-call-diagnostics

[ConstraintSystem] Start to allow diagnosing ambiguity with fixes for solution sets with more than one fix.

Author: hborla

include/swift/AST/DiagnosticsSema.def

@@ -73,6 +73,10 @@ ERROR(no_overloads_match_exactly_in_assignment,none,
       "no exact matches in assignment to %0",
       (DeclBaseName))
 
+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))
@@ -274,9 +278,6 @@ ERROR(cannot_call_with_params, none,
 ERROR(cannot_call_non_function_value,none,
       "cannot call value of non-function type %0", (Type))
 
-ERROR(wrong_argument_labels_overload,none,
-      "argument labels '%0' do not match any available overloads", (StringRef))
-
 ERROR(no_candidates_match_result_type,none,
       "no '%0' candidates produce the expected contextual result type %1",
       (StringRef, Type))

lib/Sema/CSDiag.cpp

@@ -1715,21 +1715,6 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
                                                 calleeInfo, TCC_ForceRecheck);
   if (!argExpr)
     return true; // already diagnosed.
-  
-  // Handle argument label mismatches when we have multiple candidates.
-  if (calleeInfo.closeness == CC_ArgumentLabelMismatch) {
-    auto args = decomposeArgType(CS.getType(argExpr), argLabels);
-
-    // If we have multiple candidates that we fail to match, just say we have
-    // the wrong labels and list the candidates out.
-    diagnose(callExpr->getLoc(), diag::wrong_argument_labels_overload,
-             getParamListAsString(args))
-      .highlight(argExpr->getSourceRange());
-
-    // Did the user intend on invoking a different overload?
-    calleeInfo.suggestPotentialOverloads(fnExpr->getLoc());
-    return true;
-  }
 
   auto overloadName = calleeInfo.declName;
 

lib/Sema/ConstraintSystem.cpp

@@ -2750,34 +2750,52 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
   // overloads of the same declaration.
   ConstraintLocator *commonCalleeLocator = nullptr;
   SmallPtrSet<ValueDecl *, 4> distinctChoices;
-  SmallVector<std::pair<const Solution *, const ConstraintFix *>, 4>
-      viableSolutions;
+  SmallVector<const Solution *, 4> viableSolutions;
+
+  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,
+  };
+  auto ambiguityKind = AmbiguityKind::CloseMatch;
 
   bool diagnosable = llvm::all_of(solutions, [&](const Solution &solution) {
     ArrayRef<ConstraintFix *> fixes = solution.Fixes;
 
-    // Currently only support a single fix in a solution,
-    // but ultimately should be able to deal with multiple.
-    if (fixes.size() != 1)
+    if (fixes.empty())
       return false;
 
-    const auto *fix = fixes.front();
-    auto  *locator = fix->getLocator();
+    if (fixes.size() > 1) {
+      ambiguityKind = (ambiguityKind == AmbiguityKind::CloseMatch ||
+                       ambiguityKind == AmbiguityKind::ParameterList) &&
+          llvm::all_of(fixes, [](const ConstraintFix *fix) -> bool {
+            auto *locator = fix->getLocator();
+            return locator->findLast<LocatorPathElt::ApplyArgument>().hasValue();
+          }) ? AmbiguityKind::ParameterList
+             : AmbiguityKind::General;
+    }
+
+    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<AssignExpr>(anchor))
+          locator = getConstraintLocator(assignExpr->getSrc());
+      }
 
-    // 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<AssignExpr>(anchor))
-        locator = getConstraintLocator(assignExpr->getSrc());
+      auto *calleeLocator = getCalleeLocator(locator);
+      if (!commonCalleeLocator)
+        commonCalleeLocator = calleeLocator;
+      else if (commonCalleeLocator != calleeLocator)
+        return false;
     }
 
-    auto *calleeLocator = getCalleeLocator(locator);
-    if (commonCalleeLocator && commonCalleeLocator != calleeLocator)
-      return false;
-
-    commonCalleeLocator = calleeLocator;
-
-    auto overload = solution.getOverloadChoiceIfAvailable(calleeLocator);
+    auto overload = solution.getOverloadChoiceIfAvailable(commonCalleeLocator);
     if (!overload)
       return false;
 
@@ -2789,7 +2807,7 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
     // 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, fix});
+      viableSolutions.push_back(&solution);
     return true;
   });
 
@@ -2803,32 +2821,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
   {
     DiagnosticTransaction transaction(getASTContext().Diags);
 
-    const auto *fix = viableSolutions.front().second;
     auto *commonAnchor = commonCalleeLocator->getAnchor();
     auto &DE = getASTContext().Diags;
     auto name = decl->getFullName();
 
-    if (fix->getKind() == FixKind::UseSubscriptOperator) {
-      auto *UDE = cast<UnresolvedDotExpr>(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 std::pair<const Solution *, const ConstraintFix *>
-                          &fix) {
-                     auto *locator = fix.second->getLocator();
-                     return locator
-                         ->isLastElement<LocatorPathElt::ContextualType>();
-                   })) {
-      auto baseName = name.getBaseName();
-      DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type,
-                  baseName.userFacingName(), getContextualType());
-    } else {
+    auto emitGeneralAmbiguityFailure = [&]() {
       // Three choices here:
       // 1. If this is a special name avoid printing it because
       //    printing kind is sufficient;
@@ -2855,14 +2852,61 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
                     diag::no_overloads_match_exactly_in_call_no_labels,
                     decl->getDescriptiveKind(), name.getBaseName());
       }
+    };
+
+    switch (ambiguityKind) {
+    case AmbiguityKind::CloseMatch:
+      // Handled below
+      break;
+    case AmbiguityKind::ParameterList: {
+      emitGeneralAmbiguityFailure();
+
+      for (const auto &viable: viableSolutions) {
+        auto overload = viable->getOverloadChoice(commonCalleeLocator);
+        auto *fn = overload.openedType->getAs<AnyFunctionType>();
+        assert(fn);
+        DE.diagnose(overload.choice.getDecl()->getLoc(),
+                    diag::candidate_partial_match,
+                    fn->getParamListAsString(fn->getParams()));
+      }
+
+      return true;
+    }
+    case AmbiguityKind::General:
+      // TODO: Handle general ambiguity here.
+      return false;
+    }
+
+    auto *fix = viableSolutions.front()->Fixes.front();
+    if (fix->getKind() == FixKind::UseSubscriptOperator) {
+      auto *UDE = cast<UnresolvedDotExpr>(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<LocatorPathElt::ContextualType>();
+                   })) {
+      auto baseName = name.getBaseName();
+      DE.diagnose(commonAnchor->getLoc(), diag::no_candidates_match_result_type,
+                  baseName.userFacingName(), getContextualType());
+    } else {
+      emitGeneralAmbiguityFailure();
     }
 
     for (const auto &viable : viableSolutions) {
       // Create scope so each applied solution is rolled back.
       ConstraintSystem::SolverScope scope(*this);
-      applySolution(*viable.first);
+      applySolution(*viable);
       // All of the solutions supposed to produce a "candidate" note.
-      diagnosed &= viable.second->diagnose(/*asNote*/ true);
+      diagnosed &= viable->Fixes.front()->diagnose(/*asNote*/ true);
     }
 
     // If not all of the fixes produced a note, we can't diagnose this.

test/Constraints/diagnostics.swift

@@ -492,7 +492,9 @@ enum Color {
   static func rainbow() -> Color {}
 
   static func overload(a : Int) -> Color {} // expected-note {{incorrect labels for candidate (have: '(_:)', expected: '(a:)')}}
+  // expected-note@-1 {{candidate has partially matching parameter list (a: Int)}}
   static func overload(b : Int) -> Color {} // expected-note {{incorrect labels for candidate (have: '(_:)', expected: '(b:)')}}
+  // expected-note@-1 {{candidate has partially matching parameter list (b: Int)}}
 
   static func frob(_ a : Int, b : inout Int) -> Color {}
   static var svar: Color { return .Red }
@@ -517,8 +519,7 @@ let _ : (Int, Float) = (42.0, 12)  // expected-error {{cannot convert value of t
 let _ : Color = .rainbow  // expected-error {{member 'rainbow()' is a function; did you mean to call it?}} {{25-25=()}}
 
 let _: Color = .overload(a : 1.0)  // expected-error {{cannot convert value of type 'Double' to expected argument type 'Int'}}
-let _: Color = .overload(1.0)  // expected-error {{ambiguous reference to member 'overload'}}
-// expected-note @-1 {{overloads for 'overload' exist with these partially matching parameter lists: (a: Int), (b: Int)}}
+let _: Color = .overload(1.0)  // expected-error {{no exact matches in call to static method 'overload'}}
 let _: Color = .overload(1)  // expected-error {{no exact matches in call to static method 'overload'}}
 let _: Color = .frob(1.0, &i) // expected-error {{missing argument label 'b:' in call}}
 // expected-error@-1 {{cannot convert value of type 'Double' to expected argument type 'Int'}}

test/Constraints/super_constructor.swift

@@ -20,8 +20,7 @@ class D : B {
   }
 
   init(g:Int) {
-    super.init("aoeu") // expected-error{{argument labels '(_:)' do not match any available overloads}}
-    // expected-note @-1 {{overloads for 'B.init' exist with these partially matching parameter lists: (a: UnicodeScalar), (b: UnicodeScalar), (x: Int), (z: Float)}}
+    super.init("aoeu") // expected-error{{no exact matches in call to initializer}}
   }
 
   init(h:Int) {
@@ -40,15 +39,15 @@ class B {
   init() {
   }
 
-  init(x:Int) {
+  init(x:Int) { // expected-note{{candidate has partially matching parameter list (x: Int)}}
   }
 
-  init(a:UnicodeScalar) {
+  init(a:UnicodeScalar) { // expected-note {{candidate has partially matching parameter list (a: UnicodeScalar)}}
   }
-  init(b:UnicodeScalar) {
+  init(b:UnicodeScalar) { // expected-note{{candidate has partially matching parameter list (b: UnicodeScalar)}}
   }
 
-  init(z:Float) {
+  init(z:Float) { // expected-note{{candidate has partially matching parameter list (z: Float)}}
     super.init() // expected-error{{'super' members cannot be referenced in a root class}}
   }
 }

test/expr/postfix/dot/init_ref_delegation.swift

@@ -323,13 +323,12 @@ class TestOverloadSets {
     self.init(5, 5) // expected-error{{extra argument in call}}
   }
 
-  convenience init(a : Z0) {
-    self.init(42 as Int8) // expected-error{{argument labels '(_:)' do not match any available overloads}}
-    // expected-note @-1 {{overloads for 'TestOverloadSets.init' exist with these partially matching parameter lists: (a: Z0), (value: Double), (value: Int)}}
+  convenience init(a : Z0) { // expected-note{{candidate has partially matching parameter list (a: Z0)}}
+    self.init(42 as Int8) // expected-error{{no exact matches in call to initializer}}
   }
 
-  init(value: Int) { /* ... */ }
-  init(value: Double) { /* ... */ }
+  init(value: Int) { /* ... */ } // expected-note{{candidate has partially matching parameter list (value: Int)}}
+  init(value: Double) { /* ... */ } // expected-note{{candidate has partially matching parameter list (value: Double)}}
 }
 
 class TestNestedExpr {