Merge pull request swiftlang#36481 from etcwilde/ewilde/suggest-async-function

etcwilde · web-flow · commit 3bf1f074b381 · 2021-03-25T08:18:00.000-07:00
[Concurrency] Emit warning suggesting async function
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -1682,6 +1682,62 @@ class PatternBindingDecl final : public Decl,
     return const_cast<PatternBindingDecl*>(this)->getMutablePatternList();
   }
 
+  /// Clean up walking the initializers for the pattern
+  class InitIterator {
+
+    const PatternBindingDecl &decl;
+    unsigned currentPatternEntryIndex;
+
+    void next() { ++currentPatternEntryIndex; }
+
+  public:
+    using value_type = Expr *;
+    using pointer = value_type;
+    using reference = value_type;
+    using difference_type = unsigned;
+
+    InitIterator(const PatternBindingDecl &decl, unsigned start = 0)
+        : decl(decl), currentPatternEntryIndex(start) {}
+
+    InitIterator &operator++() {
+      next();
+      return *this;
+    }
+
+    InitIterator operator++(int) {
+      InitIterator newIterator(decl, currentPatternEntryIndex);
+      newIterator.next();
+      return newIterator;
+    }
+
+    pointer operator->() { return decl.getInit(currentPatternEntryIndex); }
+
+    pointer operator*() { return decl.getInit(currentPatternEntryIndex); }
+
+    difference_type operator-(const InitIterator &other) {
+      return currentPatternEntryIndex - other.currentPatternEntryIndex;
+    }
+
+    bool operator==(const InitIterator &other) const {
+      return &decl == &other.decl &&
+             currentPatternEntryIndex == other.currentPatternEntryIndex;
+    }
+
+    bool operator!=(const InitIterator &other) const {
+      return !(*this == other);
+    }
+  };
+
+  InitIterator beginInits() const { return InitIterator(*this); }
+
+  InitIterator endInits() const {
+    return InitIterator(*this, getNumPatternEntries());
+  }
+
+  llvm::iterator_range<InitIterator> initializers() const {
+    return llvm::make_range(beginInits(), endInits());
+  }
+
   void setInitStringRepresentation(unsigned i, StringRef str) {
     getMutablePatternList()[i].setInitStringRepresentation(str);
   }
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -3377,6 +3377,9 @@ ERROR(attr_completion_handler_async_ambiguous_function,none,
 ERROR(attr_completion_handler_async_no_suitable_function,none,
       "no corresponding async function named %0", (DeclNameRef))
 
+WARNING(warn_use_async_alternative,none,
+        "consider using asynchronous alternative function",())
+
 //------------------------------------------------------------------------------
 // MARK: Type Check Expressions
 //------------------------------------------------------------------------------
diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h
@@ -2936,6 +2936,26 @@ class ConditionalRequirementsRequest
   bool isCached() const { return true; }
 };
 
+class TypeCheckCompletionHandlerAsyncAttrRequest
+    : public SimpleRequest<TypeCheckCompletionHandlerAsyncAttrRequest,
+                           bool(AbstractFunctionDecl *,
+                                CompletionHandlerAsyncAttr *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  bool evaluate(Evaluator &evaluator,
+                AbstractFunctionDecl *attachedFucntionDecl,
+                CompletionHandlerAsyncAttr *attr) const;
+
+public:
+  // Caching
+  bool isCached() const { return true; }
+};
+
 void simple_display(llvm::raw_ostream &out, Type value);
 void simple_display(llvm::raw_ostream &out, const TypeRepr *TyR);
 void simple_display(llvm::raw_ostream &out, ImplicitMemberAction action);
diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def
@@ -323,3 +323,6 @@ SWIFT_REQUEST(TypeChecker, SynthesizeMainFunctionRequest,
 SWIFT_REQUEST(TypeChecker, GetImplicitSendableRequest,
               NormalProtocolConformance *(NominalTypeDecl *),
               Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, TypeCheckCompletionHandlerAsyncAttrRequest,
+              bool (AbstractFunctionDecl *, CompletionHandlerAsyncAttr *),
+              Cached, NoLocationInfo)
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -24,6 +24,7 @@
 #include "swift/AST/NameLookupRequests.h"
 #include "swift/AST/Pattern.h"
 #include "swift/AST/SourceFile.h"
+#include "swift/AST/TypeCheckRequests.h"
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/SourceManager.h"
 #include "swift/Basic/Statistic.h"
@@ -4617,6 +4618,68 @@ static void diagnoseComparisonWithNaN(const Expr *E, const DeclContext *DC) {
   const_cast<Expr *>(E)->walk(Walker);
 }
 
+namespace {
+
+class CompletionHandlerUsageChecker final : public ASTWalker {
+  ASTContext &ctx;
+
+public:
+  CompletionHandlerUsageChecker(ASTContext &ctx) : ctx(ctx) {}
+
+  bool walkToDeclPre(Decl *D) override { return !isa<PatternBindingDecl>(D); }
+
+  std::pair<bool, Expr *> walkToExprPre(Expr *expr) override {
+    if (expr->getType().isNull())
+      return {false, expr}; // Something failed to typecheck, bail out
+
+    if (ClosureExpr *closure = dyn_cast<ClosureExpr>(expr))
+      return {closure->isBodyAsync(), closure};
+
+    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) {
+          // Ensure that the attribute typechecks,
+          // this also resolves the async function decl.
+          if (!evaluateOrDefault(
+                  ctx.evaluator,
+                  TypeCheckCompletionHandlerAsyncAttrRequest{
+                      cast<AbstractFunctionDecl>(fnDecl), asyncAltAttr},
+                  false)) {
+            return {false, call};
+          }
+          ctx.Diags.diagnose(call->getLoc(), diag::warn_use_async_alternative);
+          ctx.Diags.diagnose(asyncAltAttr->AsyncFunctionDecl->getLoc(),
+                             diag::decl_declared_here,
+                             asyncAltAttr->AsyncFunctionDecl->getName());
+        }
+      }
+    }
+    return {true, expr};
+  }
+};
+
+} // namespace
+
+void swift::checkFunctionAsyncUsage(AbstractFunctionDecl *decl) {
+  if (!decl->isAsyncContext())
+    return;
+  CompletionHandlerUsageChecker checker(decl->getASTContext());
+  BraceStmt *body = decl->getBody();
+  if (body)
+    body->walk(checker);
+}
+
+void swift::checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl) {
+  CompletionHandlerUsageChecker checker(decl->getASTContext());
+  for (Expr *init : decl->initializers()) {
+    if (auto closure = dyn_cast_or_null<ClosureExpr>(init))
+      closure->walk(checker);
+  }
+}
+
 //===----------------------------------------------------------------------===//
 // High-level entry points.
 //===----------------------------------------------------------------------===//
diff --git a/lib/Sema/MiscDiagnostics.h b/lib/Sema/MiscDiagnostics.h
@@ -108,6 +108,13 @@ void fixItEncloseTrailingClosure(ASTContext &ctx,
                                  const CallExpr *call,
                                  Identifier closureLabel);
 
+/// Check that we use the async version of a function where available
+///
+/// If a completion-handler function is called from an async context and it has
+/// a '@completionHandlerAsync' attribute, we emit a diagnostic suggesting the
+/// async call.
+void checkFunctionAsyncUsage(AbstractFunctionDecl *decl);
+void checkPatternBindingDeclAsyncUsage(PatternBindingDecl *decl);
 } // namespace swift
 
 #endif // SWIFT_SEMA_MISC_DIAGNOSTICS_H
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
@@ -5609,7 +5609,18 @@ void TypeChecker::checkClosureAttributes(ClosureExpr *closure) {
 
 void AttributeChecker::visitCompletionHandlerAsyncAttr(
     CompletionHandlerAsyncAttr *attr) {
+  AbstractFunctionDecl *AFD = dyn_cast<AbstractFunctionDecl>(D);
+  if (!AFD)
+    return;
+
+  evaluateOrDefault(Ctx.evaluator,
+                    TypeCheckCompletionHandlerAsyncAttrRequest{AFD, attr}, {});
+}
 
+bool TypeCheckCompletionHandlerAsyncAttrRequest::evaluate(
+    Evaluator &evaluator, AbstractFunctionDecl *attachedFunctionDecl,
+    CompletionHandlerAsyncAttr *attr) const {
+  auto &Diags = attachedFunctionDecl->getASTContext().Diags;
   // Check phases:
   //  1. Attached function shouldn't be async and should have enough args
   //     to have a completion handler
@@ -5621,30 +5632,29 @@ void AttributeChecker::visitCompletionHandlerAsyncAttr(
   //      - Do some sanity checking on types
 
   // Phase 1: Typecheck the function the attribute is attached to
-  auto attachedFunctionDecl = cast<AbstractFunctionDecl>(D);
   if (attachedFunctionDecl->hasAsync()) {
-    diagnose(attr->getLocation(),
-             diag::attr_completion_handler_async_handler_not_func, attr);
-    diagnose(attachedFunctionDecl->getAsyncLoc(),
-             diag::note_attr_function_declared_async);
-    return;
+    Diags.diagnose(attr->getLocation(),
+                   diag::attr_completion_handler_async_handler_not_func, attr);
+    Diags.diagnose(attachedFunctionDecl->getAsyncLoc(),
+                   diag::note_attr_function_declared_async);
+    return false;
   }
 
   const ParameterList *attachedFunctionParams =
       attachedFunctionDecl->getParameters();
   assert(attachedFunctionParams && "Attached function has no parameter list");
   if (attachedFunctionParams->size() == 0) {
-    diagnose(attr->getLocation(),
-             diag::attr_completion_handler_async_handler_not_func, attr);
-    return;
+    Diags.diagnose(attr->getLocation(),
+                   diag::attr_completion_handler_async_handler_not_func, attr);
+    return false;
   }
   size_t completionHandlerIndex = attr->CompletionHandlerIndexLoc.isValid()
                                       ? attr->CompletionHandlerIndex
                                       : attachedFunctionParams->size() - 1;
   if (attachedFunctionParams->size() < completionHandlerIndex) {
-    diagnose(attr->CompletionHandlerIndexLoc,
-             diag::attr_completion_handler_async_handler_out_of_range);
-    return;
+    Diags.diagnose(attr->CompletionHandlerIndexLoc,
+                   diag::attr_completion_handler_async_handler_out_of_range);
+    return false;
   }
 
   // Phase 2: Typecheck the completion handler
@@ -5654,14 +5664,17 @@ void AttributeChecker::visitCompletionHandlerAsyncAttr(
     AnyFunctionType *handlerType =
         completionHandlerParamDecl->getType()->getAs<AnyFunctionType>();
     if (!handlerType) {
-      diagnose(attr->getLocation(),
-               diag::attr_completion_handler_async_handler_not_func, attr);
-      diagnose(completionHandlerParamDecl->getTypeRepr()->getLoc(),
-               diag::note_attr_completion_handler_async_type_is_not_function,
-               completionHandlerParamDecl->getType())
+      Diags.diagnose(attr->getLocation(),
+                     diag::attr_completion_handler_async_handler_not_func,
+                     attr);
+      Diags
+          .diagnose(
+              completionHandlerParamDecl->getTypeRepr()->getLoc(),
+              diag::note_attr_completion_handler_async_type_is_not_function,
+              completionHandlerParamDecl->getType())
           .highlight(
               completionHandlerParamDecl->getTypeRepr()->getSourceRange());
-      return;
+      return false;
     }
 
     auto handlerTypeRepr =
@@ -5678,27 +5691,31 @@ void AttributeChecker::visitCompletionHandlerAsyncAttr(
     const bool hasError = missingVoid | hasAutoclosure | !hasEscaping;
 
     if (hasError) {
-      diagnose(attr->getLocation(),
-               diag::attr_completion_handler_async_handler_not_func, attr);
+      Diags.diagnose(attr->getLocation(),
+                     diag::attr_completion_handler_async_handler_not_func,
+                     attr);
 
       if (missingVoid)
-        diagnose(completionHandlerParamDecl->getLoc(),
-                 diag::note_attr_completion_function_must_return_void)
+        Diags
+            .diagnose(completionHandlerParamDecl->getLoc(),
+                      diag::note_attr_completion_function_must_return_void)
             .highlight(
                 completionHandlerParamDecl->getTypeRepr()->getSourceRange());
 
       if (!hasEscaping)
-        diagnose(completionHandlerParamDecl->getLoc(),
-                 diag::note_attr_completion_handler_async_handler_attr_req,
-                 true, "escaping")
+        Diags
+            .diagnose(completionHandlerParamDecl->getLoc(),
+                      diag::note_attr_completion_handler_async_handler_attr_req,
+                      true, "escaping")
             .highlight(
                 completionHandlerParamDecl->getTypeRepr()->getSourceRange());
 
       if (hasAutoclosure)
-        diagnose(handlerTypeAttrs->getLoc(TAK_autoclosure),
-                 diag::note_attr_completion_handler_async_handler_attr_req,
-                 false, "autoclosure");
-      return;
+        Diags.diagnose(
+            handlerTypeAttrs->getLoc(TAK_autoclosure),
+            diag::note_attr_completion_handler_async_handler_attr_req, false,
+            "autoclosure");
+      return false;
     }
   }
 
@@ -5747,21 +5764,23 @@ void AttributeChecker::visitCompletionHandlerAsyncAttr(
     }
 
     if (candidates.empty()) {
-      diagnose(attr->AsyncFunctionNameLoc,
-               diag::attr_completion_handler_async_no_suitable_function,
-               attr->getAsyncFunctionName());
-      return;
+      Diags.diagnose(attr->AsyncFunctionNameLoc,
+                     diag::attr_completion_handler_async_no_suitable_function,
+                     attr->getAsyncFunctionName());
+      return false;
     } else if (candidates.size() > 1) {
-      diagnose(attr->AsyncFunctionNameLoc,
-               diag::attr_completion_handler_async_ambiguous_function, attr,
-               attr->getAsyncFunctionName());
+      Diags.diagnose(attr->AsyncFunctionNameLoc,
+                     diag::attr_completion_handler_async_ambiguous_function,
+                     attr, attr->getAsyncFunctionName());
 
       for (AbstractFunctionDecl *candidate : candidates) {
-        diagnose(candidate->getLoc(), diag::decl_declared_here,
-                 candidate->getName());
+        Diags.diagnose(candidate->getLoc(), diag::decl_declared_here,
+                       candidate->getName());
       }
-      return;
+      return false;
     }
     attr->AsyncFunctionDecl = candidates.front();
   }
+
+  return true;
 }
diff --git a/lib/Sema/TypeCheckConcurrency.h b/lib/Sema/TypeCheckConcurrency.h
@@ -238,7 +238,6 @@ Type getExplicitGlobalActor(ClosureExpr *closure);
 /// \returns true if an error occurred.
 bool checkSendableConformance(
     ProtocolConformance *conformance, SendableCheck check);
-
 } // end namespace swift
 
 #endif /* SWIFT_SEMA_TYPECHECKCONCURRENCY_H */
diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp
diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp
diff --git a/test/attr/attr_completionhandlerasync.swift b/test/attr/attr_completionhandlerasync.swift
