[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

This revision was automatically updated to reflect the committed changes. Closed by commit rC354090: PR40642: Fix determination of whether the final statement of a statement (authored by rsmith, committed by ). Changed prior to commit: https://reviews.llvm.org/D57984?vs=186731&id=186945#toc Re

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Parse/ParseStmt.cpp:1090-1091 +R = handleExprStmt(Res, SubStmtCtx); +if (R.isUsable()) + R = Actions.ProcessStmtA

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith updated this revision to Diff 186731. rsmith marked 7 inline comments as done. rsmith added a comment. Address review comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57984/new/ https://reviews.llvm.org/D57984 Files: include/clang/AST/Expr.h in

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith added inline comments. Comment at: include/clang/Parse/Parser.h:374 +/// This context is at the top level of a GNU statement expression. +InStmtExpr = 0x4, + aaron.ballman wrote: > It's a bit strange that the previous two enumerators are "Allow" an

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

ABataev added inline comments. Comment at: include/clang/Parse/Parser.h:382 + + friend ParsedStmtContext operator|(ParsedStmtContext A, ParsedStmtContext B) { +return ParsedStmtContext((unsigned)A | (unsigned)B); We have `llvm/ADT/BitmaskEnum.h`, which defi

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

aaron.ballman added inline comments. Comment at: include/clang/Parse/Parser.h:374 +/// This context is at the top level of a GNU statement expression. +InStmtExpr = 0x4, + It's a bit strange that the previous two enumerators are "Allow" and this is "In".

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith updated this revision to Diff 186578. rsmith added a comment. Combine WithinStmtExpr flag with AllowedStmtKinds into a more general statement context. In doing so, fix some bugs where the OpenMP context was not being propagated properly through labels and expression-statements starting with

Re: [PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

On Tue, 12 Feb 2019, 05:28 Aaron Ballman via Phabricator via cfe-commits, < cfe-commits@lists.llvm.org> wrote: > aaron.ballman added a comment. > > Considering that this has been fertile ground for buggy edge cases, should > we revert my original changes from the 8.0 branch to give this more time

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

aaron.ballman added a comment. Considering that this has been fertile ground for buggy edge cases, should we revert my original changes from the 8.0 branch to give this more time to bake on trunk before releasing? Or do you think this is fine to move onto the release branch once finalized? =

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

ABataev added a comment. Herald added a subscriber: jdoerfert. In D57984#1394167 , @rsmith wrote: > In D57984#1394067 , @ABataev wrote: > > > In D57984#1394050 , @rsmith wrot

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith added a comment. In D57984#1394067 , @ABataev wrote: > In D57984#1394050 , @rsmith wrote: > > > @ABataev Is it intentional that we do not propagate `Allowed` through > > labels? For example: > > > > void f

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

ABataev added a comment. In D57984#1394050 , @rsmith wrote: > @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,

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

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 } ?

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith updated this revision to Diff 186337. rsmith marked an inline comment as done. rsmith added a comment. Address a couple of review comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57984/new/ https://reviews.llvm.org/D57984 Files: include/clang/AST

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

aaron.ballman added a comment. Thank you for working on this! It looks like we did have to add the new AST node after all, but the changes seem pretty good. Should anything be updated in the AST dumper for this (such as printing whether a Stmt is a value-producing statement or not)?

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

rsmith created this revision. rsmith added reviewers: aaron.ballman, rjmccall. Herald added a project: clang. We used to get this wrong in three ways: 1. During parsing, an expression-statement followed by the }) ending a statement expression was always treated as producing the value of the stat