[Sema] Walk separately-checked closures normally in MiscDiagnostics

We no longer need to make any special affordances
for separately-checked closures, walk them normally.

Author: hamishknight

include/swift/AST/ASTWalker.h

@@ -634,16 +634,6 @@ class ASTWalker {
     }
   }
 
-  /// This method configures whether the walker should visit the body of a
-  /// closure that was checked separately from its enclosing expression.
-  ///
-  /// For work that is performed for every top-level expression, this should
-  /// be overridden to return false, to avoid duplicating work or visiting
-  /// bodies of closures that have not yet been type checked.
-  virtual bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *) {
-    return true;
-  }
-
   /// This method configures whether the walker should visit the body of a
   /// TapExpr.
   virtual bool shouldWalkIntoTapExpression() { return true; }

lib/AST/ASTWalker.cpp

@@ -1012,12 +1012,6 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
         return nullptr;
     }
 
-    // If the closure was separately type checked and we don't want to
-    // visit separately-checked closure bodies, bail out now.
-    if (expr->isSeparatelyTypeChecked() &&
-        !Walker.shouldWalkIntoSeparatelyCheckedClosure(expr))
-      return expr;
-
     // Handle other closures.
     if (BraceStmt *body = cast_or_null<BraceStmt>(doIt(expr->getBody()))) {
       expr->setBody(body);

lib/Sema/MiscDiagnostics.cpp

@@ -59,32 +59,6 @@ static Expr *isImplicitPromotionToOptional(Expr *E) {
   return nullptr;
 }
 
-ASTWalker::PreWalkAction BaseDiagnosticWalker::walkToDeclPre(Decl *D) {
-  return Action::VisitNodeIf(isa<ClosureExpr>(D->getDeclContext()) &&
-                             shouldWalkIntoDeclInClosureContext(D));
-}
-
-bool BaseDiagnosticWalker::shouldWalkIntoDeclInClosureContext(Decl *D) {
-  auto *closure = dyn_cast<ClosureExpr>(D->getDeclContext());
-  assert(closure);
-
-  if (closure->isSeparatelyTypeChecked())
-    return false;
-
-  // Let's not walk into declarations contained in a multi-statement
-  // closure because they'd be handled via `typeCheckDecl` that runs
-  // syntactic diagnostics.
-  if (!closure->hasSingleExpressionBody()) {
-    // Since pattern bindings get their types through solution application,
-    // `typeCheckDecl` doesn't touch initializers (because they are already
-    // fully type-checked), so pattern bindings have to be allowed to be
-    // walked to diagnose syntactic issues.
-    return isa<PatternBindingDecl>(D);
-  }
-
-  return true;
-}
-
 /// Diagnose syntactic restrictions of expressions.
 ///
 ///   - Module values may only occur as part of qualification.
@@ -1605,10 +1579,6 @@ static void diagRecursivePropertyAccess(const Expr *E, const DeclContext *DC) {
              cast<VarDecl>(DRE->getDecl())->isSelfParameter();
     }
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     bool shouldWalkCaptureInitializerExpressions() override { return true; }
 
     MacroWalking getMacroWalkingBehavior() const override {
@@ -4590,10 +4560,6 @@ static void checkStmtConditionTrailingClosure(ASTContext &ctx, const Expr *E) {
   public:
     DiagnoseWalker(ASTContext &ctx) : Ctx(ctx) { }
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     bool shouldWalkCaptureInitializerExpressions() override { return true; }
 
     MacroWalking getMacroWalkingBehavior() const override {
@@ -4719,10 +4685,6 @@ class ObjCSelectorWalker : public ASTWalker {
   ObjCSelectorWalker(const DeclContext *dc, Type selectorTy)
     : Ctx(dc->getASTContext()), DC(dc), SelectorTy(selectorTy) { }
 
-  bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-    return false;
-  }
-
   bool shouldWalkCaptureInitializerExpressions() override { return true; }
 
   MacroWalking getMacroWalkingBehavior() const override {
@@ -5585,10 +5547,6 @@ static void diagnoseUnintendedOptionalBehavior(const Expr *E,
       }
     }
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     bool shouldWalkCaptureInitializerExpressions() override { return true; }
 
     MacroWalking getMacroWalkingBehavior() const override {
@@ -5664,10 +5622,6 @@ static void diagnoseDeprecatedWritableKeyPath(const Expr *E,
       }
     }
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     bool shouldWalkCaptureInitializerExpressions() override { return true; }
 
     MacroWalking getMacroWalkingBehavior() const override {
@@ -5970,10 +5924,6 @@ static void diagUnqualifiedAccessToMethodNamedSelf(const Expr *E,
   public:
     DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()), DC(DC) {}
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     MacroWalking getMacroWalkingBehavior() const override {
       return MacroWalking::Expansion;
     }
@@ -6129,10 +6079,6 @@ diagnoseDictionaryLiteralDuplicateKeyEntries(const Expr *E,
   public:
     DiagnoseWalker(const DeclContext *DC) : Ctx(DC->getASTContext()) {}
 
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
-    }
-
     MacroWalking getMacroWalkingBehavior() const override {
       return MacroWalking::Expansion;
     }

lib/Sema/MiscDiagnostics.h

@@ -133,19 +133,14 @@ namespace swift {
                                              ForEachStmt *forEach);
 
   class BaseDiagnosticWalker : public ASTWalker {
-    PreWalkAction walkToDeclPre(Decl *D) override;
-
-    bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-      return false;
+    PreWalkAction walkToDeclPre(Decl *D) override {
+      return Action::VisitNodeIf(isa<PatternBindingDecl>(D));
     }
 
     // Only emit diagnostics in the expansion of macros.
     MacroWalking getMacroWalkingBehavior() const override {
       return MacroWalking::Expansion;
     }
-
-  private:
-    static bool shouldWalkIntoDeclInClosureContext(Decl *D);
   };
 
   // A simple, deferred diagnostic container.

lib/Sema/TypeCheckAvailability.cpp

@@ -3484,10 +3484,6 @@ class ExprAvailabilityWalker : public ASTWalker {
   explicit ExprAvailabilityWalker(const ExportContext &Where)
     : Context(Where.getDeclContext()->getASTContext()), Where(Where) {}
 
-  bool shouldWalkIntoSeparatelyCheckedClosure(ClosureExpr *expr) override {
-    return false;
-  }
-
   MacroWalking getMacroWalkingBehavior() const override {
     // Expanded source should be type checked and diagnosed separately.
     return MacroWalking::Arguments;

lib/Sema/TypeCheckConstraints.cpp

@@ -329,15 +329,12 @@ void ParentConditionalConformance::diagnoseConformanceStack(
 namespace {
 /// Produce any additional syntactic diagnostics for a SyntacticElementTarget.
 class SyntacticDiagnosticWalker final : public ASTWalker {
-  SmallVector<DeclContext *, 4> dcStack;
   const SyntacticElementTarget &Target;
   bool IsTopLevelExprStmt;
 
   SyntacticDiagnosticWalker(const SyntacticElementTarget &target,
                             bool isExprStmt)
-      : Target(target), IsTopLevelExprStmt(isExprStmt) {
-    dcStack.push_back(target.getDeclContext());
-  }
+      : Target(target), IsTopLevelExprStmt(isExprStmt) {}
 
 public:
   static void check(const SyntacticElementTarget &target, bool isExprStmt) {
@@ -351,31 +348,12 @@ class SyntacticDiagnosticWalker final : public ASTWalker {
 
   PreWalkResult<Expr *> walkToExprPre(Expr *expr) override {
     auto isExprStmt = (expr == Target.getAsExpr()) ? IsTopLevelExprStmt : false;
-    performSyntacticExprDiagnostics(expr, dcStack.back(), isExprStmt);
-
-    if (auto closure = dyn_cast<ClosureExpr>(expr)) {
-      if (closure->isSeparatelyTypeChecked()) {
-        dcStack.push_back(closure);
-        return Action::Continue(expr);
-      }
-    }
-
+    performSyntacticExprDiagnostics(expr, Target.getDeclContext(), isExprStmt);
     return Action::SkipNode(expr);
   }
 
-  PostWalkResult<Expr *> walkToExprPost(Expr *expr) override {
-    if (auto closure = dyn_cast<ClosureExpr>(expr)) {
-      if (closure->isSeparatelyTypeChecked()) {
-        assert(dcStack.back() == closure);
-        dcStack.pop_back();
-      }
-    }
-
-    return Action::Continue(expr);
-  }
-
   PreWalkResult<Stmt *> walkToStmtPre(Stmt *stmt) override {
-    performStmtDiagnostics(stmt, dcStack.back());
+    performStmtDiagnostics(stmt, Target.getDeclContext());
     return Action::Continue(stmt);
   }