Merge pull request #6801 from xedin/keypath-crashers

[QoI] Cleanup AST after trying to shrink constraint system of invalid expression

Author: slavapestov

lib/Sema/CSSolver.cpp

@@ -1449,53 +1449,77 @@ ConstraintSystem::solveSingle(FreeTypeVariableBinding allowFreeTypeVariables) {
 }
 
 bool ConstraintSystem::Candidate::solve() {
-  // Cleanup after constraint system generation/solving,
-  // because it would assign types to expressions, which
-  // might interfere with solving higher-level expressions.
-  ExprCleaner cleaner(E);
+  // For cleanup to work correctly e.g. `CleanupIllFormedExpressionRAII`
+  // `E` has to be 'reference to pointer' but, in this situation, it can't
+  // because we don't want to accidently modify AST while trying to reduce
+  // type domains of sub-expressions. Because constraint generation _might_
+  // produce completely different expression let's use a locally scoped
+  // 'reference to pointer' variable instead of `E` which enables both
+  // proper constraint generation/solving and cleanup.
+  auto *&expr = E;
+  {
+    // Cleanup after constraint system generation/solving,
+    // because it would assign types to expressions, which
+    // might interfere with solving higher-level expressions.
+    ExprCleaner cleaner(expr);
+    CleanupIllFormedExpressionRAII cleanup(TC.Context, expr);
+
+    // Allocate new constraint system for sub-expression.
+    ConstraintSystem cs(TC, DC, None);
+
+    // Generate constraints for the new system.
+    if (auto generatedExpr = cs.generateConstraints(expr)) {
+      expr = generatedExpr;
+    } else {
+      // Failure to generate constraint system for sub-expression
+      // means we can't continue solving sub-expressions.
+      return true;
+    }
 
-  // Allocate new constraint system for sub-expression.
-  ConstraintSystem cs(TC, DC, None);
+    // If there is contextual type present, add an explicit "conversion"
+    // constraint to the system.
+    if (!CT.isNull()) {
+      auto constraintKind = ConstraintKind::Conversion;
+      if (CTP == CTP_CallArgument)
+        constraintKind = ConstraintKind::ArgumentConversion;
 
-  // Generate constraints for the new system.
-  if (auto generatedExpr = cs.generateConstraints(E)) {
-    E = generatedExpr;
-  } else {
-    // Failure to generate constraint system for sub-expression means we can't
-    // continue solving sub-expressions.
-    return true;
-  }
-
-  // If there is contextual type present, add an explicit "conversion"
-  // constraint to the system.
-  if (!CT.isNull()) {
-    auto constraintKind = ConstraintKind::Conversion;
-    if (CTP == CTP_CallArgument)
-      constraintKind = ConstraintKind::ArgumentConversion;
+      cs.addConstraint(constraintKind, cs.getType(expr), CT,
+                       cs.getConstraintLocator(expr), /*isFavored=*/true);
+    }
 
-    cs.addConstraint(constraintKind, cs.getType(E), CT,
-                     cs.getConstraintLocator(E), /*isFavored=*/true);
-  }
+    // Try to solve the system and record all available solutions.
+    llvm::SmallVector<Solution, 2> solutions;
+    {
+      SolverState state(cs);
 
-  // Try to solve the system and record all available solutions.
-  llvm::SmallVector<Solution, 2> solutions;
-  {
-    SolverState state(cs);
+      // Use solveRec() instead of solve() in here, because solve()
+      // would try to deduce the best solution, which we don't
+      // really want. Instead, we want the reduced set of domain choices.
+      cs.solveRec(solutions, FreeTypeVariableBinding::Allow);
+    }
 
-    // Use solveRec() instead of solve() in here, because solve()
-    // would try to deduce the best solution, which we don't
-    // really want. Instead, we want the reduced set of domain choices.
-    cs.solveRec(solutions, FreeTypeVariableBinding::Allow);
-  }
+    // No solutions for the sub-expression means that either main expression
+    // needs salvaging or it's inconsistent (read: doesn't have solutions).
+    if (solutions.empty())
+      return true;
 
-  // No solutions for the sub-expression means that either main expression
-  // needs salvaging or it's inconsistent (read: doesn't have solutions).
-  if (solutions.empty())
-    return true;
+    // Record found solutions as suggestions.
+    this->applySolutions(solutions);
+
+    // Solution application is going to modify types of sub-expresssions,
+    // so we need to avoid clean-up afterwords, but let's double-check
+    // if we have any implicit expressions with type variables and
+    // nullify their types.
+    cleanup.disable();
+    expr->forEachChildExpr([&](Expr *childExpr) -> Expr * {
+      Type type = childExpr->getType();
+      if (childExpr->isImplicit() && type && type->hasTypeVariable())
+        childExpr->setType(Type());
+      return childExpr;
+    });
 
-  // Record found solutions as suggestions.
-  this->applySolutions(solutions);
-  return false;
+    return false;
+  }
 }
 
 void ConstraintSystem::Candidate::applySolutions(

validation-test/compiler_crashers/28593-unreachable-executed-at-swift-lib-ast-type-cpp-3771.swift renamed to validation-test/compiler_crashers_fixed/28593-unreachable-executed-at-swift-lib-ast-type-cpp-3771.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 [{_=#keyPath(t>w

validation-test/compiler_crashers/28623-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift renamed to validation-test/compiler_crashers_fixed/28623-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 f#keyPath(n&_==a>c{{{{{{{{{{{{{{{{{_=b:{{{{c{{{{{{d

validation-test/compiler_crashers/28631-unreachable-executed-at-swift-lib-ast-type-cpp-1130.swift renamed to validation-test/compiler_crashers_fixed/28631-unreachable-executed-at-swift-lib-ast-type-cpp-1130.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 [{{{{{{{{{{{{{{0=#keyPath(n&_=d

validation-test/compiler_crashers/28632-unreachable-executed-at-swift-lib-ast-type-cpp-1130.swift renamed to validation-test/compiler_crashers_fixed/28632-unreachable-executed-at-swift-lib-ast-type-cpp-1130.swift

@@ -5,9 +5,8 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
 // REQUIRES: asserts
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 
 {func a>
 print(a==#keyPath(a{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{

validation-test/compiler_crashers/28635-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift renamed to validation-test/compiler_crashers_fixed/28635-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift

@@ -5,7 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 func a|Set(#keyPath(t>a>a{

validation-test/compiler_crashers/28639-unreachable-executed-at-swift-lib-ast-type-cpp-1337.swift renamed to validation-test/compiler_crashers_fixed/28639-unreachable-executed-at-swift-lib-ast-type-cpp-1337.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 print([{#keyPath(a}(t>A

validation-test/compiler_crashers/28640-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift renamed to validation-test/compiler_crashers_fixed/28640-swift-tupletype-get-llvm-arrayref-swift-tupletypeelt-swift-astcontext-const.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 protocol a func a|Set(#keyPath(t>a{

validation-test/compiler_crashers/28641-result-case-not-implemented.swift renamed to validation-test/compiler_crashers_fixed/28641-result-case-not-implemented.swift

@@ -5,7 +5,6 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
 // REQUIRES: asserts
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 {let β=b&[{{{#keyPath(n&[{{{{{{{{{{{{{{{{{{{{{{{{{{a{{{{s

validation-test/compiler_crashers/28642-swift-optionaltype-get-swift-type.swift renamed to validation-test/compiler_crashers_fixed/28642-swift-optionaltype-get-swift-type.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 Array(_==#keyPath(t>Void!

validation-test/compiler_crashers/28645-swift-type-transform-llvm-function-ref-swift-type-swift-type-const.swift renamed to validation-test/compiler_crashers_fixed/28645-swift-type-transform-llvm-function-ref-swift-type-swift-type-const.swift

@@ -5,6 +5,5 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 (.n).h.n&[(#keyPath(t>A

validation-test/compiler_crashers/28646-swift-lvaluetype-get-swift-type.swift renamed to validation-test/compiler_crashers_fixed/28646-swift-lvaluetype-get-swift-type.swift

@@ -5,8 +5,7 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: deterministic-behavior
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+// RUN: not %target-swift-frontend %s -emit-ir
 #keyPath(a
 print(Int
 print(_=#keyPath(a