Merge pull request #58915 from xedin/multi-statement-closure-improvements

[ConstraintSystem] A couple of improvements to multi-statement closure handling

Author: xedin

lib/Sema/CSClosure.cpp

@@ -441,7 +441,7 @@ class SyntacticElementConstraintGenerator
 
     cs.addConstraint(
         ConstraintKind::Conversion, elementType, initType,
-        cs.getConstraintLocator(contextualLocator,
+        cs.getConstraintLocator(sequenceLocator,
                                 ConstraintLocator::SequenceElementType));
 
     // Reference the makeIterator witness.
@@ -450,9 +450,10 @@ class SyntacticElementConstraintGenerator
 
     Type makeIteratorType =
         cs.createTypeVariable(locator, TVO_CanBindToNoEscape);
-    cs.addValueWitnessConstraint(LValueType::get(sequenceType), makeIterator,
-                                 makeIteratorType, context.getAsDeclContext(),
-                                 FunctionRefKind::Compound, contextualLocator);
+    cs.addValueWitnessConstraint(
+        LValueType::get(sequenceType), makeIterator, makeIteratorType,
+        context.getAsDeclContext(), FunctionRefKind::Compound,
+        cs.getConstraintLocator(sequenceLocator, ConstraintLocator::Witness));
 
     // After successful constraint generation, let's record
     // solution application target with all relevant information.

lib/Sema/CSDiagnostics.h

@@ -1188,6 +1188,12 @@ class MissingMemberFailure : public InvalidMemberRefFailure {
       : InvalidMemberRefFailure(solution, baseType, memberName, locator) {}
 
   SourceLoc getLoc() const override {
+    auto *locator = getLocator();
+
+    if (locator->findLast<LocatorPathElt::SyntacticElement>()) {
+      return constraints::getLoc(getAnchor());
+    }
+
     // Diagnostic should point to the member instead of its base expression.
     return constraints::getLoc(getRawAnchor());
   }

lib/Sema/CSStep.cpp

@@ -942,13 +942,10 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
         // and scoring information.
         Snapshot.reset();
 
-        // Restore original scores of outer context before
-        // trying to produce a combined solution.
-        restoreOriginalScores();
-
         // Apply all of the information deduced from the
         // conjunction (up to the point of ambiguity)
         // back to the outer context and form a joined solution.
+        unsigned numSolutions = 0;
         for (auto &solution : Solutions) {
           ConstraintSystem::SolverScope scope(CS);
 
@@ -958,24 +955,47 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
           // of the constraint system, so they have to be
           // restored right afterwards because score of the
           // element does contribute to the overall score.
-          restoreOriginalScores();
+          restoreBestScore();
+          restoreCurrentScore(solution.getFixedScore());
+
+          // Transform all of the unbound outer variables into
+          // placeholders since we are not going to solve for
+          // each ambguous solution.
+          {
+            unsigned numHoles = 0;
+            for (auto *typeVar : CS.getTypeVariables()) {
+              if (!typeVar->getImpl().hasRepresentativeOrFixed()) {
+                CS.assignFixedType(
+                    typeVar, PlaceholderType::get(CS.getASTContext(), typeVar));
+                ++numHoles;
+              }
+            }
+            CS.increaseScore(SK_Hole, numHoles);
+          }
+
+          if (CS.worseThanBestSolution())
+            continue;
 
           // Note that `worseThanBestSolution` isn't checked
           // here because `Solutions` were pre-filtered, and
           // outer score is the same for all of them.
           OuterSolutions.push_back(CS.finalize());
+          ++numSolutions;
         }
 
-        return done(/*isSuccess=*/true);
+        return done(/*isSuccess=*/numSolutions > 0);
       }
 
+      auto solution = Solutions.pop_back_val();
+      auto score = solution.getFixedScore();
+
       // Restore outer type variables and prepare to solve
       // constraints associated with outer context together
       // with information deduced from the conjunction.
-      Snapshot->setupOuterContext(Solutions.pop_back_val());
+      Snapshot->setupOuterContext(std::move(solution));
 
       // Pretend that conjunction never happened.
-      restoreOuterState();
+      restoreOuterState(score);
 
       // Now that all of the information from the conjunction has
       // been applied, let's attempt to solve the outer scope.
@@ -987,10 +1007,11 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
   return take(prevFailed);
 }
 
-void ConjunctionStep::restoreOuterState() const {
+void ConjunctionStep::restoreOuterState(const Score &solutionScore) const {
   // Restore best/current score, since upcoming step is going to
   // work with outer scope in relation to the conjunction.
-  restoreOriginalScores();
+  restoreBestScore();
+  restoreCurrentScore(solutionScore);
 
   // Active all of the previously out-of-scope constraints
   // because conjunction can propagate type information up

lib/Sema/CSStep.h

@@ -962,8 +962,11 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
 
     // Restore best score only if conjunction fails because
     // successful outcome should keep a score set by `restoreOuterState`.
-    if (HadFailure)
-      restoreOriginalScores();
+    if (HadFailure) {
+      auto solutionScore = Score();
+      restoreBestScore();
+      restoreCurrentScore(solutionScore);
+    }
 
     if (OuterTimeRemaining) {
       auto anchor = OuterTimeRemaining->first;
@@ -1015,16 +1018,19 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
 
 private:
   /// Restore best and current scores as they were before conjunction.
-  void restoreOriginalScores() const {
-    CS.solverState->BestScore = BestScore;
+  void restoreCurrentScore(const Score &solutionScore) const {
     CS.CurrentScore = CurrentScore;
+    CS.increaseScore(SK_Fix, solutionScore.Data[SK_Fix]);
+    CS.increaseScore(SK_Hole, solutionScore.Data[SK_Hole]);
   }
 
+  void restoreBestScore() const { CS.solverState->BestScore = BestScore; }
+
   // Restore constraint system state before conjunction.
   //
   // Note that this doesn't include conjunction constraint
   // itself because we don't want to re-solve it.
-  void restoreOuterState() const;
+  void restoreOuterState(const Score &solutionScore) const;
 };
 
 } // end namespace constraints

test/expr/closure/multi_statement.swift

@@ -350,3 +350,18 @@ func test_no_crash_with_circular_ref_due_to_error() {
     return 0
   }
 }
+
+func test_diagnosing_on_missing_member_in_case() {
+  enum E {
+    case one
+  }
+
+  func test(_: (E) -> Void) {}
+
+  test {
+    switch $0 {
+    case .one: break
+    case .unknown: break // expected-error {{type 'E' has no member 'unknown'}}
+    }
+  }
+}