r285271 - [coroutines] Build fallthrough and set_exception statements.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 27 00:30:32 PDT 2016


Author: ericwf
Date: Thu Oct 27 02:30:31 2016
New Revision: 285271

URL: http://llvm.org/viewvc/llvm-project?rev=285271&view=rev
Log:
[coroutines] Build fallthrough and set_exception statements.

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. 


Reviewers: majnemer, GorNishanov, rsmith

Subscribers: mehdi_amini, cfe-commits

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

Added:
    cfe/trunk/test/SemaCXX/coreturn.cpp
Modified:
    cfe/trunk/include/clang/AST/StmtCXX.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
    cfe/trunk/lib/Sema/SemaCoroutine.cpp
    cfe/trunk/lib/Sema/TreeTransform.h
    cfe/trunk/test/SemaCXX/coroutines.cpp

Modified: cfe/trunk/include/clang/AST/StmtCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtCXX.h?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/StmtCXX.h (original)
+++ cfe/trunk/include/clang/AST/StmtCXX.h Thu Oct 27 02:30:31 2016
@@ -405,10 +405,13 @@ public:
 
   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);
   }
 

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 27 02:30:31 2016
@@ -512,6 +512,12 @@ def err_maybe_falloff_nonvoid_block : Er
   "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;
@@ -8643,6 +8649,13 @@ def err_implied_std_coroutine_traits_pro
 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 implicitly "
+  "uses '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 {

Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Oct 27 02:30:31 2016
@@ -365,7 +365,7 @@ static ControlFlowKind CheckFallThrough(
 
     CFGStmt CS = ri->castAs<CFGStmt>();
     const Stmt *S = CS.getStmt();
-    if (isa<ReturnStmt>(S)) {
+    if (isa<ReturnStmt>(S) || isa<CoreturnStmt>(S)) {
       HasLiveReturn = true;
       continue;
     }
@@ -416,7 +416,7 @@ struct CheckFallThroughDiagnostics {
   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) {
@@ -452,6 +452,19 @@ struct CheckFallThroughDiagnostics {
     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 =
@@ -494,7 +507,13 @@ struct CheckFallThroughDiagnostics {
              (!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;
   }
@@ -514,11 +533,14 @@ static void CheckFallThroughForBody(Sema
   bool ReturnsVoid = false;
   bool HasNoReturn = false;
 
-  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
-    ReturnsVoid = FD->getReturnType()->isVoidType();
+  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+    if (const auto *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)) {
+  else if (const auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
     ReturnsVoid = MD->getReturnType()->isVoidType();
     HasNoReturn = MD->hasAttr<NoReturnAttr>();
   }
@@ -1986,13 +2008,22 @@ AnalysisBasedWarnings::IssueWarnings(sem
   
   // 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<CXXMethodDecl>(D) &&
-          cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
-          cast<CXXMethodDecl>(D)->getParent()->isLambda())
-            ? CheckFallThroughDiagnostics::MakeForLambda()
-            : CheckFallThroughDiagnostics::MakeForFunction(D));
+        (isa<BlockDecl>(D)
+             ? CheckFallThroughDiagnostics::MakeForBlock()
+             : (isa<CXXMethodDecl>(D) &&
+                cast<CXXMethodDecl>(D)->getOverloadedOperator() == OO_Call &&
+                cast<CXXMethodDecl>(D)->getParent()->isLambda())
+                   ? CheckFallThroughDiagnostics::MakeForLambda()
+                   : (IsCoro()
+                          ? CheckFallThroughDiagnostics::MakeForCoroutine(D)
+                          : CheckFallThroughDiagnostics::MakeForFunction(D)));
     CheckFallThroughForBody(S, D, Body, blkExpr, CD, AC);
   }
 

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Thu Oct 27 02:30:31 2016
@@ -378,6 +378,34 @@ StmtResult Sema::BuildCoreturnStmt(Sourc
   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");
@@ -432,10 +460,59 @@ void Sema::CheckCompletedCoroutineBody(F
   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.
@@ -462,6 +539,5 @@ void Sema::CheckCompletedCoroutineBody(F
   // 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);
 }

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Thu Oct 27 02:30:31 2016
@@ -6654,6 +6654,7 @@ 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());
 }
 

Added: cfe/trunk/test/SemaCXX/coreturn.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coreturn.cpp?rev=285271&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/coreturn.cpp (added)
+++ cfe/trunk/test/SemaCXX/coreturn.cpp Thu Oct 27 02:30:31 2016
@@ -0,0 +1,73 @@
+// 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();
+  void return_void();
+};
+
+struct promise_float {
+  float get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  void return_void();
+};
+
+struct promise_int {
+  int get_return_object();
+  suspend_always initial_suspend();
+  suspend_always final_suspend();
+  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;
+}

Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=285271&r1=285270&r2=285271&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Thu Oct 27 02:30:31 2016
@@ -118,9 +118,6 @@ struct promise_void {
   void get_return_object();
   suspend_always initial_suspend();
   suspend_always final_suspend();
-  awaitable yield_value(int);
-  awaitable yield_value(yielded_thing);
-  not_awaitable yield_value(void());
   void return_void();
 };
 
@@ -313,6 +310,47 @@ coro<bad_promise_5> bad_final_suspend()
   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 at -1 {{you need to include <exception> before defining a coroutine that implicitly uses '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 at -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; };




More information about the cfe-commits mailing list