[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
modocache added a comment.
Thank you for the review!
Comment at: test/SemaCXX/exceptions.cpp:25
+ int ref = k;
+}
int j = i; // expected-error {{use of undeclared identifier 'i'}}
riccibruno wrote:
> I am wondering if there is already a test which checks systematically that a
> declaration which shadows another declaration is valid/invalid. I am not
> seeing such a systematic test, but it might be a nice addition (though that
> it clearly out of scope for this patch)
That's a good point. I was very surprised when I learned that my original patch
had broken the lookup behavior described in
https://bugs.llvm.org/show_bug.cgi?id=41171 but still passed all the tests in
`check-clang`. I'll try to follow up this patch with some more comprehensive
tests.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59752/new/
https://reviews.llvm.org/D59752
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356865: Un-revert "[coroutines][PR40978] Emit error for
co_yield within catch block" (authored by modocache, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D59752?vs=192045&id=192050#toc
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59752/new/
https://reviews.llvm.org/D59752
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Scope.h
lib/Parse/ParseStmt.cpp
lib/Sema/Scope.cpp
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
test/SemaCXX/exceptions.cpp
Index: test/SemaCXX/exceptions.cpp
===
--- test/SemaCXX/exceptions.cpp
+++ test/SemaCXX/exceptions.cpp
@@ -7,6 +7,7 @@
struct Abstract { virtual void f() = 0; }; // expected-note {{unimplemented pure virtual method 'f'}}
void trys() {
+ int k = 42;
try {
} catch(int i) { // expected-note {{previous definition}}
int j = i;
@@ -18,6 +19,10 @@
} catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'A'}}
} catch(Abstract) { // expected-error {{variable type 'Abstract' is an abstract class}}
} catch(...) {
+int ref = k;
+{
+ int ref = k;
+}
int j = i; // expected-error {{use of undeclared identifier 'i'}}
}
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -314,13 +314,23 @@
}
};
+namespace std { class type_info; }
+
void unevaluated() {
- 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}}
+ decltype(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+ sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+ // expected-error@-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@-1 {{expression with side effects has no effect in an unevaluated context}}
+ // expected-warning@-2 {{expression result unused}}
+ decltype(co_yield 1); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+ sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+ // expected-error@-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@-1 {{expression with side effects has no effect in an unevaluated context}}
+ // expected-warning@-2 {{expression result unused}}
}
// [expr.await]p2: "An await-expression shall not appear in a default argument."
@@ -328,6 +338,47 @@
// 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
[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
riccibruno accepted this revision.
riccibruno added a comment.
This revision is now accepted and ready to land.
Great! Since this already received one round of reviews I guess this looks okay.
Comment at: test/SemaCXX/exceptions.cpp:25
+ int ref = k;
+}
int j = i; // expected-error {{use of undeclared identifier 'i'}}
I am wondering if there is already a test which checks systematically that a
declaration which shadows another declaration is valid/invalid. I am not seeing
such a systematic test, but it might be a nice addition (though that it clearly
out of scope for this patch)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59752/new/
https://reviews.llvm.org/D59752
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
modocache added inline comments.
Comment at: lib/Parse/ParseStmt.cpp:2293
// FIXME: Possible draft standard bug: attribute-specifier should be allowed?
StmtResult Block(ParseCompoundStatement());
if (Block.isInvalid())
riccibruno wrote:
> Just to make sure I understood the problem correctly, the issue was that you
> passed `ScopeFlags` to `ParseCompoundStatement()`. This override the default
> flags which are `Scope::DeclScope | Scope::CompoundStmtScope`. In particular
> now `ParseCompoundStatement()` was done as if in the controlling scope of an
> if statement. Now as per [basic.scope.block]/p4:
>
> > Names declared in the init-statement, the for-range-declaration, and in the
> > condition of if, while, for, and switch statements are local to the if,
> > while, for, or switch statement (including the controlled statement), and
> > shall not be redeclared in a subsequent condition of that statement nor in
> > the outermost block (or, for the if statement, any of the outermost blocks)
> > of the controlled statement; see 9.4.
>
> This caused the declaration in the compound statement to be detected as
> erroneous. Indeed the following worked just fine.
>
> ```
> void f() {
> try {}
> catch (...) {
> int i;
> {
> {
> int i;
> }
> }
> }
> }
> ```
>
> Does this make sense or I am completely off base here ?
>
That's exactly my understanding, yes! In fact thank you for the clear
explanation.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59752/new/
https://reviews.llvm.org/D59752
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
riccibruno added inline comments.
Comment at: lib/Parse/ParseStmt.cpp:2293
// FIXME: Possible draft standard bug: attribute-specifier should be allowed?
StmtResult Block(ParseCompoundStatement());
if (Block.isInvalid())
Just to make sure I understood the problem correctly, the issue was that you
passed `ScopeFlags` to `ParseCompoundStatement()`. This override the default
flags which are `Scope::DeclScope | Scope::CompoundStmtScope`. In particular
now `ParseCompoundStatement()` was done as if in the controlling scope of an if
statement. Now as per [basic.scope.block]/p4:
> Names declared in the init-statement, the for-range-declaration, and in the
> condition of if, while, for, and switch statements are local to the if,
> while, for, or switch statement (including the controlled statement), and
> shall not be redeclared in a subsequent condition of that statement nor in
> the outermost block (or, for the if statement, any of the outermost blocks)
> of the controlled statement; see 9.4.
This caused the declaration in the compound statement to be detected as
erroneous. Indeed the following worked just fine.
```
void f() {
try {}
catch (...) {
int i;
{
{
int i;
}
}
}
}
```
Does this make sense or I am completely off base here ?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59752/new/
https://reviews.llvm.org/D59752
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D59752: Un-revert "[coroutines][PR40978] Emit error for co_yield within catch block"
modocache created this revision.
modocache added reviewers: GorNishanov, tks2103, rsmith.
Herald added subscribers: jdoerfert, EricWF.
Herald added a project: clang.
https://reviews.llvm.org/D59076 added a new coroutine error that
prevented users from using 'co_await' or 'co_yield' within a exception
handler. However, it was reverted in https://reviews.llvm.org/rC356774
because it caused a regression in nested scopes in C++ catch statements,
as documented by https://bugs.llvm.org/show_bug.cgi?id=41171.
The issue was due to an incorrect use of a `clang::ParseScope`. To fix:
1. Add a regression test for catch statement parsing that mimics the bug report
from https://bugs.llvm.org/show_bug.cgi?id=41171.
2. Re-apply the coroutines error patch from https://reviews.llvm.org/D59076,
but this time with the correct ParseScope behavior.
Repository:
rC Clang
https://reviews.llvm.org/D59752
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Scope.h
lib/Parse/ParseStmt.cpp
lib/Sema/Scope.cpp
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutines.cpp
test/SemaCXX/exceptions.cpp
Index: test/SemaCXX/exceptions.cpp
===
--- test/SemaCXX/exceptions.cpp
+++ test/SemaCXX/exceptions.cpp
@@ -7,6 +7,7 @@
struct Abstract { virtual void f() = 0; }; // expected-note {{unimplemented pure virtual method 'f'}}
void trys() {
+ int k = 42;
try {
} catch(int i) { // expected-note {{previous definition}}
int j = i;
@@ -18,6 +19,10 @@
} catch(A &a) { // expected-error {{cannot catch reference to incomplete type 'A'}}
} catch(Abstract) { // expected-error {{variable type 'Abstract' is an abstract class}}
} catch(...) {
+int ref = k;
+{
+ int ref = k;
+}
int j = i; // expected-error {{use of undeclared identifier 'i'}}
}
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -314,13 +314,23 @@
}
};
+namespace std { class type_info; }
+
void unevaluated() {
- 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}}
+ decltype(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+ sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
+ // expected-error@-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@-1 {{expression with side effects has no effect in an unevaluated context}}
+ // expected-warning@-2 {{expression result unused}}
+ decltype(co_yield 1); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+// expected-warning@-1 {{declaration does not declare anything}}
+ sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
+ // expected-error@-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@-1 {{expression with side effects has no effect in an unevaluated context}}
+ // expected-warning@-2 {{expression result unused}}
}
// [expr.await]p2: "An await-expression shall not appear in a default argument."
@@ -328,6 +338,47 @@
// 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_cat
