rsmith added a comment.
@ABataev Is it intentional that we do not propagate `Allowed` through labels?
For example:
void f() {
#pragma omp barrier // ok
label:
#pragma omp barrier // error, "cannot be an immediate substatement"
label:
;
#pragma omp barrier // ok
}
?
================
Comment at: include/clang/AST/Stmt.h:1598-1602
+ const Expr *getExprStmt() const;
+ Expr *getExprStmt() {
+ const ValueStmt *ConstThis = this;
+ return const_cast<Expr*>(ConstThis->getExprStmt());
+ }
----------------
aaron.ballman wrote:
> We typically implement this in reverse, where the non-const version holds the
> actual implementation and the const version performs a `const_cast`.
We do. Do you think that's preferable? I like that this way around proves that
the `const` version is const-correct, but it's a tiny benefit and I'm fine with
just being consistent.
================
Comment at: include/clang/Parse/Parser.h:374
+ /// a statement expression and builds a suitable expression statement.
+ StmtResult handleExprStmt(ExprResult E, WithinStmtExpr IsInStmtExpr);
----------------
aaron.ballman wrote:
> Rather than passing around `IsInStmtExpr` to a bunch of parser APIs, would it
> make more sense to add an RAII object that sets some state on `Parser` to
> test it? The proliferation of arguments seems unfortunate given how much of a
> corner case statement expressions are.
Yeah, I tried that approach first, but the parser state turns out to be much
worse, because it puts a burden on every form of statement that constructs a
nested parsing context to reset that state. We can put the resetting on
`ParseScope`, but it still needs to have an effect in the case where the scope
is disabled, which is surprising, and there's no guarantee such statement
constructs that can end in an expression will have a nested scope (consider,
for instance, constructs like `return x;` or `goto *label;`). This is really
local state that should only be propagated through a very small number of
syntactic constructs rather than global state.
Maybe we could combine the new flag with the `AllowOpenMPStandalone` /
`AllowedConstructsKind` flag into a more general kind of "statement context".
The propagation rules aren't quite the same (`AllowOpenMPStandalone` doesn't
get propagated through labels whereas `IsInStmtExpr` does), which is a little
awkward, but maybe that's not so bad -- and maybe that's actually a bug in the
OpenMP implementation?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57984/new/
https://reviews.llvm.org/D57984
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
