r304094 - [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Sun May 28 13:21:12 2017
New Revision: 304094

URL: http://llvm.org/viewvc/llvm-project?rev=304094&view=rev
Log:
[coroutines] Diagnose invalid result  types for `await_resume` and `await_suspend` and add missing conversions.

Summary:
The expression `await_ready` is required to be contextually convertible to bool and `await_suspend` must be a prvalue of either `void` or `bool`.
This patch adds diagnostics for when those requirements are violated.

It also correctly performs the contextual conversion to bool on the result of `await_ready`



Reviewers: GorNishanov, rsmith

Reviewed By: GorNishanov

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D33625

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaCoroutine.cpp
    cfe/trunk/test/SemaCXX/coroutines.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=304094&r1=304093&r2=304094&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun May 28 13:21:12 2017
@@ -8973,6 +8973,12 @@ def err_coroutine_promise_new_requires_n
    "type declares 'get_return_object_on_allocation_failure()'">;
 def note_coroutine_promise_call_implicitly_required : Note<
   "call to %0 implicitly required by coroutine function here">;
+def err_await_suspend_invalid_return_type : Error<
+  "the return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+>;
+def note_await_ready_no_bool_conversion : Note<
+  "the return type of 'await_ready' is required to be contextually convertible to 'bool'"
+>;
 }
 
 let CategoryName = "Documentation Issue" in {

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=304094&r1=304093&r2=304094&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Sun May 28 13:21:12 2017
@@ -321,6 +321,7 @@ static ExprResult buildCoroutineHandle(S
 }
 
 struct ReadySuspendResumeResult {
+  enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume };
   Expr *Results[3];
   OpaqueValueExpr *OpaqueValue;
   bool IsInvalid;
@@ -366,7 +367,41 @@ static ReadySuspendResumeResult buildCoa
     Calls.Results[I] = Result.get();
   }
 
+  // Assume the calls are valid; all further checking should make them invalid.
   Calls.IsInvalid = false;
+
+  using ACT = ReadySuspendResumeResult::AwaitCallType;
+  CallExpr *AwaitReady = cast<CallExpr>(Calls.Results[ACT::ACT_Ready]);
+  if (!AwaitReady->getType()->isDependentType()) {
+    // [expr.await]p3 [...]
+    // — await-ready is the expression e.await_ready(), contextually converted
+    // to bool.
+    ExprResult Conv = S.PerformContextuallyConvertToBool(AwaitReady);
+    if (Conv.isInvalid()) {
+      S.Diag(AwaitReady->getDirectCallee()->getLocStart(),
+             diag::note_await_ready_no_bool_conversion);
+      S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+          << AwaitReady->getDirectCallee() << E->getSourceRange();
+      Calls.IsInvalid = true;
+    }
+    Calls.Results[ACT::ACT_Ready] = Conv.get();
+  }
+  CallExpr *AwaitSuspend = cast<CallExpr>(Calls.Results[ACT::ACT_Suspend]);
+  if (!AwaitSuspend->getType()->isDependentType()) {
+    // [expr.await]p3 [...]
+    //   - await-suspend is the expression e.await_suspend(h), which shall be
+    //     a prvalue of type void or bool.
+    QualType RetType = AwaitSuspend->getType();
+    if (RetType != S.Context.BoolTy && RetType != S.Context.VoidTy) {
+      S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+             diag::err_await_suspend_invalid_return_type)
+          << RetType;
+      S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+          << AwaitSuspend->getDirectCallee();
+      Calls.IsInvalid = true;
+    }
+  }
+
   return Calls;
 }
 

Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=304094&r1=304093&r2=304094&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Sun May 28 13:21:12 2017
@@ -837,3 +837,40 @@ coro<bad_promise_no_return_func> no_retu
   // expected-error at -1 {{'bad_promise_no_return_func' must declare either 'return_value' or 'return_void'}}
   co_await a;
 }
+
+struct bad_await_suspend_return {
+  bool await_ready();
+  // expected-error at +1 {{the return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}
+  char await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct bad_await_ready_return {
+  // expected-note at +1 {{the return type of 'await_ready' is required to be contextually convertible to 'bool'}}
+  void await_ready();
+  bool await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct await_ready_explicit_bool {
+  struct BoolT {
+    explicit operator bool() const;
+  };
+  BoolT await_ready();
+  void await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+void test_bad_suspend() {
+  {
+    // FIXME: The actual error emitted here is terrible, and no number of notes can save it.
+    bad_await_ready_return a;
+    // expected-error at +1 {{value of type 'void' is not contextually convertible to 'bool'}}
+    co_await a; // expected-note {{call to 'await_ready' implicitly required by coroutine function here}}
+  }
+  {
+    bad_await_suspend_return b;
+    co_await b; // expected-note {{call to 'await_suspend' implicitly required by coroutine function here}}
+  }
+  {
+    await_ready_explicit_bool c;
+    co_await c; // OK
+  }
+}