Check global closures

This patch adds checking for usage of completion handler functions with
async alternatives in closures at either the global scope (for
-parse-as-library) or as top-level-decls.

The top-level and global scope are typechecked in the order that they
are declared. This means that if the closure calls a completion
handler function that is declared later, the attribute won't have been
typechecked yet and thus won't have a resolved async function. This
patch updates the warning emitter to typecheck and resolve the attribute
if it needs to. If the thing doesn't typecheck, we don't need emit the
warning at all because the attribute is bad and we tell them to fix it.

Author: etcwilde

lib/Sema/TypeCheckConcurrency.cpp

@@ -3096,12 +3096,29 @@ class CompletionHandlerUsageChecker final : public ASTWalker {
   CompletionHandlerUsageChecker(ASTContext &ctx) : ctx(ctx) {}
 
   std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
+    if (ClosureExpr *closure = dyn_cast<ClosureExpr>(expr)) {
+      if (closure->isBodyAsync())
+        return {true, closure};
+      else
+        return {true, nullptr};
+    }
+
     if (ApplyExpr *call = dyn_cast<ApplyExpr>(expr)) {
       if (DeclRefExpr *fnDeclExpr = dyn_cast<DeclRefExpr>(call->getFn())) {
         ValueDecl *fnDecl = fnDeclExpr->getDecl();
         CompletionHandlerAsyncAttr *asyncAltAttr =
             fnDecl->getAttrs().getAttribute<CompletionHandlerAsyncAttr>();
-        if (asyncAltAttr && asyncAltAttr->AsyncFunctionDecl) {
+        if (asyncAltAttr) {
+          // Ensure that the attribute typechecks,
+          // this also resolves the async function decl.
+          if (!evaluateOrDefault(
+                  ctx.evaluator,
+                  TypeCheckCompletionHandlerAsyncAttrRequest{
+                      cast<AbstractFunctionDecl>(fnDecl), asyncAltAttr},
+                  false)) {
+            return {true,
+                    nullptr}; // This didn't typecheck, don't traverse down
+          }
           ctx.Diags.diagnose(call->getLoc(), diag::warn_use_async_alternative);
           ctx.Diags.diagnose(asyncAltAttr->AsyncFunctionDecl->getLoc(),
                              diag::decl_declared_here,
@@ -3122,3 +3139,8 @@ void swift::checkFunctionAsyncUsage(AbstractFunctionDecl *decl) {
   if (body)
     body->walk(checker);
 }
+
+void swift::checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl) {
+  CompletionHandlerUsageChecker checker(decl->getASTContext());
+  decl->walk(checker);
+}

lib/Sema/TypeCheckConcurrency.h

@@ -245,7 +245,7 @@ bool checkSendableConformance(
 /// a '@completionHandlerAsync' attribute, we emit a diagnostic suggesting the
 /// async call.
 void checkFunctionAsyncUsage(AbstractFunctionDecl *decl);
-
+void checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl);
 } // end namespace swift
 
 #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */

lib/Sema/TypeCheckConstraints.cpp

@@ -17,6 +17,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "MiscDiagnostics.h"
+#include "TypeCheckConcurrency.h"
 #include "TypeChecker.h"
 #include "swift/AST/ASTVisitor.h"
 #include "swift/AST/ASTWalker.h"
@@ -35,14 +36,14 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/raw_ostream.h"
-#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/raw_ostream.h"
 #include <iterator>
 #include <map>
 #include <memory>
-#include <utility>
 #include <tuple>
+#include <utility>
 
 using namespace swift;
 using namespace constraints;
@@ -538,12 +539,14 @@ bool TypeChecker::typeCheckPatternBinding(PatternBindingDecl *PBD,
       }
     }
   }
-  
+
   if (hadError)
     PBD->setInvalid();
 
   PBD->setInitializerChecked(patternNumber);
-  
+
+  checkPatternBindingDeclAsyncUsage(PBD);
+
   return hadError;
 }
 

test/attr/attr_completionhandlerasync.swift

@@ -1,12 +1,13 @@
 // REQUIRES: concurrency
 
 // RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency -parse-as-library
 
 // ===================
 // Parsing
 // ===================
 
-// expected-note@+1{{'asyncFunc' declared here}}
+// expected-note@+1 3 {{'asyncFunc' declared here}}
 func asyncFunc(_ text: String) async -> Int { }
 
 @completionHandlerAsync("asyncFunc(_:)", completionHandlerIndex: 1)
@@ -145,11 +146,21 @@ func typecheckFunc9(handler: @escaping () -> Void) {}
 
 // Suggest using async alternative function in async context
 
-
 func asyncContext() async {
-  // expected-warning@+1:3{{consider using asynchronous alternative}}
+  // expected-warning@+1:3{{consider using asynchronous alternative function}}
   goodFunc1(value: "Hello") { _ in }
 
+  let _ = {
+    // No warning or error since we're in a sync context here
+    goodFunc1(value: "Hello") { _ in }
+  }
+
+  let _ = { () async -> () in
+    let _ = await asyncFunc("Hello World")
+    // expected-warning@+1{{consider using asynchronous alternative function}}
+    goodFunc1(value: "Hello") { _ in }
+  }
+
   let _ = await asyncFunc("World")
 
   // This doesn't get the warning because the completionHandlerAsync failed to
@@ -160,3 +171,8 @@ func asyncContext() async {
 func syncContext() {
   goodFunc1(value: "Hello") { _ in }
 }
+
+let asyncGlobalClosure = { () async -> () in
+  // expected-warning@+1:3{{consider using asynchronous alternative function}}
+  goodFunc1(value: "neat") { _ in }
+}