[Sema] Remove separate closure type-checking logic

`participatesInInference` is now always true for
a non-empty body, remove it along with the separate
type-checking logic such that empty bodies are
type-checked together with the context.

Author: hamishknight

include/swift/AST/AnyFunctionRef.h

@@ -130,7 +130,7 @@ class AnyFunctionRef {
     auto *ACE = TheFunction.get<AbstractClosureExpr *>();
     if (auto *CE = dyn_cast<ClosureExpr>(ACE)) {
       CE->setBody(stmt);
-      CE->setBodyState(ClosureExpr::BodyState::ReadyForTypeChecking);
+      CE->setBodyState(ClosureExpr::BodyState::Parsed);
       return;
     }
 
@@ -146,7 +146,7 @@ class AnyFunctionRef {
     auto *ACE = TheFunction.get<AbstractClosureExpr *>();
     if (auto *CE = dyn_cast<ClosureExpr>(ACE)) {
       CE->setBody(stmt);
-      CE->setBodyState(ClosureExpr::BodyState::TypeCheckedWithSignature);
+      CE->setBodyState(ClosureExpr::BodyState::TypeChecked);
       return;
     }
 

include/swift/AST/Expr.h

@@ -4119,20 +4119,11 @@ class ClosureExpr : public AbstractClosureExpr {
 
 public:
   enum class BodyState {
-    /// The body was parsed, but not ready for type checking because
-    /// the closure parameters haven't been type checked.
+    /// The body was parsed.
     Parsed,
 
-    /// The type of the closure itself was type checked. But the body has not
-    /// been type checked yet.
-    ReadyForTypeChecking,
-
-    /// The body was typechecked with the enclosing closure.
-    /// i.e. single expression closure or result builder closure.
-    TypeCheckedWithSignature,
-
-    /// The body was type checked separately from the enclosing closure.
-    SeparatelyTypeChecked,
+    /// The body was type-checked.
+    TypeChecked,
   };
 
 private:
@@ -4370,13 +4361,6 @@ class ClosureExpr : public AbstractClosureExpr {
     ExplicitResultTypeAndBodyState.setInt(v);
   }
 
-  /// Whether this closure's body is/was type checked separately from its
-  /// enclosing expression.
-  bool isSeparatelyTypeChecked() const {
-    return getBodyState() == BodyState::SeparatelyTypeChecked ||
-           getBodyState() == BodyState::ReadyForTypeChecking;
-  }
-
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::Closure;
   }

include/swift/Sema/ConstraintSystem.h

@@ -5687,11 +5687,6 @@ class ConstraintSystem {
   /// imported from C/ObjectiveC.
   bool isArgumentOfImportedDecl(ConstraintLocatorBuilder locator);
 
-  /// Check whether given closure should participate in inference e.g.
-  /// if it's a single-expression closure - it always does, but
-  /// multi-statement closures require special flags.
-  bool participatesInInference(ClosureExpr *closure) const;
-
   /// Visit each subexpression that will be part of the constraint system
   /// of the given expression, including those in closure bodies that will be
   /// part of the constraint system.

lib/Sema/CSApply.cpp

@@ -8853,28 +8853,21 @@ namespace {
         // Note that in this mode `ClosuresToTypeCheck` acts
         // as a stack because multi-statement closures could
         // have other multi-statement closures in the body.
-        if (cs.participatesInInference(closure)) {
-          hadError |= cs.applySolutionToBody(
-              solution, closure, Rewriter.dc,
-              [&](SyntacticElementTarget target) {
-                auto resultTarget = rewriteTarget(target);
-                if (resultTarget) {
-                  if (auto expr = resultTarget->getAsExpr())
-                    solution.setExprTypes(expr);
-                }
-
-                return resultTarget;
-              });
+        hadError |= cs.applySolutionToBody(
+            solution, closure, Rewriter.dc, [&](SyntacticElementTarget target) {
+              auto resultTarget = rewriteTarget(target);
+              if (resultTarget) {
+                if (auto expr = resultTarget->getAsExpr())
+                  solution.setExprTypes(expr);
+              }
 
-          if (!hadError) {
-            TypeChecker::checkClosureAttributes(closure);
-            TypeChecker::checkParameterList(closure->getParameters(), closure);
-          }
+              return resultTarget;
+            });
 
-          continue;
+        if (!hadError) {
+          TypeChecker::checkClosureAttributes(closure);
+          TypeChecker::checkParameterList(closure->getParameters(), closure);
         }
-
-        hadError |= TypeChecker::typeCheckClosureBody(closure);
       }
 
       return hadError;

lib/Sema/CSGen.cpp

@@ -2555,10 +2555,7 @@ namespace {
         // If this is a multi-statement closure, let's mark result
         // as potential hole right away.
         return Type(CS.createTypeVariable(
-            resultLocator,
-            (!CS.participatesInInference(closure) || allowResultBindToHole)
-                ? TVO_CanBindToHole
-                : 0));
+            resultLocator, allowResultBindToHole ? TVO_CanBindToHole : 0));
       }();
 
       // Determine the isolation of the closure.

lib/Sema/CSSyntacticElement.cpp

@@ -1171,18 +1171,6 @@ class SyntacticElementConstraintGenerator
         for (const auto &capture : captureList->getCaptureList())
           visitPatternBinding(capture.PBD, elements);
       }
-
-      // Let's not walk into the body if empty or multi-statement closure
-      // doesn't participate in inference.
-      if (!cs.participatesInInference(closure)) {
-        // Although the body doesn't participate in inference we still
-        // want to type-check captures to make sure that the context
-        // is valid.
-        if (captureList)
-          createConjunction(elements, locator);
-
-        return;
-      }
     }
 
     if (isChildOf(StmtKind::Case)) {
@@ -2598,7 +2586,6 @@ SolutionApplicationToFunctionResult ConstraintSystem::applySolution(
 
   // Otherwise, we need to delay type checking of the closure until later.
   solution.setExprTypes(closure);
-  closure->setBodyState(ClosureExpr::BodyState::ReadyForTypeChecking);
   return SolutionApplicationToFunctionResult::Delay;
 }
 

lib/Sema/ConstraintSystem.cpp

@@ -7432,16 +7432,6 @@ bool ConstraintSystem::isArgumentGenericFunction(Type argType, Expr *argExpr) {
   return false;
 }
 
-bool ConstraintSystem::participatesInInference(ClosureExpr *closure) const {
-  if (getAppliedResultBuilderTransform(closure))
-    return true;
-
-  if (closure->hasEmptyBody())
-    return false;
-
-  return true;
-}
-
 ProtocolConformanceRef
 ConstraintSystem::lookupConformance(Type type, ProtocolDecl *protocol) {
   auto cacheKey = std::make_pair(type.getPointer(), protocol);

lib/Sema/TypeCheckAvailability.cpp

@@ -1822,11 +1822,8 @@ static void findAvailabilityFixItNodes(
   InnermostAncestorFinder::MatchPredicate IsGuardable =
       [](ASTNode Node, ASTWalker::ParentTy Parent) {
         if (Expr *ParentExpr = Parent.getAsExpr()) {
-          auto *ParentClosure = dyn_cast<ClosureExpr>(ParentExpr);
-          if (!ParentClosure ||
-              ParentClosure->isSeparatelyTypeChecked()) {
+          if (!isa<ClosureExpr>(ParentExpr))
             return false;
-          }
         } else if (auto *ParentStmt = Parent.getAsStmt()) {
           if (!isa<BraceStmt>(ParentStmt)) {
             return false;

lib/Sema/TypeCheckConstraints.cpp

@@ -2363,10 +2363,6 @@ void ConstraintSystem::forEachExpr(
       if (!NewE)
         return Action::Stop();
 
-      if (auto closure = dyn_cast<ClosureExpr>(E)) {
-        if (!CS.participatesInInference(closure))
-          return Action::SkipNode(NewE);
-      }
       return Action::Continue(NewE);
     }
 

lib/Sema/TypeCheckStmt.cpp

@@ -104,12 +104,7 @@ namespace {
 
       if (auto CE = dyn_cast<ClosureExpr>(E)) {
         CE->setParent(ParentDC);
-
-        // If the closure was type checked within its enclosing context,
-        // we need to walk into it. Otherwise, it'll have been separately
-        // type-checked.
-        if (!CE->isSeparatelyTypeChecked())
-          CE->getBody()->walk(ContextualizeClosuresAndMacros(CE));
+        CE->getBody()->walk(ContextualizeClosuresAndMacros(CE));
 
         TypeChecker::computeCaptures(CE);
         return Action::SkipNode(E);
@@ -283,18 +278,14 @@ namespace {
         if(CE->getRawDiscriminator() == ClosureExpr::InvalidDiscriminator)
           CE->setDiscriminator(NextClosureDiscriminator++);
 
-        // If the closure was type checked within its enclosing context,
-        // we need to walk into it with a new sequence.
-        // Otherwise, it'll have been separately type-checked.
-        if (!CE->isSeparatelyTypeChecked()) {
-          SetLocalDiscriminators innerVisitor;
-          if (auto params = CE->getParameters()) {
-            for (auto *param : *params) {
-              innerVisitor.setLocalDiscriminator(param);
-            }
+        // We need to walk into closure bodies with a new sequence.
+        SetLocalDiscriminators innerVisitor;
+        if (auto params = CE->getParameters()) {
+          for (auto *param : *params) {
+            innerVisitor.setLocalDiscriminator(param);
           }
-          CE->getBody()->walk(innerVisitor);
         }
+        CE->getBody()->walk(innerVisitor);
 
         return Action::SkipNode(E);
       }
@@ -2978,25 +2969,6 @@ TypeCheckFunctionBodyRequest::evaluate(Evaluator &eval,
   return hadError ? errorBody() : body;
 }
 
-bool TypeChecker::typeCheckClosureBody(ClosureExpr *closure) {
-  TypeChecker::checkClosureAttributes(closure);
-  TypeChecker::checkParameterList(closure->getParameters(), closure);
-
-  BraceStmt *body = closure->getBody();
-
-  std::optional<FunctionBodyTimer> timer;
-  const auto &tyOpts = closure->getASTContext().TypeCheckerOpts;
-  if (tyOpts.DebugTimeFunctionBodies || tyOpts.WarnLongFunctionBodies)
-    timer.emplace(closure);
-
-  bool HadError = StmtChecker(closure).typeCheckBody(body);
-  if (body) {
-    closure->setBody(body);
-  }
-  closure->setBodyState(ClosureExpr::BodyState::SeparatelyTypeChecked);
-  return HadError;
-}
-
 bool TypeChecker::typeCheckTapBody(TapExpr *expr, DeclContext *DC) {
   // We intentionally use typeCheckStmt instead of typeCheckBody here
   // because we want to contextualize TapExprs with the body they're in.

lib/Sema/TypeChecker.h

@@ -463,8 +463,6 @@ std::optional<BraceStmt *> applyResultBuilderBodyTransform(FuncDecl *func,
 /// Find the return statements within the body of the given function.
 std::vector<ReturnStmt *> findReturnStatements(AnyFunctionRef fn);
 
-bool typeCheckClosureBody(ClosureExpr *closure);
-
 bool typeCheckTapBody(TapExpr *expr, DeclContext *DC);
 
 Type typeCheckParameterDefault(Expr *&defaultValue, DeclContext *DC,

unittests/Sema/ConstraintSimplificationTests.cpp

@@ -109,6 +109,7 @@ TEST_F(SemaTest, TestClosureInferenceFromOptionalContext) {
   cs.setClosureType(closure, defaultTy);
 
   auto *closureTy = cs.createTypeVariable(closureLoc, /*options=*/0);
+  cs.setType(closure, closureTy);
 
   cs.addUnsolvedConstraint(Constraint::create(
       cs, ConstraintKind::FallbackType, closureTy, defaultTy,

validation-test/Sema/SwiftUI/rdar88256059.swift

@@ -10,7 +10,6 @@ struct MyView: View {
   var body: some View {
     Table(self.data) {
       // expected-error@-1 {{expected expression in result builder 'TableColumnBuilder'}} {{23-23=<#result#>}}
-      // expected-error@-2 {{cannot infer return type of empty closure}} {{23-23=<#result#>}}
     }
   }
 }