[Diagnostics] Add CSFix::diagnoseForAmbiguity for diagnosing common

fixes that appear in all solutions.

Author: hborla

lib/Sema/CSFix.cpp

@@ -479,6 +479,24 @@ bool DefineMemberBasedOnUse::diagnose(bool asNote) const {
   return AlreadyDiagnosed || failure.diagnose(asNote);
 }
 
+bool
+DefineMemberBasedOnUse::diagnoseForAmbiguity(ArrayRef<Solution> 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,

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<Solution> 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<Solution> 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<Solution> solutions) const override {
+    return diagnose();
+  }
+
   static AddMissingArguments *create(ConstraintSystem &cs,
                                      llvm::ArrayRef<Param> synthesizedArgs,
                                      ConstraintLocator *locator);
@@ -1149,6 +1157,10 @@ class RemoveExtraneousArguments final
 
   bool diagnose(bool asNote = false) const override;
 
+  bool diagnoseForAmbiguity(ArrayRef<Solution> 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<Solution> 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<Solution> solutions) const override {
+    return diagnose();
+  }
+
   static IgnoreContextualType *create(ConstraintSystem &cs, Type resultTy,
                                       Type specifiedTy,
                                       ConstraintLocator *locator);

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();

test/Constraints/closures.swift

@@ -186,7 +186,7 @@ func testMap() {
 }
 
 // <rdar://problem/22414757> "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}}
 
 
 

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<Element> {}
+struct MyArray<Element> {} // expected-note {{'Element' declared as parameter to type 'MyArray'}}
 class A {
     var a: MyArray<Int>
     init() {
-        a = MyArray<Int // expected-error{{binary operator '<' cannot be applied to operands of type 'MyArray<_>.Type' and 'Int.Type'}}
-	// expected-note @-1 {{overloads for '<' exist with these partially matching parameter lists:}}
+        a = MyArray<Int // expected-error {{generic parameter 'Element' could not be inferred}}
+        // expected-note@-1 {{explicitly specify the generic arguments to fix this issue}}
     }
 }
 

test/Sema/diag_ambiguous_overloads.swift

@@ -12,8 +12,8 @@ func fe(_: Int, _: Int) {}
 fe(E.baz) // expected-error {{type 'E' has no member 'baz'; did you mean 'bar'?}}
 fe(.baz) // expected-error {{reference to member 'baz' cannot be resolved without a contextual type}}
 
-// FIXME: maybe complain about .nope also?
-fe(.nope, .nyet) // expected-error {{reference to member 'nyet' cannot be resolved without a contextual type}}
+fe(.nope, .nyet) // expected-error {{type 'Int' has no member 'nope'}}
+// expected-error@-1 {{reference to member 'nyet' cannot be resolved without a contextual type}}
 
 func fg<T>(_ f: (T) -> T) -> Void {} // expected-note {{in call to function 'fg'}}
 fg({x in x}) // expected-error {{generic parameter 'T' could not be inferred}}

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}}