[coroutines] Build fallthrough and set_exception statements.

[EricWF]

Summary

This patch adds semantic checking and building of the fall-through co_return; statement as well as the p.set_exception(std::current_exception()) call for handling uncaught exceptions.

The fall-through statement is built and checked according to:

[dcl.fct.def.coroutine]/4
The unqualified-ids return_void and return_value are looked up in the scope of class P. If
both are found, the program is ill-formed. If the unqualified-id return_void is found, flowing
off the end of a coroutine is equivalent to a co_return with no operand. Otherwise, flowing off
the end of a coroutine results in undefined behavior.

Similarly the set_exception call is only built when that unqualified-id is found in the scope of class P.

Additionally this patch adds fall-through warnings for non-void returning coroutines. Since it's surprising undefined behavior I thought it would be important to add the warning right away.

[EricWF]

• Remove useless line of code.

[GorNishanov]

test/SemaCXX/coreturn.cpp

36

No need to hve yield_value here, unless you want to use co_yield expression in the coroutine. In this file, we are using co_await only. (Same comment for promise_float and promise_int)

[majnemer]

lib/Sema/AnalysisBasedWarnings.cpp

537

const auto *

[EricWF]

Address review comments.

test/SemaCXX/coreturn.cpp

36

Ack. I'll add them back if I need them.

[GorNishanov]

LGTM, but, will need to wait for @rsmith to sign off.

[GorNishanov]

@rsmith, I am wondering what were your thoughts on where to generate try { body } catch (...) { p.set_exception(std::exception()); }

Would it be in SemaCoroutine.cpp? Essentially, add something like this:

`
  bool makeBody() {
    if (!OnException)
      return true;

    StmtResult CatchBlock = S.ActOnCXXCatchBlock(Loc, nullptr, OnException);
    if (CatchBlock.isInvalid())
      return false;

    StmtResult TryBlock = S.ActOnCXXTryBlock(Loc, Body, {CatchBlock.get()});
    if (TryBlock.isInvalid())
      return false;

    Body = TryBlock.get();

    return true;
  }
`

[rsmith]

In D25349#571778, @GorNishanov wrote:

@rsmith, I am wondering what were your thoughts on where to generate try { body } catch (...) { p.set_exception(std::exception()); }

Would it be in SemaCoroutine.cpp?

Ideally, we'd avoid doing this desugaring at all, and just generate the correct code from CGCoroutine.cpp. Generally, we try to avoid desugaring in the AST whenever we can.

include/clang/AST/StmtCXX.h

public:
   /// statement. Will be nullptr if either the coroutine has not yet been
   /// finalized or the coroutine has no eventual return type.
   Expr *getPromiseCall() const {
     return static_cast<Expr*>(SubStmts[PromiseCall]);
   }
 
   SourceLocation getLocStart() const LLVM_READONLY { return CoreturnLoc; }
   SourceLocation getLocEnd() const LLVM_READONLY {
-    return getOperand()->getLocEnd();
+    return getOperand() ? getOperand()->getLocEnd() : getLocStart();
   }
 
   child_range children() {
+    if (!getOperand())
+      return child_range(SubStmts + SubStmt::PromiseCall,
+                         SubStmts + SubStmt::Count);
     return child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CoreturnStmtClass;
   }
 };
 
 }  // end namespace clang
 
 #endif

include/clang/Basic/DiagnosticSemaKinds.td

   InGroup<ReturnType>;
 def warn_falloff_nonvoid_function : Warning<
   "control reaches end of non-void function">,
   InGroup<ReturnType>;
 def err_maybe_falloff_nonvoid_block : Error<
   "control may reach end of non-void block">;
 def err_falloff_nonvoid_block : Error<
   "control reaches end of non-void block">;
+def warn_maybe_falloff_nonvoid_coroutine : Warning<
+  "control may reach end of non-void coroutine">,
+  InGroup<ReturnType>;
+def warn_falloff_nonvoid_coroutine : Warning<
+  "control reaches end of non-void coroutine">,
+  InGroup<ReturnType>;
 def warn_suggest_noreturn_function : Warning<
   "%select{function|method}0 %1 could be declared with attribute 'noreturn'">,
   InGroup<MissingNoreturn>, DefaultIgnore;
 def warn_suggest_noreturn_block : Warning<
   "block could be declared with attribute 'noreturn'">,
   InGroup<MissingNoreturn>, DefaultIgnore;
 
 // Unreachable code.

   "'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_implied_std_coroutine_traits_promise_type_not_class : Error<
   "this function cannot be a coroutine: %0 is not a class">;
 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<
+  "you need to include <exception> before defining a coroutine that declares "
+  "'set_exception'">;
+def err_malformed_std_current_exception : Error<
+  "'std::current_exception' must be a function">;
+def err_coroutine_promise_return_ill_formed : Error<
+  "%0 declares both 'return_value' and 'return_void'">;
 }
 
 let CategoryName = "Documentation Issue" in {
 def warn_not_a_doxygen_trailing_member_comment : Warning<
   "not a Doxygen trailing comment">, InGroup<Documentation>, DefaultIgnore;
 } // end of documentation issue category
 
 let CategoryName = "Instrumentation Issue" in {

lib/Sema/AnalysisBasedWarnings.cpp

if (ri == re) {
       }
       // A labeled empty statement, or the entry block...
       HasPlainEdge = true;
       continue;
     }
 
     CFGStmt CS = ri->castAs<CFGStmt>();
     const Stmt *S = CS.getStmt();
-    if (isa<ReturnStmt>(S)) {
+    if (isa<ReturnStmt>(S) || isa<CoreturnStmt>(S)) {
       HasLiveReturn = true;
       continue;
     }
     if (isa<ObjCAtThrowStmt>(S)) {
       HasFakeEdge = true;
       continue;
     }
     if (isa<CXXThrowExpr>(S)) {

 namespace {
 
 struct CheckFallThroughDiagnostics {
   unsigned diag_MaybeFallThrough_HasNoReturn;
   unsigned diag_MaybeFallThrough_ReturnsNonVoid;
   unsigned diag_AlwaysFallThrough_HasNoReturn;
   unsigned diag_AlwaysFallThrough_ReturnsNonVoid;
   unsigned diag_NeverFallThroughOrReturn;
-  enum { Function, Block, Lambda } funMode;
+  enum { Function, Block, Lambda, Coroutine } funMode;
   SourceLocation FuncLoc;
 
   static CheckFallThroughDiagnostics MakeForFunction(const Decl *Func) {
     CheckFallThroughDiagnostics D;
     D.FuncLoc = Func->getLocation();
     D.diag_MaybeFallThrough_HasNoReturn =
       diag::warn_falloff_noreturn_function;
     D.diag_MaybeFallThrough_ReturnsNonVoid =
if (!isVirtualMethod && !isTemplateInstantiation)
         diag::warn_suggest_noreturn_function;
     else
       D.diag_NeverFallThroughOrReturn = 0;
     
     D.funMode = Function;
     return D;
   }
 
+  static CheckFallThroughDiagnostics MakeForCoroutine(const Decl *Func) {
+    CheckFallThroughDiagnostics D;
+    D.FuncLoc = Func->getLocation();
+    D.diag_MaybeFallThrough_HasNoReturn = 0;
+    D.diag_MaybeFallThrough_ReturnsNonVoid =
+        diag::warn_maybe_falloff_nonvoid_coroutine;
+    D.diag_AlwaysFallThrough_HasNoReturn = 0;
+    D.diag_AlwaysFallThrough_ReturnsNonVoid =
+        diag::warn_falloff_nonvoid_coroutine;
+    D.funMode = Coroutine;
+    return D;
+  }
+
   static CheckFallThroughDiagnostics MakeForBlock() {
     CheckFallThroughDiagnostics D;
     D.diag_MaybeFallThrough_HasNoReturn =
       diag::err_noreturn_block_has_return_expr;
     D.diag_MaybeFallThrough_ReturnsNonVoid =
       diag::err_maybe_falloff_nonvoid_block;
     D.diag_AlwaysFallThrough_HasNoReturn =
       diag::err_noreturn_block_has_return_expr;
if (funMode == Function) {
               D.isIgnored(diag::warn_maybe_falloff_nonvoid_function,
                           FuncLoc)) &&
              (!HasNoReturn ||
               D.isIgnored(diag::warn_noreturn_function_has_return_expr,
                           FuncLoc)) &&
              (!ReturnsVoid ||
               D.isIgnored(diag::warn_suggest_noreturn_block, FuncLoc));
     }
-
+    if (funMode == Coroutine) {
+      return (ReturnsVoid ||
+              D.isIgnored(diag::warn_maybe_falloff_nonvoid_function, FuncLoc) ||
+              D.isIgnored(diag::warn_maybe_falloff_nonvoid_coroutine,
+                          FuncLoc)) &&
+             (!HasNoReturn);
+    }
     // For blocks / lambdas.
     return ReturnsVoid && !HasNoReturn;
   }
 };
 
 } // anonymous namespace
 
 /// CheckFallThroughForFunctionDef - Check that we don't fall off the end of a
 /// function that should return a value.  Check that we don't fall off the end
 /// of a noreturn function.  We assume that functions and blocks not marked
 /// noreturn will return.
 static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
                                     const BlockExpr *blkExpr,
                                     const CheckFallThroughDiagnostics& CD,
                                     AnalysisDeclContext &AC) {
 
   bool ReturnsVoid = false;
   bool HasNoReturn = false;
 
   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+    if (const CoroutineBodyStmt *CBody = dyn_cast<CoroutineBodyStmt>(Body))
+      ReturnsVoid = CBody->getFallthroughHandler() != nullptr;
+    else
       ReturnsVoid = FD->getReturnType()->isVoidType();
     HasNoReturn = FD->isNoReturn();
   }
   else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
     ReturnsVoid = MD->getReturnType()->isVoidType();
     HasNoReturn = MD->hasAttr<NoReturnAttr>();
   }
   else if (isa<BlockDecl>(D)) {
     QualType BlockTy = blkExpr->getType();
if (!fscope->PossiblyUnreachableDiags.empty()) {
     }
 
     if (!analyzed)
       flushDiagnostics(S, fscope);
   }
   
   // Warning: check missing 'return'
   if (P.enableCheckFallThrough) {
+    auto IsCoro = [&]() {
+      if (auto *FD = dyn_cast<FunctionDecl>(D))
+        if (FD->getBody() && isa<CoroutineBodyStmt>(FD->getBody()))
+          return true;
+      return false;
+    };
     const CheckFallThroughDiagnostics &CD =
-      (isa<BlockDecl>(D) ? CheckFallThroughDiagnostics::MakeForBlock()
+        (isa<BlockDecl>(D)
+             ? CheckFallThroughDiagnostics::MakeForBlock()
              : (isa<CXXMethodDecl>(D) &&
                 cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
                 cast<CXXMethodDecl>(D)->getParent()->isLambda())
                    ? CheckFallThroughDiagnostics::MakeForLambda()
-            : CheckFallThroughDiagnostics::MakeForFunction(D));
+                   : (IsCoro()
+                          ? CheckFallThroughDiagnostics::MakeForCoroutine(D)
+                          : CheckFallThroughDiagnostics::MakeForFunction(D)));
     CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC);
   }
 
   // Warning: check for unreachable code
   if (P.enableCheckUnreachable) {
     // Only check for unreachable code on non-template instantiations.
     // Different template instantiations can effectively change the control-flow
     // and it is very difficult to prove that a snippet of code in a template

lib/Sema/SemaCoroutine.cpp

StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E) {
 
   Expr *PCE = ActOnFinishFullExpr(PC.get()).get();
 
   Stmt *Res = new (Context) CoreturnStmt(Loc, E, PCE);
   Coroutine->CoroutineStmts.push_back(Res);
   return Res;
 }
 
+static ExprResult buildStdCurrentExceptionCall(Sema &S, SourceLocation Loc) {
+  NamespaceDecl *Std = S.getStdNamespace();
+  if (!Std) {
+    S.Diag(Loc, diag::err_implied_std_current_exception_not_found);
+    return ExprError();
+  }
+  LookupResult Result(S, &S.PP.getIdentifierTable().get("current_exception"),
+                      Loc, Sema::LookupOrdinaryName);
+  if (!S.LookupQualifiedName(Result, Std)) {
+    S.Diag(Loc, diag::err_implied_std_current_exception_not_found);
+    return ExprError();
+  }
+
+  // FIXME The STL is free to provide more than one overload.
+  FunctionDecl *FD = Result.getAsSingle<FunctionDecl>();
+  if (!FD) {
+    S.Diag(Loc, diag::err_malformed_std_current_exception);
+    return ExprError();
+  }
+  ExprResult Res = S.BuildDeclRefExpr(FD, FD->getType(), VK_LValue, Loc);
+  Res = S.ActOnCallExpr(/*Scope*/ nullptr, Res.get(), Loc, None, Loc);
+  if (Res.isInvalid()) {
+    S.Diag(Loc, diag::err_malformed_std_current_exception);
+    return ExprError();
+  }
+  return Res;
+}
+
 void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) {
   FunctionScopeInfo *Fn = getCurFunction();
   assert(Fn && !Fn->CoroutineStmts.empty() && "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);
ExprResult FinalSuspend =
       buildPromiseCall(*this, Fn, Loc, "final_suspend", None);
   // FIXME: Support operator co_await here.
   if (!FinalSuspend.isInvalid())
     FinalSuspend = BuildCoawaitExpr(Loc, FinalSuspend.get());
   FinalSuspend = ActOnFinishFullExpr(FinalSuspend.get());
   if (FinalSuspend.isInvalid())
     return FD->setInvalidDecl();
 
-  // FIXME: Perform analysis of set_exception call.
-
-  // FIXME: Try to form 'p.return_void();' expression statement to handle
+  // Try to form 'p.return_void();' expression statement to handle
   // control flowing off the end of the coroutine.
+  // Also try to form 'p.set_exception(std::current_exception());' to handle
+  // uncaught exceptions.
+  ExprResult SetException;
+  StmtResult Fallthrough;
+  if (Fn->CoroutinePromise &&
+      !Fn->CoroutinePromise->getType()->isDependentType()) {
+    CXXRecordDecl *RD = Fn->CoroutinePromise->getType()->getAsCXXRecordDecl();
+    assert(RD && "Type should have already been checked");
+    // [dcl.fct.def.coroutine]/4
+    // The unqualified-ids 'return_void' and 'return_value' are looked up in
+    // the scope of class P. If both are found, the program is ill-formed.
+    DeclarationName RVoidDN = PP.getIdentifierInfo("return_void");
+    LookupResult RVoidResult(*this, RVoidDN, Loc, Sema::LookupMemberName);
+    const bool HasRVoid = LookupQualifiedName(RVoidResult, RD);
+
+    DeclarationName RValueDN = PP.getIdentifierInfo("return_value");
+    LookupResult RValueResult(*this, RValueDN, Loc, Sema::LookupMemberName);
+    const bool HasRValue = LookupQualifiedName(RValueResult, RD);
+
+    if (HasRVoid && HasRValue) {
+      // FIXME Improve this diagnostic
+      Diag(FD->getLocation(), diag::err_coroutine_promise_return_ill_formed)
+          << RD;
+      return FD->setInvalidDecl();
+    } else if (HasRVoid) {
+      // If the unqualified-id return_void is found, flowing off the end of a
+      // coroutine is equivalent to a co_return with no operand. Otherwise,
+      // flowing off the end of a coroutine results in undefined behavior.
+      Fallthrough = BuildCoreturnStmt(FD->getLocation(), nullptr);
+      Fallthrough = ActOnFinishFullStmt(Fallthrough.get());
+      if (Fallthrough.isInvalid())
+        return FD->setInvalidDecl();
+    }
+
+    // [dcl.fct.def.coroutine]/3
+    // The unqualified-id set_exception is found in the scope of P by class
+    // member access lookup (3.4.5).
+    DeclarationName SetExDN = PP.getIdentifierInfo("set_exception");
+    LookupResult SetExResult(*this, SetExDN, Loc, Sema::LookupMemberName);
+    if (LookupQualifiedName(SetExResult, RD)) {
+      // Form the call 'p.set_exception(std::current_exception())'
+      SetException = buildStdCurrentExceptionCall(*this, Loc);
+      if (SetException.isInvalid())
+        return FD->setInvalidDecl();
+      Expr *E = SetException.get();
+      SetException = buildPromiseCall(*this, Fn, Loc, "set_exception", E);
+      SetException = ActOnFinishFullExpr(SetException.get(), Loc);
+      if (SetException.isInvalid())
+        return FD->setInvalidDecl();
+    }
+  }
 
   // Build implicit 'p.get_return_object()' expression and form initialization
   // of return type from it.
   ExprResult ReturnObject =
     buildPromiseCall(*this, Fn, Loc, "get_return_object", None);
   if (ReturnObject.isInvalid())
     return FD->setInvalidDecl();
   QualType RetType = FD->getReturnType();
if (ReturnObject.isInvalid())
     return FD->setInvalidDecl();
 
   // FIXME: Perform move-initialization of parameters into frame-local copies.
   SmallVector<Expr*, 16> ParamMoves;
 
   // Build body for the coroutine wrapper statement.
   Body = new (Context) CoroutineBodyStmt(
       Body, PromiseStmt.get(), InitialSuspend.get(), FinalSuspend.get(),
-      /*SetException*/nullptr, /*Fallthrough*/nullptr,
-      ReturnObject.get(), ParamMoves);
+      SetException.get(), Fallthrough.get(), ReturnObject.get(), ParamMoves);
 }

lib/Sema/TreeTransform.h

 }
 
 // C++ Coroutines TS
 
 template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
   // The coroutine body should be re-formed by the caller if necessary.
+  // FIXME: The coroutine body is always rebuilt by ActOnFinishFunctionBody
   return getDerived().TransformStmt(S->getBody());
 }
 
 template<typename Derived>
 StmtResult
 TreeTransform<Derived>::TransformCoreturnStmt(CoreturnStmt *S) {
   ExprResult Result = getDerived().TransformInitializer(S->getOperand(),
                                                         /*NotCopyInit*/false);

test/SemaCXX/coreturn.cpp

+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wno-unreachable-code -Wno-unused-value
+
+struct awaitable {
+  bool await_ready();
+  void await_suspend(); // FIXME: coroutine_handle
+  void await_resume();
+} a;
+
+struct suspend_always {
+  bool await_ready() { return false; }
+  void await_suspend() {}
+  void await_resume() {}
+};
+
+struct suspend_never {
+  bool await_ready() { return true; }
+  void await_suspend() {}
+  void await_resume() {}
+};
+
+namespace std {
+namespace experimental {
+
+template <class Ret, typename... T>
+struct coroutine_traits { using promise_type = typename Ret::promise_type; };
+
+template <class Promise = void>
+struct coroutine_handle {};
+}
+}
+
+struct promise_void {
+  void get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  awaitable yield_value(int);
+  void return_void();
+};
+
+struct promise_float {
+  float get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  awaitable yield_value(int);
+  void return_void();
+};
+
+struct promise_int {
+  int get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  awaitable yield_value(int);
+  void return_value(int);
+};
+
+template <typename... T>
+struct std::experimental::coroutine_traits<void, T...> { using promise_type = promise_void; };
+
+template <typename... T>
+struct std::experimental::coroutine_traits<float, T...> { using promise_type = promise_float; };
+
+template <typename... T>
+struct std::experimental::coroutine_traits<int, T...> { using promise_type = promise_int; };
+
+void test0() { co_await a; }
+float test1() { co_await a; }
+
+int test2() {
+  co_await a;
+} // expected-warning {{control reaches end of non-void coroutine}}
+
+int test3() {
+  co_await a;
+b:
+  goto b;
+}

test/SemaCXX/coroutines.cpp

struct bad_promise_5 {
   suspend_always initial_suspend();
   not_awaitable final_suspend();
 };
 // FIXME: This diagnostic is terrible.
 coro<bad_promise_5> bad_final_suspend() { // expected-error {{no member named 'await_ready' in 'not_awaitable'}}
   co_await a;
 }
 
+struct bad_promise_6 {
+  coro<bad_promise_6> get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void return_void();
+  void return_value(int) const;
+  void return_value(int);
+};
+coro<bad_promise_6> bad_implicit_return() { // expected-error {{'bad_promise_6' declares both 'return_value' and 'return_void'}}
+  co_await a;
+}
+
+struct bad_promise_7 {
+  coro<bad_promise_7> get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void return_void();
+  void set_exception(int *);
+};
+coro<bad_promise_7> no_std_current_exc() {
+  // expected-error@-1 {{you need to include <exception> before defining a coroutine that declares 'set_exception'}}
+  co_await a;
+}
+
+namespace std {
+int *current_exception();
+}
+
+struct bad_promise_8 {
+  coro<bad_promise_8> get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void return_void();
+  void set_exception();                                   // expected-note {{function not viable}}
+  void set_exception(int *) __attribute__((unavailable)); // expected-note {{explicitly made unavailable}}
+  void set_exception(void *);                             // expected-note {{candidate function}}
+};
+coro<bad_promise_8> calls_set_exception() {
+  // expected-error@-1 {{call to unavailable member function 'set_exception'}}
+  co_await a;
+}
 
 template<> struct std::experimental::coroutine_traits<int, int, const char**>
 { using promise_type = promise; };
 
 int main(int, const char**) { // expected-error {{'main' cannot be a coroutine}}
   co_await a; // expected-note {{function is a coroutine due to use of 'co_await' here}}
 }