modocache created this revision. modocache added reviewers: GorNishanov, tks2103, rsmith. Herald added subscribers: jdoerfert, EricWF. Herald added a project: clang.
As reported in https://bugs.llvm.org/show_bug.cgi?id=40978, it's an error to use the `co_yield` or `co_await` keywords outside of a valid "suspension context" as defined by [expr.await]p2 of http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/n4775.pdf. Whether or not the current scope was in a function-try-block's (https://en.cppreference.com/w/cpp/language/function-try-block) handler could be determined using scope flag `Scope::FnTryCatchScope`. No such flag existed for a simple C++ catch statement, so this commit adds one. Repository: rC Clang https://reviews.llvm.org/D59076 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Scope.h lib/Parse/ParseStmt.cpp lib/Sema/SemaCoroutine.cpp test/SemaCXX/coroutines.cpp
Index: test/SemaCXX/coroutines.cpp =================================================================== --- test/SemaCXX/coroutines.cpp +++ test/SemaCXX/coroutines.cpp @@ -328,6 +328,27 @@ // 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 (...) { + co_await a; // expected-error {{'co_await' cannot be used in the handler of a try block}} + } +} + +void yield_in_catch_coroutine() { + try { + } catch (...) { + co_yield a; // 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@-1 {{'co_yield' cannot be used in a function with a deduced return type}} Index: lib/Sema/SemaCoroutine.cpp =================================================================== --- lib/Sema/SemaCoroutine.cpp +++ lib/Sema/SemaCoroutine.cpp @@ -185,21 +185,8 @@ static bool isValidCoroutineContext(Sema &S, SourceLocation Loc, StringRef Keyword) { - // '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. + // [expr.await]p2 dictates that 'co_await' and 'co_yield' must be used within + // a function body. // 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 @@ -668,8 +655,34 @@ return true; } +// [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 bool isValidSuspensionContext(Sema &S, SourceLocation Loc, + StringRef Keyword, + bool IsImplicit = false) { + // 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; + return false; + } + + // Second emphasis of [expr.await]p2: must be outside of an exception handler. + if (S.getCurScope()->getFlags() & Scope::CatchScope) { + S.Diag(Loc, diag::err_coroutine_within_handler) << Keyword; + return false; + } + + return true; +} + ExprResult Sema::ActOnCoawaitExpr(Scope *S, SourceLocation Loc, Expr *E) { - if (!ActOnCoroutineBodyStart(S, Loc, "co_await")) { + if (!ActOnCoroutineBodyStart(S, Loc, "co_await") || + !isValidSuspensionContext(*this, Loc, "co_await")) { CorrectDelayedTyposInExpr(E); return ExprError(); } @@ -689,7 +702,7 @@ ExprResult Sema::BuildUnresolvedCoawaitExpr(SourceLocation Loc, Expr *E, UnresolvedLookupExpr *Lookup) { auto *FSI = checkCoroutineContext(*this, Loc, "co_await"); - if (!FSI) + if (!FSI || !isValidSuspensionContext(*this, Loc, "co_await")) return ExprError(); if (E->getType()->isPlaceholderType()) { @@ -766,7 +779,8 @@ } ExprResult Sema::ActOnCoyieldExpr(Scope *S, SourceLocation Loc, Expr *E) { - if (!ActOnCoroutineBodyStart(S, Loc, "co_yield")) { + if (!ActOnCoroutineBodyStart(S, Loc, "co_yield") || + !isValidSuspensionContext(*this, Loc, "co_yield")) { CorrectDelayedTyposInExpr(E); return ExprError(); } Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -2260,8 +2260,10 @@ // 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. - ParseScope CatchScope(this, Scope::DeclScope | Scope::ControlScope | - (FnCatch ? Scope::FnTryCatchScope : 0)); + unsigned ScopeFlags = Scope::DeclScope | Scope::ControlScope | + Scope::CatchScope | + (FnCatch ? Scope::FnTryCatchScope : 0); + ParseScope CatchScope(this, ScopeFlags); // exception-declaration is equivalent to '...' or a parameter-declaration // without default arguments. @@ -2290,7 +2292,7 @@ return StmtError(Diag(Tok, diag::err_expected) << tok::l_brace); // FIXME: Possible draft standard bug: attribute-specifier should be allowed? - StmtResult Block(ParseCompoundStatement()); + StmtResult Block(ParseCompoundStatement(/*isStmtExpr=*/false, ScopeFlags)); if (Block.isInvalid()) return Block; Index: include/clang/Sema/Scope.h =================================================================== --- include/clang/Sema/Scope.h +++ include/clang/Sema/Scope.h @@ -131,6 +131,9 @@ /// 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: Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -9244,6 +9244,8 @@ "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<
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits