Merge pull request #9963 from xedin/rdar-32204609

[ConstraintSolver] Skip generic overloads only if non-generic choices produce higher score solutions

Author: xedin

lib/Sema/CSRanking.cpp

@@ -83,6 +83,10 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
     case SK_KeyPathSubscript:
       log << "key path subscript";
       break;
+
+    case SK_StringToPointerConversion:
+      log << "string-to-pointer conversion";
+      break;
     }
     log << ")\n";
   }

lib/Sema/CSSimplify.cpp

@@ -4397,6 +4397,8 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
     // If we haven't resolved the element type, generate constraints.
     if (baseType2->isTypeVariableOrMember()) {
       if (flags.contains(TMF_GenerateConstraints)) {
+        increaseScore(SK_StringToPointerConversion);
+
         auto int8Con = Constraint::create(*this, ConstraintKind::Bind,
                                        baseType2, TC.getInt8Type(DC),
                                        getConstraintLocator(locator));
@@ -4418,6 +4420,8 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
     if (!isStringCompatiblePointerBaseType(TC, DC, baseType2)) {
       return SolutionKind::Error;
     }
+
+    increaseScore(SK_StringToPointerConversion);
     return SolutionKind::Solved;
   }
 

lib/Sema/CSSolver.cpp

@@ -2480,8 +2480,9 @@ bool ConstraintSystem::solveSimplified(
   auto afterDisjunction = InactiveConstraints.erase(disjunction);
   CG.removeConstraint(disjunction);
 
+  Score initialScore = CurrentScore;
   Optional<DisjunctionChoice> lastSolvedChoice;
-  Optional<DisjunctionChoice> firstNonGenericOperatorSolution;
+  Optional<Score> bestNonGenericScore;
 
   ++solverState->NumDisjunctions;
   auto constraints = disjunction->getNestedConstraints();
@@ -2507,9 +2508,13 @@ bool ConstraintSystem::solveSimplified(
     //        already have a solution involving non-generic operators,
     //        but continue looking for a better non-generic operator
     //        solution.
-    if (firstNonGenericOperatorSolution &&
-        currentChoice.isGenericOperatorOrUnavailable())
-      continue;
+    if (bestNonGenericScore && currentChoice.isGenericOperatorOrUnavailable()) {
+      // If non-generic solution increased the score by applying any
+      // fixes or restrictions to the solution, let's not skip generic
+      // overloads because they could produce a better solution.
+      if (bestNonGenericScore <= initialScore)
+        continue;
+    }
 
     // We already have a solution; check whether we should
     // short-circuit the disjunction.
@@ -2542,11 +2547,12 @@ bool ConstraintSystem::solveSimplified(
       DisjunctionChoices.push_back({locator, index});
     }
 
-    if (currentChoice.solve(solutions, allowFreeTypeVariables)) {
-      if (!firstNonGenericOperatorSolution &&
-          !currentChoice.isGenericOperatorOrUnavailable() &&
-          currentChoice.isSymmetricOperator())
-        firstNonGenericOperatorSolution = currentChoice;
+    if (auto score = currentChoice.solve(solutions, allowFreeTypeVariables)) {
+      if (!currentChoice.isGenericOperatorOrUnavailable() &&
+          currentChoice.isSymmetricOperator()) {
+        if (!bestNonGenericScore || score < bestNonGenericScore)
+          bestNonGenericScore = score;
+      }
 
       lastSolvedChoice = currentChoice;
 
@@ -2578,10 +2584,12 @@ bool ConstraintSystem::solveSimplified(
   return tooComplex || !lastSolvedChoice;
 }
 
-bool DisjunctionChoice::solve(SmallVectorImpl<Solution> &solutions,
-                              FreeTypeVariableBinding allowFreeTypeVariables) {
+Optional<Score>
+DisjunctionChoice::solve(SmallVectorImpl<Solution> &solutions,
+                         FreeTypeVariableBinding allowFreeTypeVariables) {
   CS->simplifyDisjunctionChoice(Choice);
-  return !CS->solveRec(solutions, allowFreeTypeVariables);
+  bool failed = CS->solveRec(solutions, allowFreeTypeVariables);
+  return failed ? None : Optional<Score>(CS->CurrentScore);
 }
 
 bool DisjunctionChoice::isGenericOperatorOrUnavailable() const {

lib/Sema/ConstraintSystem.h

@@ -416,8 +416,10 @@ enum ScoreKind {
   SK_EmptyExistentialConversion,
   /// A key path application subscript.
   SK_KeyPathSubscript,
-  
-  SK_LastScoreKind = SK_KeyPathSubscript,
+  // A conversion from a string to a pointer.
+  SK_StringToPointerConversion,
+
+  SK_LastScoreKind = SK_StringToPointerConversion,
 };
 
 /// The number of score kinds.
@@ -2671,8 +2673,8 @@ class DisjunctionChoice {
   bool isSymmetricOperator() const;
 
   /// \brief Apply given choice to the system and try to solve it.
-  bool solve(SmallVectorImpl<Solution> &solutions,
-             FreeTypeVariableBinding allowFreeTypeVariables);
+  Optional<Score> solve(SmallVectorImpl<Solution> &solutions,
+                        FreeTypeVariableBinding allowFreeTypeVariables);
 
 private:
   static ValueDecl *getOperatorDecl(Constraint *constraint) {

test/Constraints/overload.swift

@@ -222,3 +222,4 @@ func curry<F, S, T, R>(_ f: @escaping (F, S, T) -> R) -> (F) -> (S) -> (T) -> R
 // Ensure that we consider these unambiguous
 let _ = curry(+)(1)
 let _ = [0].reduce(0, +)
+let _ = curry(+)("string vs. pointer")

validation-test/Sema/rdar32204609.swift

@@ -0,0 +1,11 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: %target-build-swift %s -o %t/a.out
+// RUN: %target-run %t/a.out
+
+// REQUIRES: executable_test
+
+let x: Int! = nil
+let y: Int! = 1
+
+print(x == y)