Move diagnostics to postWalk and make all checks consistent

Author: aviralg

lib/Sema/PerformanceHints.cpp

@@ -75,15 +75,15 @@ bool hasExistentialAnyInType(Type type) {
   return type->getCanonicalType().findIf(isExistentialType);
 }
 
-void checkExistentialInFunctionReturnType(FuncDecl *FD,
+void checkExistentialInFunctionReturnType(const FuncDecl *FD,
                                           DiagnosticEngine &Diags) {
   Type T = FD->getResultInterfaceType();
 
   if (hasExistentialAnyInType(T))
     Diags.diagnose(FD, diag::perf_hint_func_returns_existential, FD);
 }
 
-void checkExistentialInClosureReturnType(ClosureExpr *CE,
+void checkExistentialInClosureReturnType(const ClosureExpr *CE,
                                          DiagnosticEngine &Diags) {
   Type T = CE->getResultType();
 
@@ -132,6 +132,13 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
     if (P->isImplicit())
       return Action::SkipNode(P);
 
+    return Action::Continue(P);
+  }
+
+  PostWalkResult<Pattern *> walkToPatternPost(Pattern *P) override {
+    assert(!P->isImplicit() &&
+           "Traversing implicit patterns is disabled in the pre-walk visitor");
+
     if (const AnyPattern *AP = dyn_cast<AnyPattern>(P)) {
       checkExistentialInPatternType(AP, Ctx.Diags);
     }
@@ -143,10 +150,6 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
     if (E->isImplicit())
       return Action::SkipNode(E);
 
-    if (const ClosureExpr* Closure = dyn_cast<ClosureExpr>(E)) {
-      checkImplicitCopyReturnType(Closure, Ctx.Diags);
-    }
-
     return Action::Continue(E);
   }
 
@@ -155,8 +158,9 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
         !E->isImplicit() &&
         "Traversing implicit expressions is disabled in the pre-walk visitor");
 
-    if (auto Closure = dyn_cast<ClosureExpr>(E)) {
-      checkExistentialInClosureReturnType(Closure, Ctx.Diags);
+    if (const ClosureExpr *CE = dyn_cast<ClosureExpr>(E)) {
+      checkImplicitCopyReturnType(CE, Ctx.Diags);
+      checkExistentialInClosureReturnType(CE, Ctx.Diags);
     }
 
     return Action::Continue(E);
@@ -166,14 +170,6 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
     if (D->isImplicit())
       return Action::SkipNode();
 
-    if (const FuncDecl *FD = dyn_cast<FuncDecl>(D)) {
-      checkImplicitCopyReturnType(FD, Ctx.Diags);
-    } else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
-      checkExistentialInVariableType(VD, Ctx.Diags);
-    } else if (const TypeAliasDecl *TAD = dyn_cast<TypeAliasDecl>(D)) {
-      checkExistentialInTypeAlias(TAD, Ctx.Diags);
-    }
-
     return Action::Continue();
   }
 
@@ -182,8 +178,13 @@ class PerformanceHintDiagnosticWalker final : public ASTWalker {
         !D->isImplicit() &&
         "Traversing implicit declarations is disabled in the pre-walk visitor");
 
-    if (auto *FD = dyn_cast<FuncDecl>(D)) {
+    if (const FuncDecl *FD = dyn_cast<FuncDecl>(D)) {
+      checkImplicitCopyReturnType(FD, Ctx.Diags);
       checkExistentialInFunctionReturnType(FD, Ctx.Diags);
+    } else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
+      checkExistentialInVariableType(VD, Ctx.Diags);
+    } else if (const TypeAliasDecl *TAD = dyn_cast<TypeAliasDecl>(D)) {
+      checkExistentialInTypeAlias(TAD, Ctx.Diags);
     }
 
     return Action::Continue();