[ConstraintSystem] Improve handling of ambiguities related to fixes

Solutions passed to `diagnoseAmbiguityWithFixes` aren't filtered
so we need to remove all of the solutions with the score worse
than overall "best". Also logic has to account for some fixes being
"warnings".

Author: xedin

lib/Sema/ConstraintSystem.cpp

@@ -2583,6 +2583,18 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
   if (!applyExpr)
     return;
 
+  auto isNameOfStandardComparisonOperator = [](StringRef opName) -> bool {
+    return opName == "==" || opName == "!=" || opName == "===" ||
+           opName == "!==" || opName == "<" || opName == ">" ||
+           opName == "<=" || opName == ">=";
+  };
+
+  auto isEnumWithAssociatedValues = [](Type type) -> bool {
+    if (auto *enumType = type->getAs<EnumType>())
+      return !enumType->getDecl()->hasOnlyCasesWithoutAssociatedValues();
+    return false;
+  };
+
   const auto &solution = solutions.front();
   if (auto *binaryOp = dyn_cast<BinaryExpr>(applyExpr)) {
     auto *lhs = binaryOp->getArg()->getElement(0);
@@ -2596,6 +2608,14 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
                   operatorName.str(), lhsType)
           .highlight(lhs->getSourceRange())
           .highlight(rhs->getSourceRange());
+
+      if (isNameOfStandardComparisonOperator(operatorName.str()) &&
+          isEnumWithAssociatedValues(lhsType)) {
+        DE.diagnose(applyExpr->getLoc(),
+                    diag::no_binary_op_overload_for_enum_with_payload,
+                    operatorName.str());
+        return;
+      }
     } else {
       DE.diagnose(anchor->getLoc(), diag::cannot_apply_binop_to_args,
                   operatorName.str(), lhsType, rhsType)
@@ -2624,16 +2644,35 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
           FunctionType::getParamListAsString(fnType->getParams()));
   }
 
+  // All of the overload choices had generic parameters like `Self`.
+  if (parameters.empty())
+    return;
+
   DE.diagnose(anchor->getLoc(), diag::suggest_partial_overloads,
               /*isResult=*/false, operatorName.str(),
               llvm::join(parameters, ", "));
 }
 
 bool ConstraintSystem::diagnoseAmbiguityWithFixes(
-    ArrayRef<Solution> solutions) {
+    SmallVectorImpl<Solution> &solutions) {
   if (solutions.empty())
     return false;
 
+  if (auto bestScore = solverState->BestScore) {
+    solutions.erase(llvm::remove_if(solutions,
+                                    [&](const Solution &solution) {
+                                      return solution.getFixedScore() >
+                                             *bestScore;
+                                    }),
+                    solutions.end());
+
+    if (llvm::all_of(solutions, [&](const Solution &solution) {
+          auto score = solution.getFixedScore();
+          return score.Data[SK_Fix] == 0 && solution.Fixes.empty();
+        }))
+      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

lib/Sema/ConstraintSystem.h

@@ -2147,7 +2147,7 @@ class ConstraintSystem {
   void diagnoseFailureForExpr(Expr *expr);
 
   bool diagnoseAmbiguity(ArrayRef<Solution> solutions);
-  bool diagnoseAmbiguityWithFixes(ArrayRef<Solution> solutions);
+  bool diagnoseAmbiguityWithFixes(SmallVectorImpl<Solution> &solutions);
 
   /// Give the deprecation warning for referring to a global function
   /// when there's a method from a conditional conformance in a smaller/closer

test/Constraints/closures.swift

@@ -717,7 +717,7 @@ func rdar37790062() {
 }
 
 // <rdar://problem/39489003>
-typealias KeyedItem<K, T> = (key: K, value: T) // expected-note {{'T' declared as parameter to type 'KeyedItem'}}
+typealias KeyedItem<K, T> = (key: K, value: T)
 
 protocol Node {
   associatedtype T
@@ -731,7 +731,7 @@ extension Node {
   func getChild(for key:K)->(key: K, value: T) {
     return children.first(where: { (item:KeyedItem) -> Bool in
         return item.key == key
-        // expected-error@-1 {{generic parameter 'T' could not be inferred}}
+        // expected-error@-1 {{binary operator '==' cannot be applied to two 'Self.K' operands}}
       })!
   }
 }

test/Generics/deduction.swift

@@ -7,7 +7,7 @@
 func identity<T>(_ value: T) -> T { return value }
 
 func identity2<T>(_ value: T) -> T { return value }
-// expected-note@-1 {{candidate expects value of type 'X' for parameter #1}}
+// expected-note@-1 {{'identity2' produces 'Y', not the expected contextual result type 'X'}}
 func identity2<T>(_ value: T) -> Int { return 0 }
 // expected-note@-1 {{'identity2' produces 'Int', not the expected contextual result type 'X'}}
 

test/Interpreter/SDK/misc_osx.swift

@@ -7,8 +7,7 @@ import CoreServices
 func testFSEventStreamRef(stream: FSEventStreamRef) {
   // FIXME: These should be distinct types, constructible from one another.
   _ = stream as ConstFSEventStreamRef // works by coincidence because both are currently OpaquePointer
-  _ = ConstFSEventStreamRef(stream) // expected-error {{cannot invoke initializer for type 'ConstFSEventStreamRef' with an argument list of type '(FSEventStreamRef)'}}
-  // expected-note @-1 {{overloads for 'ConstFSEventStreamRef' exist with these partially matching parameter lists:}}
+  _ = ConstFSEventStreamRef(stream) // expected-error {{no exact matches in call to initializer}}
 
   // This is not a CF object.
   FSEventStreamRetain(stream) // no-warning

test/Sema/struct_equatable_hashable.swift

@@ -115,7 +115,6 @@ struct StructWithoutExplicitConformance {
 
 func structWithoutExplicitConformance() {
   if StructWithoutExplicitConformance(a: 1, b: "b") == StructWithoutExplicitConformance(a: 2, b: "a") { } // expected-error{{binary operator '==' cannot be applied to two 'StructWithoutExplicitConformance' operands}}
-  // expected-note @-1 {{overloads for '==' exist with these partially matching parameter lists: }}
 }
 
 // Structs with non-hashable/equatable stored properties don't derive conformance.