[Constraint system] Make solver less expression-centric, part N/M

lib/Sema/BuilderTransform.cpp

@@ -1107,8 +1107,14 @@ Optional<BraceStmt *> TypeChecker::applyFunctionBuilderBodyTransform(
   }
 
   // Apply the solution to the function body.
-  return cast_or_null<BraceStmt>(
-     cs.applySolutionToBody(solutions.front(), func));
+  if (auto result = cs.applySolution(
+          solutions.front(),
+          SolutionApplicationTarget(func),
+          /*performingDiagnostics=*/false)) {
+    return result->getFunctionBody();
+  }
+
+  return nullptr;
 }
 
 ConstraintSystem::TypeMatchResult ConstraintSystem::matchFunctionBuilder(

lib/Sema/CSApply.cpp

@@ -7231,14 +7231,14 @@ bool ConstraintSystem::applySolutionFixes(const Solution &solution) {
 
 /// Apply a given solution to the expression, producing a fully
 /// type-checked expression.
-llvm::PointerUnion<Expr *, Stmt *> ConstraintSystem::applySolutionImpl(
-    Solution &solution, SolutionApplicationTarget target, Type convertType,
-    bool discardedExpr, bool performingDiagnostics) {
+Optional<SolutionApplicationTarget> ConstraintSystem::applySolution(
+    Solution &solution, SolutionApplicationTarget target,
+    bool performingDiagnostics) {
   // If any fixes needed to be applied to arrive at this solution, resolve
   // them to specific expressions.
   if (!solution.Fixes.empty()) {
     if (shouldSuppressDiagnostics())
-      return nullptr;
+      return None;
 
     bool diagnosedErrorsViaFixes = applySolutionFixes(solution);
     // If all of the available fixes would result in a warning,
@@ -7248,22 +7248,26 @@ llvm::PointerUnion<Expr *, Stmt *> ConstraintSystem::applySolutionImpl(
         })) {
       // If we already diagnosed any errors via fixes, that's it.
       if (diagnosedErrorsViaFixes)
-        return nullptr;
+        return None;
 
       // If we didn't manage to diagnose anything well, so fall back to
       // diagnosing mining the system to construct a reasonable error message.
       diagnoseFailureFor(target);
-      return nullptr;
+      return None;
     }
   }
 
   ExprRewriter rewriter(*this, solution, shouldSuppressDiagnostics());
   ExprWalker walker(rewriter);
 
   // Apply the solution to the target.
-  llvm::PointerUnion<Expr *, Stmt *> result;
+  SolutionApplicationTarget result = target;
   if (auto expr = target.getAsExpr()) {
-    result = expr->walk(walker);
+    Expr *rewrittenExpr = expr->walk(walker);
+    if (!rewrittenExpr)
+      return None;
+
+    result.setExpr(rewrittenExpr);
   } else {
     auto fn = *target.getAsFunction();
 
@@ -7284,14 +7288,11 @@ llvm::PointerUnion<Expr *, Stmt *> ConstraintSystem::applySolutionImpl(
         });
 
     if (!newBody)
-      return result;
+      return None;
 
-    result = newBody;
+    result.setFunctionBody(newBody);
   }
 
-  if (result.isNull())
-    return result;
-
   // If we're re-typechecking an expression for diagnostics, don't
   // visit closures that have non-single expression bodies.
   if (!performingDiagnostics) {
@@ -7309,15 +7310,17 @@ llvm::PointerUnion<Expr *, Stmt *> ConstraintSystem::applySolutionImpl(
 
     // If any of them failed to type check, bail.
     if (hadError)
-      return nullptr;
+      return None;
   }
 
-  if (auto resultExpr = result.dyn_cast<Expr *>()) {
+  if (auto resultExpr = result.getAsExpr()) {
     Expr *expr = target.getAsExpr();
     assert(expr && "Can't have expression result without expression target");
+
     // We are supposed to use contextual type only if it is present and
     // this expression doesn't represent the implicit return of the single
     // expression function which got deduced to be `Never`.
+    Type convertType = target.getExprConversionType();
     auto shouldCoerceToContextualType = [&]() {
       return convertType &&
           !(getType(resultExpr)->isUninhabited() &&
@@ -7328,19 +7331,22 @@ llvm::PointerUnion<Expr *, Stmt *> ConstraintSystem::applySolutionImpl(
     // If we're supposed to convert the expression to some particular type,
     // do so now.
     if (shouldCoerceToContextualType()) {
-      result = rewriter.coerceToType(resultExpr, convertType,
-                                     getConstraintLocator(expr));
-      if (!result)
-        return nullptr;
-    } else if (getType(resultExpr)->hasLValueType() && !discardedExpr) {
+      resultExpr = rewriter.coerceToType(resultExpr,
+                                         simplifyType(convertType),
+                                         getConstraintLocator(expr));
+    } else if (getType(resultExpr)->hasLValueType() &&
+               !target.isDiscardedExpr()) {
       // We referenced an lvalue. Load it.
-      result = rewriter.coerceToType(resultExpr,
-                                     getType(resultExpr)->getRValueType(),
-                                     getConstraintLocator(expr));
+      resultExpr = rewriter.coerceToType(resultExpr,
+                                         getType(resultExpr)->getRValueType(),
+                                         getConstraintLocator(expr));
     }
 
-    if (resultExpr)
-      solution.setExprTypes(resultExpr);
+    if (!resultExpr)
+      return None;
+
+    solution.setExprTypes(resultExpr);
+    result.setExpr(resultExpr);
   }
 
   rewriter.finalize();
@@ -7509,16 +7515,21 @@ ArrayRef<Solution> SolutionResult::getAmbiguousSolutions() const {
 
 MutableArrayRef<Solution> SolutionResult::takeAmbiguousSolutions() && {
   assert(getKind() == Ambiguous);
+  markAsDiagnosed();
   return MutableArrayRef<Solution>(solutions, numSolutions);
 }
 
-llvm::PointerUnion<Expr *, Stmt *> SolutionApplicationTarget::walk(
-    ASTWalker &walker) {
+SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) {
   switch (kind) {
-  case Kind::expression:
-    return getAsExpr()->walk(walker);
+  case Kind::expression: {
+    SolutionApplicationTarget result = *this;
+    result.setExpr(getAsExpr()->walk(walker));
+    return result;
+  }
 
   case Kind::function:
-    return getAsFunction()->getBody()->walk(walker);
+    return SolutionApplicationTarget(
+        *getAsFunction(),
+        cast_or_null<BraceStmt>(getFunctionBody()->walk(walker)));
   }
 }

lib/Sema/CSSolver.cpp

@@ -1074,22 +1074,22 @@ void ConstraintSystem::shrink(Expr *expr) {
   }
 }
 
-static bool debugConstraintSolverForExpr(ASTContext &C, Expr *expr) {
+static bool debugConstraintSolverForTarget(
+   ASTContext &C, SolutionApplicationTarget target) {
   if (C.TypeCheckerOpts.DebugConstraintSolver)
     return true;
 
   if (C.TypeCheckerOpts.DebugConstraintSolverOnLines.empty())
     // No need to compute the line number to find out it's not present.
     return false;
 
-  // Get the lines on which the expression starts and ends.
+  // Get the lines on which the target starts and ends.
   unsigned startLine = 0, endLine = 0;
-  if (expr->getSourceRange().isValid()) {
-    auto range =
-      Lexer::getCharSourceRangeFromSourceRange(C.SourceMgr,
-                                               expr->getSourceRange());
-    startLine = C.SourceMgr.getLineNumber(range.getStart());
-    endLine = C.SourceMgr.getLineNumber(range.getEnd());
+  SourceRange range = target.getSourceRange();
+  if (range.isValid()) {
+    auto charRange = Lexer::getCharSourceRangeFromSourceRange(C.SourceMgr, range);
+    startLine = C.SourceMgr.getLineNumber(charRange.getStart());
+    endLine = C.SourceMgr.getLineNumber(charRange.getEnd());
   }
 
   assert(startLine <= endLine && "expr ends before it starts?");
@@ -1107,25 +1107,44 @@ static bool debugConstraintSolverForExpr(ASTContext &C, Expr *expr) {
   return startBound != endBound;
 }
 
-bool ConstraintSystem::solve(Expr *&expr,
-                             Type convertType,
-                             ExprTypeCheckListener *listener,
-                             SmallVectorImpl<Solution> &solutions,
-                             FreeTypeVariableBinding allowFreeTypeVariables) {
+/// If we aren't certain that we've emitted a diagnostic, emit a fallback
+/// diagnostic.
+static void maybeProduceFallbackDiagnostic(
+    ConstraintSystem &cs, SolutionApplicationTarget target) {
+  if (cs.Options.contains(ConstraintSystemFlags::SubExpressionDiagnostics) ||
+      cs.Options.contains(ConstraintSystemFlags::SuppressDiagnostics))
+    return;
+
+  // Before producing fatal error here, let's check if there are any "error"
+  // diagnostics already emitted or waiting to be emitted. Because they are
+  // a better indication of the problem.
+  ASTContext &ctx = cs.getASTContext();
+  if (ctx.Diags.hadAnyError() || ctx.hasDelayedConformanceErrors())
+    return;
+
+  ctx.Diags.diagnose(target.getLoc(), diag::failed_to_produce_diagnostic);
+}
+
+Optional<std::vector<Solution>> ConstraintSystem::solve(
+    SolutionApplicationTarget &target,
+    ExprTypeCheckListener *listener,
+    FreeTypeVariableBinding allowFreeTypeVariables
+) {
   llvm::SaveAndRestore<bool> debugForExpr(
       getASTContext().TypeCheckerOpts.DebugConstraintSolver,
-      debugConstraintSolverForExpr(getASTContext(), expr));
+      debugConstraintSolverForTarget(getASTContext(), target));
 
   /// Dump solutions for debugging purposes.
-  auto dumpSolutions = [&] {
+  auto dumpSolutions = [&](const SolutionResult &result) {
     // Debug-print the set of solutions.
     if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {
       auto &log = getASTContext().TypeCheckerDebug->getStream();
-      if (solutions.size() == 1) {
+      if (result.getKind() == SolutionResult::Success) {
         log << "---Solution---\n";
-        solutions[0].dump(log);
-      } else {
-        for (unsigned i = 0, e = solutions.size(); i != e; ++i) {
+        result.getSolution().dump(log);
+      } else if (result.getKind() == SolutionResult::Ambiguous) {
+        auto solutions = result.getAmbiguousSolutions();
+        for (unsigned i : indices(solutions)) {
           log << "--- Solution #" << i << " ---\n";
           solutions[i].dump(log);
         }
@@ -1135,59 +1154,62 @@ bool ConstraintSystem::solve(Expr *&expr,
 
   // Take up to two attempts at solving the system. The first attempts to
   // solve a system that is expected to be well-formed, the second kicks in
-  // when there is an error and attempts to salvage an ill-formed expression.
+  // when there is an error and attempts to salvage an ill-formed program.
   for (unsigned stage = 0; stage != 2; ++stage) {
     auto solution = (stage == 0)
-        ? solveImpl(expr, convertType, listener, allowFreeTypeVariables)
+        ? solveImpl(target, listener, allowFreeTypeVariables)
         : salvage();
 
     switch (solution.getKind()) {
-    case SolutionResult::Success:
+    case SolutionResult::Success: {
       // Return the successful solution.
-      solutions.clear();
-      solutions.push_back(std::move(solution).takeSolution());
-      dumpSolutions();
-      return false;
+      dumpSolutions(solution);
+      std::vector<Solution> result;
+      result.push_back(std::move(solution).takeSolution());
+      return std::move(result);
+    }
 
     case SolutionResult::Error:
-      return true;
+      maybeProduceFallbackDiagnostic(*this, target);
+      return None;
 
     case SolutionResult::TooComplex:
-      getASTContext().Diags.diagnose(expr->getLoc(), diag::expression_too_complex)
-          .highlight(expr->getSourceRange());
+      getASTContext().Diags.diagnose(
+          target.getLoc(), diag::expression_too_complex)
+            .highlight(target.getSourceRange());
       solution.markAsDiagnosed();
-      return true;
+      return None;
 
     case SolutionResult::Ambiguous:
       // If salvaging produced an ambiguous result, it has already been
       // diagnosed.
       if (stage == 1) {
         solution.markAsDiagnosed();
-        return true;
+        return None;
       }
 
       if (Options.contains(
             ConstraintSystemFlags::AllowUnresolvedTypeVariables)) {
+        dumpSolutions(solution);
         auto ambiguousSolutions = std::move(solution).takeAmbiguousSolutions();
-        solutions.assign(std::make_move_iterator(ambiguousSolutions.begin()),
-                         std::make_move_iterator(ambiguousSolutions.end()));
-        dumpSolutions();
-        solution.markAsDiagnosed();
-        return false;
+        std::vector<Solution> result(
+            std::make_move_iterator(ambiguousSolutions.begin()),
+            std::make_move_iterator(ambiguousSolutions.end()));
+        return std::move(result);
       }
 
       LLVM_FALLTHROUGH;
 
     case SolutionResult::UndiagnosedError:
       if (shouldSuppressDiagnostics()) {
         solution.markAsDiagnosed();
-        return true;
+        return None;
       }
 
       if (stage == 1) {
-        diagnoseFailureFor(expr);
+        diagnoseFailureFor(target);
         solution.markAsDiagnosed();
-        return true;
+        return None;
       }
 
       // Loop again to try to salvage.
@@ -1200,14 +1222,13 @@ bool ConstraintSystem::solve(Expr *&expr,
 }
 
 SolutionResult
-ConstraintSystem::solveImpl(Expr *&expr,
-                            Type convertType,
+ConstraintSystem::solveImpl(SolutionApplicationTarget &target,
                             ExprTypeCheckListener *listener,
                             FreeTypeVariableBinding allowFreeTypeVariables) {
   if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {
     auto &log = getASTContext().TypeCheckerDebug->getStream();
-    log << "---Constraint solving for the expression at ";
-    auto R = expr->getSourceRange();
+    log << "---Constraint solving at ";
+    auto R = target.getSourceRange();
     if (R.isValid()) {
       R.print(log, getASTContext().SourceMgr, /*PrintText=*/ false);
     } else {
@@ -1219,6 +1240,7 @@ ConstraintSystem::solveImpl(Expr *&expr,
   assert(!solverState && "cannot be used directly");
 
   // Set up the expression type checker timer.
+  Expr *expr = target.getAsExpr();
   Timer.emplace(expr, *this);
 
   Expr *origExpr = expr;
@@ -1232,14 +1254,12 @@ ConstraintSystem::solveImpl(Expr *&expr,
   if (auto generatedExpr = generateConstraints(expr, DC))
     expr = generatedExpr;
   else {
-    if (listener)
-      listener->constraintGenerationFailed(expr);
     return SolutionResult::forError();
   }
 
   // If there is a type that we're expected to convert to, add the conversion
   // constraint.
-  if (convertType) {
+  if (Type convertType = target.getExprConversionType()) {
     // Determine whether we know more about the contextual type.
     ContextualTypePurpose ctp = CTP_Unused;
     bool isOpaqueReturnType = false;
@@ -1286,6 +1306,8 @@ ConstraintSystem::solveImpl(Expr *&expr,
   if (getExpressionTooComplex(solutions))
     return SolutionResult::forTooComplex();
 
+  target.setExpr(expr);
+
   switch (solutions.size()) {
   case 0:
     return SolutionResult::forUndiagnosedError();