Merge pull request swiftlang#40708 from xedin/disable-multi-stmt-inference-inside-result-builder-bodies

[ConstraintSystem] SE-0326: Temporarily prevent multi-statement closure inference in result builder contexts

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -5292,6 +5292,10 @@ class ConstraintSystem {
   /// part of the constraint system.
   void forEachExpr(Expr *expr, llvm::function_ref<Expr *(Expr *)> callback);
 
+  /// Determine whether one of the parent closures the given one is nested
+  /// in (if any) has a result builder applied to its body.
+  bool isInResultBuilderContext(ClosureExpr *closure) const;
+
   SWIFT_DEBUG_DUMP;
   SWIFT_DEBUG_DUMPER(dump(Expr *));
 

lib/Sema/CSApply.cpp

@@ -8023,6 +8023,8 @@ namespace {
     /// \returns true if any part of the processing fails.
     bool processDelayed() {
       bool hadError = false;
+      auto &solution = Rewriter.solution;
+      auto &cs = solution.getConstraintSystem();
 
       while (!ClosuresToTypeCheck.empty()) {
         auto *closure = ClosuresToTypeCheck.pop_back_val();
@@ -8034,10 +8036,7 @@ namespace {
         // as a stack because multi-statement closures could
         // have other multi-statement closures in the body.
         auto &ctx = closure->getASTContext();
-        if (ctx.TypeCheckerOpts.EnableMultiStatementClosureInference) {
-          auto &solution = Rewriter.solution;
-          auto &cs = solution.getConstraintSystem();
-
+        if (cs.participatesInInference(closure)) {
           hadError |= cs.applySolutionToBody(
               solution, closure, Rewriter.dc,
               [&](SolutionApplicationTarget target) {

lib/Sema/CSClosure.cpp

@@ -512,28 +512,28 @@ class ClosureConstraintGenerator
   }
 
   void visitBreakStmt(BreakStmt *breakStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Break");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Break");
   }
 
   void visitContinueStmt(ContinueStmt *continueStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Continue");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Continue");
   }
 
   void visitDeferStmt(DeferStmt *deferStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Defer");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Defer");
   }
 
   void visitFallthroughStmt(FallthroughStmt *fallthroughStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Fallthrough");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Fallthrough");
   }
 
   void visitIfStmt(IfStmt *ifStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: If");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: If");
 
     SmallVector<ElementInfo, 4> elements;
 
@@ -562,8 +562,8 @@ class ClosureConstraintGenerator
   }
 
   void visitGuardStmt(GuardStmt *guardStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Guard");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Guard");
 
     createConjunction(cs,
                       {makeElement(guardStmt->getCondPointer(),
@@ -574,8 +574,8 @@ class ClosureConstraintGenerator
   }
 
   void visitWhileStmt(WhileStmt *whileStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Guard");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: While");
 
     createConjunction(cs,
                       {makeElement(whileStmt->getCondPointer(),
@@ -586,15 +586,15 @@ class ClosureConstraintGenerator
   }
 
   void visitDoStmt(DoStmt *doStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Do");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Do");
 
     visitBraceStmt(doStmt->getBody());
   }
 
   void visitRepeatWhileStmt(RepeatWhileStmt *repeatWhileStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: RepeatWhile");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: RepeatWhile");
 
     createConjunction(cs,
                       {makeElement(repeatWhileStmt->getCond(),
@@ -606,8 +606,8 @@ class ClosureConstraintGenerator
   }
 
   void visitPoundAssertStmt(PoundAssertStmt *poundAssertStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: PoundAssert");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: PoundAssert");
 
     createConjunction(cs,
                       {makeElement(poundAssertStmt->getCondition(),
@@ -618,8 +618,8 @@ class ClosureConstraintGenerator
   }
 
   void visitThrowStmt(ThrowStmt *throwStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Throw");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Throw");
 
     Type errType =
         cs.getASTContext().getErrorDecl()->getDeclaredInterfaceType();
@@ -641,8 +641,8 @@ class ClosureConstraintGenerator
   }
 
   void visitForEachStmt(ForEachStmt *forEachStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: ForEach");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: ForEach");
 
     auto *stmtLoc = cs.getConstraintLocator(locator);
 
@@ -679,8 +679,8 @@ class ClosureConstraintGenerator
   }
 
   void visitSwitchStmt(SwitchStmt *switchStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Switch");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Switch");
 
     auto *switchLoc = cs.getConstraintLocator(
         locator, LocatorPathElt::ClosureBodyElement(switchStmt));
@@ -705,8 +705,8 @@ class ClosureConstraintGenerator
   }
 
   void visitDoCatchStmt(DoCatchStmt *doStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: DoCatch");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: DoCatch");
 
     auto *doLoc = cs.getConstraintLocator(
         locator, LocatorPathElt::ClosureBodyElement(doStmt));
@@ -725,8 +725,8 @@ class ClosureConstraintGenerator
   }
 
   void visitCaseStmt(CaseStmt *caseStmt) {
-    if (!isSupportedMultiStatementClosure())
-      llvm_unreachable("Unsupported statement: Case");
+    assert(isSupportedMultiStatementClosure() &&
+           "Unsupported statement: Case");
 
     Type contextualTy;
 
@@ -919,6 +919,19 @@ bool ConstraintSystem::generateConstraints(ClosureExpr *closure) {
   return false;
 }
 
+bool ConstraintSystem::isInResultBuilderContext(ClosureExpr *closure) const {
+  if (!closure->hasSingleExpressionBody()) {
+    auto *DC = closure->getParent();
+    do {
+      if (auto *parentClosure = dyn_cast<ClosureExpr>(DC)) {
+        if (resultBuilderTransformed.count(parentClosure))
+          return true;
+      }
+    } while ((DC = DC->getParent()));
+  }
+  return false;
+}
+
 bool isConditionOfStmt(ConstraintLocatorBuilder locator) {
   auto last = locator.last();
   if (!(last && last->is<LocatorPathElt::Condition>()))

lib/Sema/ConstraintSystem.cpp

@@ -5930,8 +5930,13 @@ bool ConstraintSystem::participatesInInference(ClosureExpr *closure) const {
     return false;
 
   auto &ctx = closure->getASTContext();
-  return !closure->hasEmptyBody() &&
-         ctx.TypeCheckerOpts.EnableMultiStatementClosureInference;
+  if (closure->hasEmptyBody() ||
+      !ctx.TypeCheckerOpts.EnableMultiStatementClosureInference)
+    return false;
+
+  // If body is nested in a parent that has a function builder applied,
+  // let's prevent inference until result builders.
+  return !isInResultBuilderContext(closure);
 }
 
 TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings)

validation-test/Sema/SwiftUI/mixed_swiftui_and_multistatement_closures.swift

@@ -0,0 +1,87 @@
+// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5 -experimental-multi-statement-closures
+// REQUIRES: objc_interop
+// REQUIRES: OS=macosx
+
+import SwiftUI
+
+enum Status {
+  case complete
+  case waiting
+}
+
+struct Item : Hashable {
+  var question: String
+  var answer: Int
+}
+
+func transform(_ v: Int) -> String? { return String(v) }
+func transform(_ v: Double) -> String? { return String(v) }
+
+struct MyView : View {
+  var status: Status
+
+  var items: [Item] = []
+
+  var currItem: Item {
+    get { fatalError() }
+    nonmutating set { }
+  }
+
+  var body: some View {
+    ZStack {
+      ItemsView {
+        EmptyView()
+      } results: {
+        switch (status) {
+        case .complete:
+          ForEach(items, id: \.self) { item in
+            if let question = item.question,
+               let answer = item.answer {
+              ItemView {
+                currItem.question = question
+                currItem.answer = answer
+              } content: {
+                AnswerView(title: "",
+                           color: .red,
+                           answer: transform(answer) ?? "",
+                           selected: false)
+              }
+            }
+          }
+
+        default:
+          EmptyView()
+        }
+      }
+    }
+  }
+}
+
+struct AnswerView : View {
+  var title: String
+  var color: Color
+  var answer: String
+  var selected: Bool
+
+  var body: some View {
+    EmptyView()
+  }
+}
+
+struct ItemsView<Content: View,  Results: View>: View {
+  @ViewBuilder var content: Content
+  @ViewBuilder var results: Results
+
+  var body: some View {
+    EmptyView()
+  }
+}
+
+struct ItemView<Content: View> : View {
+  var fn: () -> Void
+  @ViewBuilder var content: Content
+
+  var body: some View {
+    EmptyView()
+  }
+}