[coroutines] Build and pass coroutine_handle to await_suspend.

[GorNishanov]

Summary

This patch adds passing a coroutine_handle object to await_suspend calls.

It builds the coroutine_handle using coroutine_handle<PromiseType>::from_address(__builtin_coro_frame()).

[EricWF]

• Update so that it merges with D26057

[GorNishanov]

Comandeering this patch.
Last week Richard gave me some feedback in person on this code (in the full coroutine implementation not this patch in particular). I am going to implement the feedback and adjust this patch and resubmit for review.

[GorNishanov]

Per's @rsmith feedback in Kona

• Added diagnostic if from_address is missing from coroutine_handle
• Switch to using BuildDeclarationNameExpr in buildCoroutineHandle

[GorNishanov]

• removed '&' that snicked near Location parameter
• reordered a few lines to minimize the diff from Eric's version

[GorNishanov]

@EricWF, LGTM?

[EricWF]

This LGTM, but I don't know enough Clang to be 100% on it. Perhaps somebody else could give it a quick second look?

lib/Sema/SemaCoroutine.cpp

157

Should this diagnostic be renamed since it's general to both coroutine_traits and coroutine_handle?

test/SemaCXX/coroutines.cpp

542

Nit, put the // expected-note directly on the co_return line.

[EricWF]

On second thought if @rsmith already reviewed most of this offline then I feel comfortable giving it the thumbs up.

[EricWF]

@GorNishanov You forgot to update test/SemaCXX/coreturn.cpp.

[GorNishanov]

• Addressed nits
• Improved error messages even further
• Merge on top of the trunk

Preparing to land

[GorNishanov]

preparing to land

[GorNishanov]

Commited as: https://reviews.llvm.org/rL297356

include/clang/Basic/DiagnosticSemaKinds.td

   "|a constexpr function|a function with a deduced return type"
   "|a varargs function}0">;
 def err_implied_std_coroutine_traits_not_found : Error<
   "you need to include <experimental/coroutine> before defining a coroutine">;
 def err_malformed_std_coroutine_traits : Error<
   "'std::experimental::coroutine_traits' must be a class template">;
 def err_implied_std_coroutine_traits_promise_type_not_found : Error<
   "this function cannot be a coroutine: %q0 has no member named 'promise_type'">;
+def err_malformed_std_coroutine_handle : Error<
+  "'std::experimental::coroutine_handle' must be a class template">;
 def err_implied_std_coroutine_traits_promise_type_not_class : Error<
   "this function cannot be a coroutine: %0 is not a class">;
 def err_coroutine_promise_type_incomplete : Error<
   "this function cannot be a coroutine: %0 is an incomplete type">;
 def err_coroutine_traits_missing_specialization : Error<
   "this function cannot be a coroutine: missing definition of "
   "specialization %q0">;
 def err_implied_std_current_exception_not_found : Error<

lib/Sema/SemaCoroutine.cpp

static QualType lookupPromiseType(Sema &S, const FunctionProtoType *FnType,
   }
   if (S.RequireCompleteType(FuncLoc, buildElaboratedType(),
                             diag::err_coroutine_promise_type_incomplete))
     return QualType();
 
   return PromiseType;
 }
 
+/// Look up the std::coroutine_traits<...>::promise_type for the given
+/// function type.
+static QualType lookupCoroutineHandleType(Sema &S, QualType PromiseType,
+                                          SourceLocation Loc) {
+  if (PromiseType.isNull())
+    return QualType();
+
+  NamespaceDecl *StdExp = S.lookupStdExperimentalNamespace();
+  assert(StdExp && "Should already be diagnosed");
+
+  LookupResult Result(S, &S.PP.getIdentifierTable().get("coroutine_handle"),
+                      Loc, Sema::LookupOrdinaryName);
+  if (!S.LookupQualifiedName(Result, StdExp)) {
+    S.Diag(Loc, diag::err_implied_std_coroutine_traits_not_found);
+    return QualType();
+  }
+
+  ClassTemplateDecl *CoroHandle = Result.getAsSingle<ClassTemplateDecl>();
+  if (!CoroHandle) {
+    Result.suppressDiagnostics();
+    // We found something weird. Complain about the first thing we found.
+    NamedDecl *Found = *Result.begin();
+    S.Diag(Found->getLocation(), diag::err_malformed_std_coroutine_handle);
+    return QualType();
+  }
+
+  // Form template argument list for coroutine_traits<R, P1, P2, ...>.
+  TemplateArgumentListInfo Args(Loc, Loc);
+  Args.addArgument(TemplateArgumentLoc(
+      TemplateArgument(PromiseType),
+      S.Context.getTrivialTypeSourceInfo(PromiseType, Loc)));
+
+  // Build the template-id.
+  QualType CoroHandleType =
+      S.CheckTemplateIdType(TemplateName(CoroHandle), Loc, Args);
+  if (CoroHandleType.isNull())
+    return QualType();
+  if (S.RequireCompleteType(Loc, CoroHandleType,
+                            diag::err_coroutine_traits_missing_specialization))
+    return QualType();
+
+  auto *RD = CoroHandleType->getAsCXXRecordDecl();
+  assert(RD && "specialization of class template is not a class?");
+
+  return CoroHandleType;
+}
+
 static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
                                     StringRef Keyword) {
   // 'co_await' and 'co_yield' are not permitted in unevaluated operands.
   if (S.isUnevaluatedContext()) {
     S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
     return false;
   }
 
static Expr *buildBuiltinCall(Sema &S, SourceLocation Loc, Builtin::ID Id,
 
   ExprResult Call =
       S.ActOnCallExpr(/*Scope=*/nullptr, DeclRef.get(), Loc, CallArgs, Loc);
 
   assert(!Call.isInvalid() && "Call to builtin cannot fail!");
   return Call.get();
 }
 
+static ExprResult buildCoroutineHandle(Sema &S, QualType PromiseType,
+                                       SourceLocation Loc) {
+  QualType HandleType = lookupCoroutineHandleType(S, PromiseType, Loc);
+  if (HandleType.isNull())
+    return ExprError();
+  auto *RD = HandleType->getAsCXXRecordDecl();
+  assert(RD && "must be class type");
+  DeclarationName DN = S.PP.getIdentifierInfo("from_address");
+  LookupResult LR(S, DN, Loc, Sema::LookupMemberName);
+  if (!S.LookupQualifiedName(LR, RD))
+    return ExprError();
+
+  Expr *FramePtr =
+      buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_frame, {});
+
+  // FIXME: Fix BuildMemberReferenceExpr to take a const CXXScopeSpec&.
+  CXXScopeSpec SS;
+  ExprResult Result = S.BuildMemberReferenceExpr(
+      /*BaseExpr*/ nullptr, HandleType, Loc, /*IsArrow=*/false, SS,
+      SourceLocation(), nullptr, LR, /*TemplateArgs=*/nullptr,
+      /*Scope=*/nullptr);
+  if (Result.isInvalid())
+    return ExprError();
+
+  return S.ActOnCallExpr(nullptr, Result.get(), Loc, FramePtr, Loc, nullptr);
+}
 
 struct ReadySuspendResumeResult {
   bool IsInvalid;
   Expr *Results[3];
 };
 
 static ExprResult buildMemberCall(Sema &S, Expr *Base, SourceLocation Loc,
                                   StringRef Name, MultiExprArg Args) {
if (Result.isInvalid())
     return ExprError();
 
   return S.ActOnCallExpr(nullptr, Result.get(), Loc, Args, Loc, nullptr);
 }
 
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema &S, SourceLocation Loc,
+                                                  QualType PromiseType,
                                                   Expr *E) {
   // Assume invalid until we see otherwise.
   ReadySuspendResumeResult Calls = {true, {}};
 
+  ExprResult HandleExprRes = buildCoroutineHandle(S, PromiseType, Loc);
+  if (HandleExprRes.isInvalid())
+    return Calls;
+  Expr *HandleExpr = HandleExprRes.get();
+
   const StringRef Funcs[] = {"await_ready", "await_suspend", "await_resume"};
   for (size_t I = 0, N = llvm::array_lengthof(Funcs); I != N; ++I) {
     Expr *Operand = new (S.Context) OpaqueValueExpr(
       Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
-    // FIXME: Pass coroutine handle to await_suspend.
-    ExprResult Result = buildMemberCall(S, Operand, Loc, Funcs[I], None);
+     MultiExprArg Args = None;
+    if (Funcs[I] == "await_suspend")
+      Args = HandleExpr;
+    ExprResult Result = buildMemberCall(S, Operand, Loc, Funcs[I], Args);
     if (Result.isInvalid())
       return Calls;
     Calls.Results[I] = Result.get();
   }
 
   Calls.IsInvalid = false;
   return Calls;
 }
ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
   }
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
   if (E->getValueKind() == VK_RValue)
     E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS = buildCoawaitCalls(*this, Loc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+          *this, Loc, Coroutine->CoroutinePromise->getType(), E);
   if (RSS.IsInvalid)
     return ExprError();
 
   Expr *Res = new (Context) CoawaitExpr(Loc, E, RSS.Results[0], RSS.Results[1],
                                         RSS.Results[2], IsImplicit);
   if (!IsImplicit)
     Coroutine->CoroutineStmts.push_back(Res);
   return Res;
ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr *E) {
   }
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
   if (E->getValueKind() == VK_RValue)
     E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS = buildCoawaitCalls(*this, Loc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+          *this, Loc, Coroutine->CoroutinePromise->getType(), E);
   if (RSS.IsInvalid)
     return ExprError();
 
   Expr *Res = new (Context) CoyieldExpr(Loc, E, RSS.Results[0], RSS.Results[1],
                                         RSS.Results[2]);
   Coroutine->CoroutineStmts.push_back(Res);
   return Res;
 }

 void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
   FunctionScopeInfo *Fn = getCurFunction();
   assert(Fn && Fn->CoroutinePromise && "not a coroutine");
 
   // Coroutines [stmt.return]p1:
   //   A return statement shall not appear in a coroutine.
   if (Fn->FirstReturnLoc.isValid()) {
     Diag(Fn->FirstReturnLoc, diag::err_return_in_coroutine);
+    // FIXME: Every Coroutine statement may be invalid and therefore not added
+    // to CoroutineStmts. Find another way to provide location information.
+    if (!Fn->CoroutineStmts.empty()) {
       auto *First = Fn->CoroutineStmts[0];
       Diag(First->getLocStart(), diag::note_declared_coroutine_here)
           << (isa<CoawaitExpr>(First) ? "co_await" :
               isa<CoyieldExpr>(First) ? "co_yield" : "co_return");
     }
+  }
+  if (Body && isa<CoroutineBodyStmt>(Body)) {
+    // FIXME(EricWF): Nothing todo. the body is already a transformed coroutine
+    // body statement.
+  } else {
     SubStmtBuilder Builder(*this, *FD, *Fn, Body);
     if (Builder.isInvalid())
       return FD->setInvalidDecl();
 
     // Build body for the coroutine wrapper statement.
     Body = CoroutineBodyStmt::Create(Context, Builder);
   }
+}
 
 bool SubStmtBuilder::makePromiseStmt() {
   // Form a declaration statement for the promise declaration, so that AST
   // visitors can more easily find it.
   StmtResult PromiseStmt =
       S.ActOnDeclStmt(S.ConvertDeclToDeclGroup(Fn.CoroutinePromise), Loc, Loc);
   if (PromiseStmt.isInvalid())
     return false;

lib/Sema/SemaExprMember.cpp

Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
   if (IsArrow) {
     assert(BaseType->isPointerType());
     BaseType = BaseType->castAs<PointerType>()->getPointeeType();
   }
   R.setBaseObjectType(BaseType);
 
   // C++1z [expr.ref]p2:
   //   For the first option (dot) the first expression shall be a glvalue [...]
-  if (!IsArrow && BaseExpr->isRValue()) {
+  if (!IsArrow && BaseExpr && BaseExpr->isRValue()) {
     ExprResult Converted = TemporaryMaterializationConversion(BaseExpr);
     if (Converted.isInvalid())
       return ExprError();
     BaseExpr = Converted.get();
   }
   
   LambdaScopeInfo *const CurLSI = getCurLambda();
   // If this is an implicit member reference and the overloaded

test/CodeGenCoroutines/coro-alloc.cpp

 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 namespace std {
 namespace experimental {
 template <typename... T>
 struct coroutine_traits; // expected-note {{declared here}}
+
+template <class Promise = void>
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) { return {}; }
+};
+
+template <>
+struct coroutine_handle<void> {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template <class PromiseType>
+  coroutine_handle(coroutine_handle<PromiseType>) {}
+};
+
 }
 }
 
 struct suspend_always {
   bool await_ready() { return false; }
-  void await_suspend() {}
+  void await_suspend(std::experimental::coroutine_handle<>) {}
   void await_resume() {}
 };
 
 struct global_new_delete_tag {};
 
 template<>
 struct std::experimental::coroutine_traits<void, global_new_delete_tag> {
   struct promise_type {

test/SemaCXX/coroutines.cpp

 }
 
 
 void no_coroutine_traits_bad_arg_return() {
   co_return a; // expected-error {{include <experimental/coroutine>}}
   // expected-error@-1 {{use of undeclared identifier 'a'}}
 }
 
+void no_coroutine_traits() {
+  co_await 4; // expected-error {{need to include <experimental/coroutine>}}
+}
+
+namespace std {
+namespace experimental {
+template <typename... T>
+struct coroutine_traits; // expected-note {{declared here}}
+
+template <class PromiseType = void>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *);
+};
+
+template <>
+struct coroutine_handle<void> {
+  template <class PromiseType>
+  coroutine_handle(coroutine_handle<PromiseType>);
+  static coroutine_handle from_address(void *);
+};
+}} // namespace std::experimental
+
+template<typename Promise> struct coro {};
+template <typename Promise, typename... Ps>
+struct std::experimental::coroutine_traits<coro<Promise>, Ps...> {
+  using promise_type = Promise;
+};
+
 
 struct awaitable {
   bool await_ready();
-  void await_suspend(); // FIXME: coroutine_handle
+  void await_suspend(std::experimental::coroutine_handle<>);
   void await_resume();
 } a;
 
 struct suspend_always {
   bool await_ready() { return false; }
-  void await_suspend() {}
+  void await_suspend(std::experimental::coroutine_handle<>) {}
   void await_resume() {}
 };
 
 struct suspend_never {
   bool await_ready() { return true; }
-  void await_suspend() {}
+  void await_suspend(std::experimental::coroutine_handle<>) {}
   void await_resume() {}
 };
 
-void no_coroutine_traits() {
-  co_await a; // expected-error {{need to include <experimental/coroutine>}}
-}
-
-namespace std {
-namespace experimental {
-template <typename... T>
-struct coroutine_traits; // expected-note {{declared here}}
-}
-}
-
-template<typename Promise> struct coro {};
-template <typename Promise, typename... Ps>
-struct std::experimental::coroutine_traits<coro<Promise>, Ps...> {
-  using promise_type = Promise;
-};
 
 void no_specialization() {
   co_await a; // expected-error {{implicit instantiation of undefined template 'std::experimental::coroutine_traits<void>'}}
 }
 
 template <typename... T>
 struct std::experimental::coroutine_traits<int, T...> {};
 

 struct promise_void;
 struct void_tag {};
 template <typename... T>
 struct std::experimental::coroutine_traits<void, T...> { using promise_type = promise; };
 template <typename... T>
 struct std::experimental::coroutine_traits<void, void_tag, T...>
 { using promise_type = promise_void; };
 
-namespace std {
-namespace experimental {
-template <typename Promise = void>
-struct coroutine_handle;
-}
-}
-
 // FIXME: This diagnostic is terrible.
 void undefined_promise() { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits<void>::promise_type' (aka 'promise') is an incomplete type}}
   co_await a;
 }
 
 struct yielded_thing { const char *p; short a, b; };
 
 struct not_awaitable {};
coro<good_promise_1> ok_static_coawait() {
   co_await 42;
 }
 
 template<> struct std::experimental::coroutine_traits<int, int, const char**>
 { using promise_type = promise; };
 
 int main(int, const char**) {
   co_await a; // expected-error {{'co_await' cannot be used in the 'main' function}}
 }