[CSStep] Properly finalize component step without follow-up steps

Currently finalization e.g. scope reset and solution minimization
is only done if component step had follow-up e.g. type variable or
disjunction step(s), but it should be done if `take` generated any
fixes as well, or component changed score in any way, otherwise
we might miss some solutions with fixes because "best score" haven't
been reset properly.

Author: xedin

lib/Sema/CSStep.cpp

@@ -277,7 +277,7 @@ StepResult ComponentStep::take(bool prevFailed) {
   // allowed in the solution.
   if (!CS.solverState->allowsFreeTypeVariables() && CS.hasFreeTypeVariables()) {
     if (!CS.shouldAttemptFixes())
-      return done(/*isSuccess=*/false);
+      return finalize(/*isSuccess=*/false);
 
     // Let's see if all of the free type variables are associated with
     // generic parameters and if so, let's default them to `Any` and continue
@@ -298,7 +298,7 @@ StepResult ComponentStep::take(bool prevFailed) {
 
       auto *anchor = locator->getAnchor();
       if (!(anchor && locator->isForGenericParameter()))
-        return done(/*isSuccess=*/false);
+        return finalize(/*isSuccess=*/false);
 
       // Increment the score for every missing generic argument
       // to make ranking of the solutions with different number
@@ -324,14 +324,14 @@ StepResult ComponentStep::take(bool prevFailed) {
       auto *fix =
           ExplicitlySpecifyGenericArguments::create(CS, missingParams, locator);
       if (CS.recordFix(fix))
-        return done(/*isSuccess=*/false);
+        return finalize(/*isSuccess=*/false);
     }
   }
 
   // If this solution is worse than the best solution we've seen so far,
   // skip it.
   if (CS.worseThanBestSolution())
-    return done(/*isSuccess=*/false);
+    return finalize(/*isSuccess=*/false);
 
   // If we only have relational or member constraints and are allowing
   // free type variables, save the solution.
@@ -341,7 +341,7 @@ StepResult ComponentStep::take(bool prevFailed) {
     case ConstraintClassification::Member:
       continue;
     default:
-      return done(/*isSuccess=*/false);
+      return finalize(/*isSuccess=*/false);
     }
   }
 
@@ -350,31 +350,31 @@ StepResult ComponentStep::take(bool prevFailed) {
     getDebugLogger() << "(found solution " << getCurrentScore() << ")\n";
 
   Solutions.push_back(std::move(solution));
-  return done(/*isSuccess=*/true);
+  return finalize(/*isSuccess=*/true);
 }
 
-StepResult ComponentStep::resume(bool prevFailed) {
+StepResult ComponentStep::finalize(bool isSuccess) {
+  // If this was a single component, there is nothing to be done,
+  // because it represents the whole constraint system at some
+  // point of the solver path.
+  if (IsSingle)
+    return done(isSuccess);
+
   // Rewind all modifications done to constraint system.
   ComponentScope.reset();
 
-  if (!IsSingle && isDebugMode()) {
+  if (isDebugMode()) {
     auto &log = getDebugLogger();
-    log << (prevFailed ? "failed" : "finished") << " component #" << Index
+    log << (isSuccess ? "finished" : "failed") << " component #" << Index
         << ")\n";
   }
 
   // If we came either back to this step and previous
   // (either disjunction or type var) failed, it means
   // that component as a whole has failed.
-  if (prevFailed)
+  if (!isSuccess)
     return done(/*isSuccess=*/false);
 
-  // If this was a single component, there is nothing to be done,
-  // because it represents the whole constraint system at some
-  // point of the solver path.
-  if (IsSingle)
-    return done(/*isSuccess=*/true);
-
   assert(!Solutions.empty() && "No Solutions?");
 
   // For each of the partial solutions, subtract off the current score.

lib/Sema/CSStep.h

@@ -374,7 +374,8 @@ class ComponentStep final : public SolverStep {
   }
 
   StepResult take(bool prevFailed) override;
-  StepResult resume(bool prevFailed) override;
+
+  StepResult resume(bool prevFailed) override { return finalize(!prevFailed); }
 
   // The number of disjunction constraints associated with this component.
   unsigned disjunctionCount() const { return NumDisjunctions; }
@@ -398,6 +399,10 @@ class ComponentStep final : public SolverStep {
     // let's return it to the graph.
     CS.CG.setOrphanedConstraint(OrphanedConstraint);
   }
+
+  /// Finalize current component by either cleanup if sub-tasks
+  /// have failed, or solution generation and minimization.
+  StepResult finalize(bool isSuccess);
 };
 
 template <typename P> class BindingStep : public SolverStep {

test/Constraints/generics.swift

@@ -224,7 +224,7 @@ func test9215114<T: P19215114, U: Q19215114>(_ t: T) -> (U) -> () {
 }
 
 // <rdar://problem/21718970> QoI: [uninferred generic param] cannot invoke 'foo' with an argument list of type '(Int)'
-class Whatever<A: Numeric, B: Numeric> {  // expected-note 2 {{'A' declared as parameter to type 'Whatever'}}
+class Whatever<A: Numeric, B: Numeric> {  // expected-note 2 {{'A' declared as parameter to type 'Whatever'}} expected-note {{'B' declared as parameter to type 'Whatever'}}
   static func foo(a: B) {}
 
   static func bar() {}
@@ -233,7 +233,9 @@ class Whatever<A: Numeric, B: Numeric> {  // expected-note 2 {{'A' declared as p
 Whatever.foo(a: 23) // expected-error {{generic parameter 'A' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{9-9=<<#A: Numeric#>, Int>}}
 
 // <rdar://problem/21718955> Swift useless error: cannot invoke 'foo' with no arguments
-Whatever.bar()  // expected-error {{generic parameter 'A' could not be inferred}} expected-note {{explicitly specify the generic arguments to fix this issue}} {{9-9=<<#A: Numeric#>, <#B: Numeric#>>}}
+// TODO(diagnostics): We should try to produce a single note in this case.
+Whatever.bar()  // expected-error {{generic parameter 'A' could not be inferred}} expected-note 2 {{explicitly specify the generic arguments to fix this issue}} {{9-9=<<#A: Numeric#>, <#B: Numeric#>>}}
+// expected-error@-1 {{generic parameter 'B' could not be inferred}}
 
 // <rdar://problem/27515965> Type checker doesn't enforce same-type constraint if associated type is Any
 protocol P27515965 {

test/decl/typealias/generic.swift

@@ -65,6 +65,8 @@ typealias C<T> = MyType<String, T>
 typealias D<T1, T2, T3> = MyType<T2, T1>  // expected-note 3 {{'T3' declared as parameter to type 'D'}}
 
 typealias E<T1, T2> = Int  // expected-note {{generic type 'E' declared here}}
+// expected-note@-1 {{'T1' declared as parameter to type 'E'}}
+// expected-note@-2 {{'T2' declared as parameter to type 'E'}}
 
 typealias F<T1, T2> = (T1) -> T2
 
@@ -73,7 +75,9 @@ typealias G<S1, S2> = A<S1, S2>
 
 let _ : E<Int, Float> = 42
 let _ : E<Float> = 42   // expected-error {{generic type 'E' specialized with too few type parameters (got 1, but expected 2)}}
-let _ : E = 42   // expected-error {{cannot convert value of type 'Int' to specified type 'E'}}
+let _ : E = 42
+// expected-error@-1 {{generic parameter 'T1' could not be inferred}}
+// expected-error@-2 {{generic parameter 'T2' could not be inferred}}
 let _ : D = D(a: 1, b: 2)
 // expected-error@-1 {{generic parameter 'T3' could not be inferred}}
 // expected-note@-2 {{explicitly specify the generic arguments to fix this issue}} {{14-14=<Int, Int, Any>}}