r356774 - Revert "[coroutines][PR40978] Emit error for co_yield within catch block"

Brian Gesiak via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 22 09:08:29 PDT 2019


Author: modocache
Date: Fri Mar 22 09:08:29 2019
New Revision: 356774

URL: http://llvm.org/viewvc/llvm-project?rev=356774&view=rev
Log:
Revert "[coroutines][PR40978] Emit error for co_yield within catch block"

The commit https://reviews.llvm.org/rC356296 is causing a regression in nested
catch scopes, https://bugs.llvm.org/show_bug.cgi?id=41171. Revert this change
for now in order to un-break that problem report.


Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Scope.h
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Sema/Scope.cpp
    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=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Mar 22 09:08:29 2019
@@ -9271,8 +9271,6 @@ def err_coroutine_objc_method : Error<
   "Objective-C methods as coroutines are not yet supported">;
 def err_coroutine_unevaluated_context : Error<
   "'%0' cannot be used in an unevaluated context">;
-def err_coroutine_within_handler : Error<
-  "'%0' cannot be used in the handler of a try block">;
 def err_coroutine_outside_function : Error<
   "'%0' cannot be used outside a function">;
 def err_coroutine_invalid_func_context : Error<

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Fri Mar 22 09:08:29 2019
@@ -131,9 +131,6 @@ public:
 
     /// We are between inheritance colon and the real class/struct definition scope.
     ClassInheritanceScope = 0x800000,
-
-    /// This is the scope of a C++ catch statement.
-    CatchScope = 0x1000000,
   };
 
 private:

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Fri Mar 22 09:08:29 2019
@@ -2260,10 +2260,8 @@ StmtResult Parser::ParseCXXCatchBlock(bo
   // C++ 3.3.2p3:
   // The name in a catch exception-declaration is local to the handler and
   // shall not be redeclared in the outermost block of the handler.
-  unsigned ScopeFlags = Scope::DeclScope | Scope::ControlScope |
-                        Scope::CatchScope |
-                        (FnCatch ? Scope::FnTryCatchScope : 0);
-  ParseScope CatchScope(this, ScopeFlags);
+  ParseScope CatchScope(this, Scope::DeclScope | Scope::ControlScope |
+                          (FnCatch ? Scope::FnTryCatchScope : 0));
 
   // exception-declaration is equivalent to '...' or a parameter-declaration
   // without default arguments.
@@ -2292,7 +2290,7 @@ StmtResult Parser::ParseCXXCatchBlock(bo
     return StmtError(Diag(Tok, diag::err_expected) << tok::l_brace);
 
   // FIXME: Possible draft standard bug: attribute-specifier should be allowed?
-  StmtResult Block(ParseCompoundStatement(/*isStmtExpr=*/false, ScopeFlags));
+  StmtResult Block(ParseCompoundStatement());
   if (Block.isInvalid())
     return Block;
 

Modified: cfe/trunk/lib/Sema/Scope.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Scope.cpp?rev=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Scope.cpp (original)
+++ cfe/trunk/lib/Sema/Scope.cpp Fri Mar 22 09:08:29 2019
@@ -166,9 +166,7 @@ void Scope::dumpImpl(raw_ostream &OS) co
       {SEHExceptScope, "SEHExceptScope"},
       {SEHFilterScope, "SEHFilterScope"},
       {CompoundStmtScope, "CompoundStmtScope"},
-      {ClassInheritanceScope, "ClassInheritanceScope"},
-      {CatchScope, "CatchScope"},
-  };
+      {ClassInheritanceScope, "ClassInheritanceScope"}};
 
   for (auto Info : FlagInfo) {
     if (Flags & Info.first) {

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Fri Mar 22 09:08:29 2019
@@ -185,8 +185,21 @@ static QualType lookupCoroutineHandleTyp
 
 static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
                                     StringRef Keyword) {
-  // [expr.await]p2 dictates that 'co_await' and 'co_yield' must be used within
-  // a function body.
+  // 'co_await' and 'co_yield' are not permitted in unevaluated operands,
+  // such as subexpressions of \c sizeof.
+  //
+  // [expr.await]p2, emphasis added: "An await-expression shall appear only in
+  // a *potentially evaluated* expression within the compound-statement of a
+  // function-body outside of a handler [...] A context within a function where
+  // an await-expression can appear is called a suspension context of the
+  // function." And per [expr.yield]p1: "A yield-expression shall appear only
+  // within a suspension context of a function."
+  if (S.isUnevaluatedContext()) {
+    S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
+    return false;
+  }
+
+  // Per [expr.await]p2, any other usage must be within a function.
   // FIXME: This also covers [expr.await]p2: "An await-expression shall not
   // appear in a default argument." But the diagnostic QoI here could be
   // improved to inform the user that default arguments specifically are not
@@ -655,57 +668,12 @@ bool Sema::ActOnCoroutineBodyStart(Scope
   return true;
 }
 
-// Recursively walks up the scope hierarchy until either a 'catch' or a function
-// scope is found, whichever comes first.
-static bool isWithinCatchScope(Scope *S) {
-  // 'co_await' and 'co_yield' keywords are disallowed within catch blocks, but
-  // lambdas that use 'co_await' are allowed. The loop below ends when a
-  // function scope is found in order to ensure the following behavior:
-  //
-  // void foo() {      // <- function scope
-  //   try {           //
-  //     co_await x;   // <- 'co_await' is OK within a function scope
-  //   } catch {       // <- catch scope
-  //     co_await x;   // <- 'co_await' is not OK within a catch scope
-  //     []() {        // <- function scope
-  //       co_await x; // <- 'co_await' is OK within a function scope
-  //     }();
-  //   }
-  // }
-  while (S && !(S->getFlags() & Scope::FnScope)) {
-    if (S->getFlags() & Scope::CatchScope)
-      return true;
-    S = S->getParent();
-  }
-  return false;
-}
-
-// [expr.await]p2, emphasis added: "An await-expression shall appear only in
-// a *potentially evaluated* expression within the compound-statement of a
-// function-body *outside of a handler* [...] A context within a function
-// where an await-expression can appear is called a suspension context of the
-// function."
-static void checkSuspensionContext(Sema &S, SourceLocation Loc,
-                                   StringRef Keyword) {
-  // First emphasis of [expr.await]p2: must be a potentially evaluated context.
-  // That is, 'co_await' and 'co_yield' cannot appear in subexpressions of
-  // \c sizeof.
-  if (S.isUnevaluatedContext())
-    S.Diag(Loc, diag::err_coroutine_unevaluated_context) << Keyword;
-
-  // Second emphasis of [expr.await]p2: must be outside of an exception handler.
-  if (isWithinCatchScope(S.getCurScope()))
-    S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword;
-}
-
 ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) {
   if (!ActOnCoroutineBodyStart(S, Loc, "co_await")) {
     CorrectDelayedTyposInExpr(E);
     return ExprError();
   }
 
-  checkSuspensionContext(*this, Loc, "co_await");
-
   if (E->getType()->isPlaceholderType()) {
     ExprResult R = CheckPlaceholderExpr(E);
     if (R.isInvalid()) return ExprError();
@@ -803,8 +771,6 @@ ExprResult Sema::ActOnCoyieldExpr(Scope
     return ExprError();
   }
 
-  checkSuspensionContext(*this, Loc, "co_yield");
-
   // Build yield_value call.
   ExprResult Awaitable = buildPromiseCall(
       *this, getCurFunction()->CoroutinePromise, Loc, "yield_value", E);

Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=356774&r1=356773&r2=356774&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Fri Mar 22 09:08:29 2019
@@ -314,23 +314,13 @@ struct CtorDtor {
   }
 };
 
-namespace std { class type_info; }
-
 void unevaluated() {
-  decltype(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
-                        // expected-warning at -1 {{declaration does not declare anything}}
-  sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
-                      // expected-error at -1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
-  typeid(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
-                      // expected-warning at -1 {{expression with side effects has no effect in an unevaluated context}}
-                      // expected-warning at -2 {{expression result unused}}
-  decltype(co_yield 1); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
-                        // expected-warning at -1 {{declaration does not declare anything}}
-  sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
-                      // expected-error at -1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
-  typeid(co_yield 3); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
-                      // expected-warning at -1 {{expression with side effects has no effect in an unevaluated context}}
-                      // expected-warning at -2 {{expression result unused}}
+  decltype(co_await a); // expected-error {{cannot be used in an unevaluated context}}
+  sizeof(co_await a); // expected-error {{cannot be used in an unevaluated context}}
+  typeid(co_await a); // expected-error {{cannot be used in an unevaluated context}}
+  decltype(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
+  sizeof(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
+  typeid(co_yield a); // expected-error {{cannot be used in an unevaluated context}}
 }
 
 // [expr.await]p2: "An await-expression shall not appear in a default argument."
@@ -338,47 +328,6 @@ void unevaluated() {
 // not allowed. A user may not understand that this is "outside a function."
 void default_argument(int arg = co_await 0) {} // expected-error {{'co_await' cannot be used outside a function}}
 
-void await_in_catch_coroutine() {
-  try {
-  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
-    []() -> void { co_await a; }(); // OK
-    co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
-  }
-}
-
-void await_nested_in_catch_coroutine() {
-  try {
-  } catch (...) { // FIXME: Emit a note diagnostic pointing out the try handler on this line.
-    try {
-      co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}}
-      []() -> void { co_await a; }(); // OK
-    } catch (...) {
-      co_return 123;
-    }
-  }
-}
-
-void await_in_lambda_in_catch_coroutine() {
-  try {
-  } catch (...) {
-    []() -> void { co_await a; }(); // OK
-  }
-}
-
-void yield_in_catch_coroutine() {
-  try {
-  } catch (...) {
-    co_yield 1; // expected-error {{'co_yield' cannot be used in the handler of a try block}}
-  }
-}
-
-void return_in_catch_coroutine() {
-  try {
-  } catch (...) {
-    co_return 123; // OK
-  }
-}
-
 constexpr auto constexpr_deduced_return_coroutine() {
   co_yield 0; // expected-error {{'co_yield' cannot be used in a constexpr function}}
   // expected-error at -1 {{'co_yield' cannot be used in a function with a deduced return type}}




More information about the cfe-commits mailing list