[ConstraintSolver] Skip performance hacks in presence of unavailable overloads

Limit the scope of the performance hacks which currently exist in the
solver even further by disallowing it to skip generic overlaods or stop
in case when previous solutions involve unavailable overloads, which
otherwise might lead to producing incorrect overall solutions.

Author: xedin

lib/Sema/CSDiag.cpp

@@ -7155,11 +7155,22 @@ bool FailureDiagnosis::visitInOutExpr(InOutExpr *IOE) {
 
 bool FailureDiagnosis::visitCoerceExpr(CoerceExpr *CE) {
   // Coerce the input to whatever type is specified by the CoerceExpr.
-  if (!typeCheckChildIndependently(CE->getSubExpr(),
-                                   CE->getCastTypeLoc().getType(),
-                                   CTP_CoerceOperand))
+  auto expr = typeCheckChildIndependently(CE->getSubExpr(),
+                                          CE->getCastTypeLoc().getType(),
+                                          CTP_CoerceOperand);
+  if (!expr)
     return true;
 
+  auto ref = expr->getReferencedDecl();
+  if (auto *decl = ref.getDecl()) {
+    // Without explicit coercion we might end up
+    // type-checking sub-expression as unavaible
+    // declaration, let's try to diagnose that here.
+    if (AvailableAttr::isUnavailable(decl))
+      return CS.TC.diagnoseExplicitUnavailability(
+          decl, expr->getSourceRange(), CS.DC, dyn_cast<ApplyExpr>(expr));
+  }
+
   return false;
 }
 

lib/Sema/CSSolver.cpp

@@ -1706,6 +1706,44 @@ void ConstraintSystem::collectDisjunctions(
   }
 }
 
+/// \brief Check if the given disjunction choice should be attempted by solver.
+static bool shouldSkipDisjunctionChoice(ConstraintSystem &cs,
+                                        DisjunctionChoice &choice,
+                                        Optional<Score> &bestNonGenericScore) {
+  if (choice->isDisabled()) {
+    auto &TC = cs.TC;
+    if (TC.getLangOpts().DebugConstraintSolver) {
+      auto &log = cs.getASTContext().TypeCheckerDebug->getStream();
+      log.indent(cs.solverState->depth)
+      << "(skipping ";
+      choice->print(log, &TC.Context.SourceMgr);
+      log << '\n';
+    }
+
+    return true;
+  }
+
+  // Don't attempt to solve for generic operators if we already have
+  // a non-generic solution.
+
+  // FIXME: Less-horrible but still horrible hack to attempt to
+  //        speed things up. Skip the generic operators if we
+  //        already have a solution involving non-generic operators,
+  //        but continue looking for a better non-generic operator
+  //        solution.
+  if (bestNonGenericScore && choice.isGenericOperatorOrUnavailable()) {
+    auto &score = bestNonGenericScore->Data;
+    // Let's skip generic overload choices only in case if
+    // non-generic score indicates that there were no forced
+    // unwrappings of optional(s) and no unavailable overload
+    // choices present in the solution.
+    if (score[SK_ForceUnchecked] == 0 && score[SK_Unavailable] == 0)
+      return true;
+  }
+
+  return false;
+}
+
 bool ConstraintSystem::solveSimplified(
     SmallVectorImpl<Solution> &solutions,
     FreeTypeVariableBinding allowFreeTypeVariables) {
@@ -1789,47 +1827,29 @@ bool ConstraintSystem::solveSimplified(
   auto afterDisjunction = InactiveConstraints.erase(disjunction);
   CG.removeConstraint(disjunction);
 
-  Score initialScore = CurrentScore;
-  Optional<DisjunctionChoice> lastSolvedChoice;
+  Optional<std::pair<DisjunctionChoice, Score>> lastSolvedChoice;
   Optional<Score> bestNonGenericScore;
 
   ++solverState->NumDisjunctions;
   auto constraints = disjunction->getNestedConstraints();
   // Try each of the constraints within the disjunction.
   for (auto index : indices(constraints)) {
     auto currentChoice = DisjunctionChoice(this, constraints[index]);
-    if (currentChoice.isDisabled()) {
-      if (TC.getLangOpts().DebugConstraintSolver) {
-        auto &log = getASTContext().TypeCheckerDebug->getStream();
-        log.indent(solverState->depth)
-          << "(skipping ";
-        currentChoice->print(log, &TC.Context.SourceMgr);
-        log << '\n';
-      }
+    if (shouldSkipDisjunctionChoice(*this, currentChoice, bestNonGenericScore))
       continue;
-    }
-
-    // Don't attempt to solve for generic operators if we already have
-    // a non-generic solution.
-
-    // FIXME: Less-horrible but still horrible hack to attempt to
-    //        speed things up. Skip the generic operators if we
-    //        already have a solution involving non-generic operators,
-    //        but continue looking for a better non-generic operator
-    //        solution.
-    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.
     if (lastSolvedChoice) {
-      auto *lastChoice = lastSolvedChoice->getConstraint();
-      if (shortCircuitDisjunctionAt(&currentChoice, lastChoice,
+      auto *lastChoice = lastSolvedChoice->first.getConstraint();
+      auto &score = lastSolvedChoice->second;
+      bool hasUnavailableOverloads = score.Data[SK_Unavailable] > 0;
+
+      // Attempt to short-circuit disjunction only if
+      // score indicates that there are no unavailable
+      // overload choices present in the solution.
+      if (!hasUnavailableOverloads &&
+          shortCircuitDisjunctionAt(&currentChoice, lastChoice,
                                     getASTContext()))
         break;
     }
@@ -1864,7 +1884,7 @@ bool ConstraintSystem::solveSimplified(
           bestNonGenericScore = score;
       }
 
-      lastSolvedChoice = currentChoice;
+      lastSolvedChoice = {currentChoice, *score};
 
       // If we see a tuple-to-tuple conversion that succeeded, we're done.
       // FIXME: This should be more general.
@@ -1894,8 +1914,24 @@ Optional<Score>
 DisjunctionChoice::solve(SmallVectorImpl<Solution> &solutions,
                          FreeTypeVariableBinding allowFreeTypeVariables) {
   CS->simplifyDisjunctionChoice(Choice);
-  bool failed = CS->solveRec(solutions, allowFreeTypeVariables);
-  return failed ? None : Optional<Score>(CS->CurrentScore);
+
+  if (CS->solveRec(solutions, allowFreeTypeVariables))
+    return None;
+
+  assert (!solutions.empty());
+
+  Score bestScore = solutions.front().getFixedScore();
+
+  if (solutions.size() == 1)
+    return bestScore;
+
+  for (unsigned i = 1, n = solutions.size(); i != n; ++i) {
+    auto &score = solutions[i].getFixedScore();
+    if (score < bestScore)
+      bestScore = score;
+  }
+
+  return bestScore;
 }
 
 bool DisjunctionChoice::isGenericOperatorOrUnavailable() const {

test/Constraints/diagnostics_swift4.swift

@@ -49,3 +49,9 @@ class R<K: Hashable, V> {
     dict.forEach(body)
   }
 }
+
+// Make sure that solver doesn't try to form solutions with available overloads when better generic choices are present.
+infix operator +=+ : AdditionPrecedence
+func +=+(_ lhs: Int, _ rhs: Int) -> Bool { return lhs == rhs }
+func +=+<T: BinaryInteger>(_ lhs: T, _ rhs: Int) -> Bool { return lhs == rhs }
+let _ = DoubleWidth<Int>(Int.min) - 1 +=+ Int.min // Ok