Merge pull request swiftlang#38770 from xedin/rdar-56167233

[ConstraintSystem] Relax the left-over information check in `ComponentStep`

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -2147,6 +2147,11 @@ class ConstraintSystem {
   /// explored.
   size_t MaxMemory = 0;
 
+  /// Flag to indicate to the solver that the system is in invalid
+  /// state and it shouldn't proceed but instead produce a fallback
+  /// diagnostic.
+  bool InvalidState = false;
+
   /// Cached member lookups.
   llvm::DenseMap<std::pair<Type, DeclNameRef>, Optional<LookupResult>>
     MemberLookups;
@@ -2646,6 +2651,11 @@ class ConstraintSystem {
     Phase = newPhase;
   }
 
+  /// Check whether constraint system is in valid state e.g.
+  /// has left-over active/inactive constraints which should
+  /// have been simplified.
+  bool inInvalidState() const { return InvalidState; }
+
   /// Cache the types of the given expression and all subexpressions.
   void cacheExprTypes(Expr *expr) {
     bool excludeRoot = false;

lib/Sema/CSSolver.cpp

@@ -422,6 +422,11 @@ ConstraintSystem::SolverState::~SolverState() {
          "Expected constraint system to have this solver state!");
   CS.solverState = nullptr;
 
+  // If constraint system ended up being in an invalid state
+  // let's just drop the state without attempting to rollback.
+  if (CS.inInvalidState())
+    return;
+
   // Make sure that all of the retired constraints have been returned
   // to constraint system.
   assert(!hasRetiredConstraints());
@@ -513,6 +518,10 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
 }
 
 ConstraintSystem::SolverScope::~SolverScope() {
+  // Don't attempt to rollback from an incorrect state.
+  if (cs.inInvalidState())
+    return;
+
   // Erase the end of various lists.
   while (cs.TypeVariables.size() > numTypeVariables)
     cs.TypeVariables.pop_back();
@@ -1384,6 +1393,13 @@ void ConstraintSystem::solveImpl(SmallVectorImpl<Solution> &solutions) {
   if (failedConstraint)
     return;
 
+  // Attempt to solve a constraint system already in an invalid
+  // state should be immediately aborted.
+  if (inInvalidState()) {
+    solutions.clear();
+    return;
+  }
+
   // Allocate new solver scope, so constraint system
   // could be restored to its original state afterwards.
   // Otherwise there is a risk that some of the constraints
@@ -1426,6 +1442,14 @@ void ConstraintSystem::solveImpl(SmallVectorImpl<Solution> &solutions) {
     // or error, which means that current path is inconsistent.
     {
       auto result = advance(step.get(), prevFailed);
+
+      // If execution of this step let constraint system in an
+      // invalid state, let's drop all of the solutions and abort.
+      if (inInvalidState()) {
+        solutions.clear();
+        return;
+      }
+
       switch (result.getKind()) {
       // It was impossible to solve this step, let's note that
       // for followup steps, to propogate the error.

lib/Sema/CSStep.cpp

@@ -362,22 +362,36 @@ StepResult ComponentStep::take(bool prevFailed) {
     return finalize(/*isSuccess=*/false);
   }
 
+  auto printConstraints = [&](const ConstraintList &constraints) {
+    for (auto &constraint : constraints)
+      constraint.print(getDebugLogger(), &CS.getASTContext().SourceMgr);
+  };
+
   // If we don't have any disjunction or type variable choices left, we're done
   // solving. Make sure we don't have any unsolved constraints left over, using
-  // report_fatal_error to make sure we trap in release builds instead of
-  // potentially miscompiling.
+  // report_fatal_error to make sure we trap in debug builds and fail the step
+  // in release builds.
   if (!CS.ActiveConstraints.empty()) {
-    CS.print(llvm::errs());
-    llvm::report_fatal_error("Active constraints left over?");
+    if (CS.isDebugMode()) {
+      getDebugLogger() << "(failed due to remaining active constraints:\n";
+      printConstraints(CS.ActiveConstraints);
+      getDebugLogger() << ")\n";
+    }
+
+    CS.InvalidState = true;
+    return finalize(/*isSuccess=*/false);
   }
+
   if (!CS.solverState->allowsFreeTypeVariables()) {
     if (!CS.InactiveConstraints.empty()) {
-      CS.print(llvm::errs());
-      llvm::report_fatal_error("Inactive constraints left over?");
-    }
-    if (CS.hasFreeTypeVariables()) {
-      CS.print(llvm::errs());
-      llvm::report_fatal_error("Free type variables left over?");
+      if (CS.isDebugMode()) {
+        getDebugLogger() << "(failed due to remaining inactive constraints:\n";
+        printConstraints(CS.InactiveConstraints);
+        getDebugLogger() << ")\n";
+      }
+
+      CS.InvalidState = true;
+      return finalize(/*isSuccess=*/false);
     }
   }
 

lib/Sema/ConstraintGraph.cpp

@@ -37,7 +37,14 @@ using namespace constraints;
 ConstraintGraph::ConstraintGraph(ConstraintSystem &cs) : CS(cs) { }
 
 ConstraintGraph::~ConstraintGraph() {
-  assert(Changes.empty() && "Scope stack corrupted");
+#ifndef NDEBUG
+  // If constraint system is in an invalid state, it's
+  // possible that constraint graph is corrupted as well
+  // so let's not attempt to check change log.
+  if (!CS.inInvalidState())
+    assert(Changes.empty() && "Scope stack corrupted");
+#endif
+
   for (unsigned i = 0, n = TypeVariables.size(); i != n; ++i) {
     auto &impl = TypeVariables[i]->getImpl();
     delete impl.getGraphNode();
@@ -412,6 +419,11 @@ ConstraintGraphScope::ConstraintGraphScope(ConstraintGraph &CG)
 }
 
 ConstraintGraphScope::~ConstraintGraphScope() {
+  // Don't attempt to rollback if constraint system ended up
+  // in an invalid state.
+  if (CG.CS.inInvalidState())
+    return;
+
   // Pop changes off the stack until we hit the change could we had prior to
   // introducing this scope.
   assert(CG.Changes.size() >= NumChanges && "Scope stack corrupted");

lib/Sema/ConstraintSystem.cpp

@@ -5515,6 +5515,14 @@ void ConstraintSystem::diagnoseFailureFor(SolutionApplicationTarget target) {
   SWIFT_DEFER { setPhase(ConstraintSystemPhase::Finalization); };
 
   auto &DE = getASTContext().Diags;
+
+  // If constraint system is in invalid state always produce
+  // a fallback diagnostic that asks to file a bug.
+  if (inInvalidState()) {
+    DE.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic);
+    return;
+  }
+
   if (auto expr = target.getAsExpr()) {
     if (auto *assignment = dyn_cast<AssignExpr>(expr)) {
       if (isa<DiscardAssignmentExpr>(assignment->getDest()))