Merge pull request #64334 from ahoppen/ahoppen/dont-precheck-twice

[Sema] Don't abort the result builder transform if the builder contained an ErrorExpr

Author: ahoppen

lib/Sema/BuilderTransform.cpp

@@ -2462,8 +2462,11 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,
     return getTypeMatchSuccess();
   }
 
-  // Pre-check the body: pre-check any expressions in it and look
-  // for return statements.
+  // We have already pre-checked the result builder body. Technically, we
+  // shouldn't need to do anything here, but there was a bug here that we did
+  // not apply the result builder transform if it contained an explicit return.
+  // To maintain source compatibility, we still need to check for HasReturnStmt.
+  // https://github.com/apple/swift/issues/64332.
   auto request =
       PreCheckResultBuilderRequest{{fn, /*SuppressDiagnostics=*/false}};
   switch (evaluateOrDefault(getASTContext().evaluator, request,
@@ -2473,16 +2476,10 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,
     break;
 
   case ResultBuilderBodyPreCheck::Error: {
-    InvalidResultBuilderBodies.insert(fn);
-
-    if (!shouldAttemptFixes())
-      return getTypeMatchFailure(locator);
-
-    if (recordFix(IgnoreInvalidResultBuilderBody::create(
-            *this, getConstraintLocator(fn.getAbstractClosureExpr()))))
-      return getTypeMatchFailure(locator);
-
-    return getTypeMatchSuccess();
+    llvm_unreachable(
+        "Running PreCheckResultBuilderRequest on a function shouldn't run "
+        "preCheckExpression and thus we should never enter this case.");
+    break;
   }
 
   case ResultBuilderBodyPreCheck::HasReturnStmt:
@@ -2800,8 +2797,11 @@ class PreCheckResultBuilderApplication : public ASTWalker {
 
 ResultBuilderBodyPreCheck PreCheckResultBuilderRequest::evaluate(
     Evaluator &evaluator, PreCheckResultBuilderDescriptor owner) const {
+  // Closures should already be pre-checked when we run this, so there's no need
+  // to pre-check them again.
+  bool skipPrecheck = owner.Fn.getAbstractClosureExpr();
   return PreCheckResultBuilderApplication(
-             owner.Fn, /*skipPrecheck=*/false,
+             owner.Fn, skipPrecheck,
              /*suppressDiagnostics=*/owner.SuppressDiagnostics)
       .run();
 }

test/Constraints/rdar65320500.swift

@@ -3,7 +3,7 @@
 struct Result {}
 
 @resultBuilder
-struct Builder {
+struct Builder { // expected-note 4 {{add 'buildOptional(_:)' to the result builder 'Builder' to add support for 'if' statements without an 'else'}}
   static func buildBlock() -> Result {
     Result()
   }
@@ -12,10 +12,12 @@ struct Builder {
 func test_builder<T>(@Builder _: () -> T) {}
 func test_builder(@Builder _: () -> Int) {}
 
-test_builder {
+test_builder { // expected-error {{no exact matches in call to global function 'test_builder'}}
   let _ = 0
 
-  if let x = does_not_exist { // expected-error {{cannot find 'does_not_exist' in scope}}
+  // expected-error@+2 {{cannot find 'does_not_exist' in scope}}
+  // expected-note@+1 2 {{closure containing control flow statement cannot be used with result builder 'Builder'}}
+  if let x = does_not_exist {
   }
 }
 
@@ -28,9 +30,10 @@ test_builder {
   test(totalseconds / 3600) // expected-error {{cannot find 'totalseconds' in scope; did you mean 'totalSeconds'?}}
 }
 
-test_builder {
+test_builder { // expected-error {{no exact matches in call to global function 'test_builder'}}
   test(doesntExist()) // expected-error {{cannot find 'doesntExist' in scope}}
 
+  // expected-note@+1 2 {{closure containing control flow statement cannot be used with result builder 'Builder'}}
   if let result = doesntExist() { // expected-error {{cannot find 'doesntExist' in scope}}
   }
 

test/Constraints/result_builder_diags.swift

@@ -5,8 +5,9 @@ enum Either<T,U> {
   case second(U)
 }
 
-@resultBuilder
-struct TupleBuilder { // expected-note {{struct 'TupleBuilder' declared here}}
+// expected-note @+2 {{add 'buildArray(_:)' to the result builder 'TupleBuilder' to add support for 'for'..'in' loops}}
+// expected-note @+1 2 {{struct 'TupleBuilder' declared here}}
+@resultBuilder struct TupleBuilder {
   static func buildBlock() -> () { }
 
   static func buildBlock<T1>(_ t1: T1) -> T1 {
@@ -78,7 +79,7 @@ struct TupleBuilderWithoutIf { // expected-note 3{{struct 'TupleBuilderWithoutIf
   static func buildDo<T>(_ value: T) -> T { return value }
 }
 
-func tuplify<T>(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { // expected-note 2{{in call to function 'tuplify(_:body:)'}}
+func tuplify<T>(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { // expected-note {{'tuplify(_:body:)' declared here}}
   print(body(cond))
 }
 
@@ -88,9 +89,9 @@ func tuplifyWithoutIf<T>(_ cond: Bool, @TupleBuilderWithoutIf body: (Bool) -> T)
 
 func testDiags() {
   // For loop
-  tuplify(true) { _ in // expected-error {{generic parameter 'T' could not be inferred}}
+  tuplify(true) { _ in
     17
-    for c in name {
+    for c in name { // expected-error {{closure containing control flow statement cannot be used with result builder 'TupleBuilder'}}
     // expected-error@-1 {{cannot find 'name' in scope}}
     }
   }
@@ -464,7 +465,7 @@ struct TestConstraintGenerationErrors {
   }
 
   func buildTupleClosure() {
-    tuplify(true) { _ in // expected-error {{generic parameter 'T' could not be inferred}}
+    tuplify(true) { _ in
       let a = nothing // expected-error {{cannot find 'nothing' in scope}}
       String(nothing) // expected-error {{cannot find 'nothing' in scope}}
     }
@@ -732,7 +733,7 @@ struct TuplifiedStructWithInvalidClosure {
 
   @TupleBuilder var nestedErrorsDiagnosedByParser: some Any {
     tuplify(true) { _ in
-      tuplify { _ in
+      tuplify { _ in // expected-error {{missing argument for parameter #1 in call}}
         self. // expected-error {{expected member name following '.'}}
       }
       42

test/IDE/complete_ambiguous.swift

@@ -424,11 +424,10 @@ CreateThings {
     }
 }
 
-// FIXME: No results in multi-statement closure with erroreous sibling result builder element
 CreateThings {
     Thing { point in
       print("hello")
-      point.#^MULTICLOSURE_FUNCBUILDER_FIXME?check=NORESULTS^#
+      point.#^MULTICLOSURE_FUNCBUILDER_FIXME?check=POINT_MEMBER^#
     }
     Thing. // ErrorExpr
 }

test/IDE/complete_literal.swift

@@ -1,13 +1,5 @@
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL1 | %FileCheck %s -check-prefix=LITERAL1
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL2 | %FileCheck %s -check-prefix=LITERAL2
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL3 | %FileCheck %s -check-prefix=LITERAL3
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL4 | %FileCheck %s -check-prefix=LITERAL4
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL5 | %FileCheck %s -check-prefix=LITERAL5
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL6 | %FileCheck %s -check-prefix=LITERAL6
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL7 | %FileCheck %s -check-prefix=LITERAL7
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL8 | %FileCheck %s -check-prefix=LITERAL8
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL9 | %FileCheck %s -check-prefix=LITERAL9
-// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=LITERAL10 | %FileCheck %s -check-prefix=LITERAL10
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t
 
 {
   1.#^LITERAL1^#

test/IDE/complete_value_expr.swift

@@ -1204,9 +1204,6 @@ func testTypeCheckWithUnsolvedVariables3() {
 // TC_UNSOLVED_VARIABLES_3-DAG: Decl[InstanceMethod]/CurrNominal: addString({#(s): String#})[#BuilderStyle<Int>#]{{; name=.+$}}
 // TC_UNSOLVED_VARIABLES_3-DAG: Decl[InstanceMethod]/CurrNominal: add({#(t): Int#})[#BuilderStyle<Int>#]{{; name=.+$}}
 // TC_UNSOLVED_VARIABLES_3-DAG: Decl[InstanceMethod]/CurrNominal: get()[#Int#]{{; name=.+$}}
-// TC_UNSOLVED_VARIABLES_3-DAG: Keyword[self]/CurrNominal: self[#BuilderStyle<Double>#]{{; name=.+$}}
-// TC_UNSOLVED_VARIABLES_3-DAG: Decl[InstanceMethod]/CurrNominal: addString({#(s): String#})[#BuilderStyle<Double>#]{{; name=.+$}}
-// TC_UNSOLVED_VARIABLES_3-DAG: Decl[InstanceMethod]/CurrNominal: add({#(t): Double#})[#BuilderStyle<Double>#]{{; name=.+$}}
 // TC_UNSOLVED_VARIABLES_3: End completions
 
 func testTypeCheckNil() {