Merge pull request #34551 from kavon/async-rethrows-70751405

[concurrency] type-checking call-sites that involve an async throws / rethrows callee

Author: kavon

lib/Sema/TypeCheckEffects.cpp

@@ -272,6 +272,17 @@ class PotentialThrowReason {
     CallRethrowsWithDefaultThrowingArgument,
   };
 
+  static StringRef kindToString(Kind k) {
+    switch (k) {
+      case Kind::Throw: return "Throw";
+      case Kind::CallThrows: return "CallThrows";
+      case Kind::CallRethrowsWithExplicitThrowingArgument: 
+        return "CallRethrowsWithExplicitThrowingArgument";
+      case Kind::CallRethrowsWithDefaultThrowingArgument: 
+        return "CallRethrowsWithDefaultThrowingArgument";
+    }
+  }
+
 private:
   Expr *TheExpression;
   Kind TheKind;
@@ -329,6 +340,26 @@ class Classification {
   ThrowingKind Result = ThrowingKind::None;
   Optional<PotentialThrowReason> Reason;
 
+  void print(raw_ostream &out) const {
+    out << "{ IsInvalid = " << IsInvalid
+        << ", IsAsync = " << IsAsync
+        << ", Result = ThrowingKind::";
+    
+    switch(Result) {
+      case ThrowingKind::None:            out << "None"; break;
+      case ThrowingKind::RethrowingOnly:  out << "RethrowingOnly"; break;
+      case ThrowingKind::Throws:          out << "Throws"; break;
+    }
+         
+    out << ", Reason = ";
+    if (!Reason)
+      out << "nil";
+    else
+      out << PotentialThrowReason::kindToString(Reason.getValue().getKind());
+
+    out << " }";
+  }
+  
 public:
   Classification() : Result(ThrowingKind::None) {}
   explicit Classification(ThrowingKind result, PotentialThrowReason reason,
@@ -364,17 +395,21 @@ class Classification {
     return result;
   }
 
-  static Classification forRethrowingOnly(PotentialThrowReason reason) {
+  static Classification forRethrowingOnly(PotentialThrowReason reason, bool isAsync) {
     Classification result;
     result.Result = ThrowingKind::RethrowingOnly;
     result.Reason = reason;
+    result.IsAsync = isAsync;
     return result;
   }
 
   void merge(Classification other) {
+    bool oldAsync = IsAsync;
+    
     if (other.getResult() > getResult())
       *this = other;
-    IsAsync |= other.IsAsync;
+
+    IsAsync = oldAsync | other.IsAsync;
   }
 
   bool isInvalid() const { return IsInvalid; }
@@ -386,6 +421,10 @@ class Classification {
   }
 
   bool isAsync() const { return IsAsync; }
+
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); }
+#endif
 };
 
 
@@ -399,7 +438,7 @@ class ApplyClassifier {
   DeclContext *RethrowsDC = nullptr;
   bool inRethrowsContext() const { return RethrowsDC != nullptr; }
 
-  /// Check to see if the given function application throws.
+  /// Check to see if the given function application throws or is async.
   Classification classifyApply(ApplyExpr *E) {
     // An apply expression is a potential throw site if the function throws.
     // But if the expression didn't type-check, suppress diagnostics.
@@ -461,7 +500,8 @@ class ApplyClassifier {
       if (!type) return Classification::forInvalidCode();
 
       // Use the most significant result from the arguments.
-      Classification result;
+      Classification result = isAsync ? Classification::forAsync() 
+                                      : Classification();
       for (auto arg : llvm::reverse(args)) {
         auto fnType = type->getAs<AnyFunctionType>();
         if (!fnType) return Classification::forInvalidCode();
@@ -527,7 +567,7 @@ class ApplyClassifier {
     // If we're currently doing rethrows-checking on the body of the
     // function which declares the parameter, it's rethrowing-only.
     if (param->getDeclContext() == RethrowsDC)
-      return Classification::forRethrowingOnly(reason);
+      return Classification::forRethrowingOnly(reason, /*async*/false);
 
     // Otherwise, it throws unconditionally.
     return Classification::forThrow(reason, /*async*/false);
@@ -1235,8 +1275,12 @@ class Context {
       highlight = apply->getSourceRange();
 
     auto diag = diag::async_call_without_await;
-    if (isAutoClosure())
+    // To produce a better error message, check if it is an autoclosure.
+    // We do not use 'Context::isAutoClosure' b/c it gives conservative answers.
+    if (Function && llvm::isa_and_nonnull<AutoClosureExpr>(
+                                            Function->getAbstractClosureExpr()))
       diag = diag::async_call_without_await_in_autoclosure;
+
     ctx.Diags.diagnose(node.getStartLoc(), diag)
         .fixItInsert(node.getStartLoc(), "await ")
         .highlight(highlight);
@@ -1464,6 +1508,10 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
       OldMaxThrowingKind = std::max(OldMaxThrowingKind, Self.MaxThrowingKind);
     }
 
+    void preserveCoverageFromOptionalOrForcedTryOperand() {
+      OldFlags.mergeFrom(ContextFlags::asyncAwaitFlags(), Self.Flags);
+    }
+
     void preserveCoverageFromInterpolatedString() {
       OldFlags.mergeFrom(ContextFlags::HasAnyThrowSite, Self.Flags);
       OldFlags.mergeFrom(ContextFlags::HasTryThrowSite, Self.Flags);
@@ -1808,6 +1856,8 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     if (!Flags.has(ContextFlags::HasTryThrowSite)) {
       Ctx.Diags.diagnose(E->getLoc(), diag::no_throw_in_try);
     }
+
+    scope.preserveCoverageFromOptionalOrForcedTryOperand();
     return ShouldNotRecurse;
   }
 
@@ -1822,6 +1872,8 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     if (!Flags.has(ContextFlags::HasTryThrowSite)) {
       Ctx.Diags.diagnose(E->getLoc(), diag::no_throw_in_try);
     }
+
+    scope.preserveCoverageFromOptionalOrForcedTryOperand();
     return ShouldNotRecurse;
   }
 };

test/Concurrency/async_cancellation.swift

@@ -53,14 +53,13 @@ func test_cancellation_loop() async -> Int {
 func int() async -> Int { 42 }
 
 func test_cancellation_withDeadline_in() async throws -> Int {
-  /* missing await */ Task.withDeadline(in: .seconds(5), operation: { // FIXME: rdar://70751405 async rethrows functions are not detected as async
+  await Task.withDeadline(in: .seconds(5), operation: {
     await int()
   })
 }
 
 func test_cancellation_withDeadline(specificDeadline: Task.Deadline) async -> Int {
-  /* missing `await` */
-  Task.withDeadline(specificDeadline) { // FIXME: rdar://70751405 async rethrows functions are not detected as async
+  await Task.withDeadline(specificDeadline) {
     await int()
   }
 }

test/Concurrency/async_throwing.swift

@@ -0,0 +1,136 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-concurrency
+// REQUIRES: concurrency
+
+// These tests cover various interactions with async functions that are 
+// either throws or rethrows.
+// See rdar://70813762 and rdar://70751405
+
+enum InvocationError : Error {
+  case ErrVal
+}
+
+func asyncThrows() async throws {
+  throw InvocationError.ErrVal
+}
+
+// T = Int
+func asyncRethrows(fn : () async throws -> Int) async rethrows -> Int {
+  return await try fn()
+}
+
+// T = String
+func asyncRethrows(fn : () async throws -> String) async rethrows -> String {
+  return await try fn()
+}
+
+// Generic. NOTE the 'rethrows'
+func invoke<T>(fn : () async throws -> T) async rethrows -> T {
+  return await try fn()
+}
+
+// NOTE the 'rethrows'
+func invokeAuto<T>(_ val : @autoclosure () async throws -> T) async rethrows -> T {
+  return await try val()
+}
+
+func normalTask() async -> Int {
+  return 42
+}
+
+func throwingTask() async throws -> String {
+  if 1.0 / 3.0 == 0.33 {
+    throw InvocationError.ErrVal
+  }
+  return "ok!"
+}
+
+// expected-note@+2 7 {{add '@asyncHandler' to function 'syncTest()' to create an implicit asynchronous context}}
+// expected-note@+1 7 {{add 'async' to function 'syncTest()' to make it asynchronous}}
+func syncTest() {
+  let _ = invoke(fn: normalTask) // expected-error{{'async' in a function that does not support concurrency}}
+  let _ = invokeAuto(42) // expected-error{{'async' in a function that does not support concurrency}}
+  let _ = invokeAuto("intuitive") // expected-error{{'async' in a function that does not support concurrency}}
+  
+  let _ = try! asyncRethrows(fn: throwingTask) // expected-error{{'async' in a function that does not support concurrency}}
+  let _ = try? invoke(fn: throwingTask) // expected-error{{'async' in a function that does not support concurrency}}
+  do {
+   let _ = try invoke(fn: throwingTask) // expected-error{{'async' in a function that does not support concurrency}}
+   let _ = try asyncThrows() // expected-error{{'async' in a function that does not support concurrency}}
+  } catch {
+    // ignore it
+  }
+}
+
+
+func asyncTest() async {
+  ///////////
+  // tests that also omit await
+
+  let _ = invoke(fn: normalTask) // expected-error{{call is 'async' but is not marked with 'await'}}
+  let _ = asyncRethrows(fn: normalTask) // expected-error{{call is 'async' but is not marked with 'await'}}
+  let _ = invokeAuto(42) // expected-error{{call is 'async' but is not marked with 'await'}}
+
+  // expected-error@+2 {{call can throw, but it is not marked with 'try' and the error is not handled}}
+  // expected-error@+1 {{call is 'async' but is not marked with 'await'}}
+  let _ = asyncThrows()
+  
+  // expected-note@+3{{call is to 'rethrows' function, but argument function can throw}}
+  // expected-error@+2{{call can throw, but it is not marked with 'try' and the error is not handled}}
+  // expected-error@+1{{call is 'async' but is not marked with 'await'}}
+  let _ = invoke(fn: throwingTask)
+
+  ///////////
+  // tests that use await and handles the exceptions
+
+  // expected-note@+2{{call is to 'rethrows' function, but argument function can throw}}
+  // expected-error@+1{{call can throw, but it is not marked with 'try' and the error is not handled}}
+  let _ = await invoke(fn: throwingTask)
+  let _ = await invoke(fn: normalTask) // ok
+
+  let _ = await asyncRethrows(fn: normalTask)
+  let _ = await try! asyncRethrows(fn: normalTask) // expected-warning{{no calls to throwing functions occur within 'try' expression}}
+  let _ = await try? asyncRethrows(fn: normalTask) // expected-warning{{no calls to throwing functions occur within 'try' expression}}
+  
+  let _ = await try! asyncRethrows(fn: throwingTask)
+  let _ = await try? asyncRethrows(fn: throwingTask)
+  let _ = await try! asyncThrows()
+  let _ = await try? asyncThrows()
+
+  //////////
+  // some auto-closure tests
+
+  let _ = await invokeAuto("intuitive")
+  let _ = await try! invokeAuto(await throwingTask())
+  let _ = await try? invokeAuto(await throwingTask())
+  let _ = await invokeAuto(await try! throwingTask())
+  let _ = await invokeAuto(await try? throwingTask())
+
+  let _ = await invokeAuto(try! throwingTask()) // expected-error{{call is 'async' in an autoclosure argument that is not marked with 'await'}}
+  let _ = await invokeAuto(try? throwingTask()) // expected-error{{call is 'async' in an autoclosure argument that is not marked with 'await'}}
+
+  let _ = await invokeAuto(await try! throwingTask())
+  let _ = await invokeAuto(await try? throwingTask())
+  /////////
+
+  do {
+    let _ = await try asyncThrows()
+    let _ = await try asyncRethrows(fn: throwingTask)
+
+    //////
+    // more auto-closure tests
+    
+    // expected-note@+6 {{did you mean to disable error propagation?}}
+    // expected-note@+5 {{did you mean to handle error as optional value?}}
+    // expected-note@+4 {{did you mean to use 'try'?}}
+    // expected-note@+3 {{call is to 'rethrows' function, but argument function can throw}}
+    // expected-error@+2 {{call is 'async' in an autoclosure argument that is not marked with 'await'}}
+    // expected-error@+1 2 {{call can throw but is not marked with 'try'}}
+    let _ = await invokeAuto(throwingTask())
+
+    let _ = await try invokeAuto(throwingTask()) // expected-error{{call is 'async' in an autoclosure argument that is not marked with 'await'}}
+    let _ = try invokeAuto(await throwingTask()) // expected-error{{call is 'async' but is not marked with 'await'}}
+    let _ = await try invokeAuto(await throwingTask())
+  } catch {
+    // ignore
+  }
+}