[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in r365498. Thank you for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 ___ cfe-commits mail

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 208550. domdom added a comment. Thank you for catching that @aaron.ballman! I forgot to update the test after the running clang-format. I thought I ran the tests again before submitting, but I guess I didn't :P Thanks again! CHANGES SINCE LAST ACTION htt

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added a comment. In D57086#1551354 , @domdom wrote: > In D57086#1550514 , @aaron.ballman > wrote: > > > In D57086#1549632 , @domdom wrote: > > > > > clang-form

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom added a comment. In D57086#1550514 , @aaron.ballman wrote: > In D57086#1549632 , @domdom wrote: > > > clang-format the patch > > > Thanks! Do you need someone to commit on your behalf? You are very welcome

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added a comment. In D57086#1549632 , @domdom wrote: > clang-format the patch Thanks! Do you need someone to commit on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 __

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 205501. domdom added a comment. clang-format the patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 Files: clang/include/clang/AST/Stmt.h clang/lib/CodeGen/CGStmt.cpp clang/lib/Parse/ParseStmt.cpp

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

rsmith added a comment. In D57086#1546386 , @aaron.ballman wrote: > In D57086#1535873 , @domdom wrote: > > > Something I should ask, it seems like GCC only ignores the NullStmts at the > > end if it's in C mode. S

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D57086#1535873 , @domdom wrote: > Sorry I it's taken me a while to get back to this work. I've rebased the > changes and taken advanta

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 203775. domdom added a comment. Sorry I it's taken me a while to get back to this work. I've rebased the changes and taken advantage of the refactoring to stop modifying the CompoundStmt after creating it. This definitely simplified the changes required in S

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

rsmith added a comment. You'll also need to update `TreeTransform::TransformCompoundStmt`. (And please add some tests for template instantiation.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 ___

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

riccibruno added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1260 + // this compound expression contains nothing but NullStmts, then we return + // the index of the last one. If the compound statement is empty, return + // None. Additionally I

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1259-1260 + // This gets the index of the last Stmt before the trailing NullStmts. If + // this compound expression contains nothing but NullStmts, then we return + // the index of the last one. If

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 185196. domdom added a comment. Switched getIndexOfLastNonNullStmt to return Optional instead of Stmt* as per comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 Files: clang/include/clang/AST/Stmt.

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1300 + void setStmtExpr(Stmt *S) { +assert(!body_empty() && "setStmtExpr"); +unsigned ExprResult = getLastNonNullStmt(); domdom wrote: > getLastNonNullStmt asserts anyway, sh

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom marked an inline comment as not done. domdom added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1259 + // the index of the last one. + unsigned getLastNonNullStmt() const { +assert(!body_empty() && "getLastNonNullStmt"); aaron.ballman

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 184414. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 Files: clang/include/clang/AST/Stmt.h clang/lib/CodeGen/CGStmt.cpp clang/lib/Parse/ParseStmt.cpp clang/lib/Sema/SemaExpr.cpp clang/test/AST/ast-

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 184413. domdom marked 6 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 Files: clang/include/clang/AST/Stmt.h clang/lib/CodeGen/CGStmt.cpp Index: clang/lib/CodeGen/CGStmt.cpp

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Stmt.h:1259 + // the index of the last one. + unsigned getLastNonNullStmt() const { +assert(!body_empty() && "getLastNonNullStmt"); How about `getIndexOfLastNonNullStmt()` since it doe

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 184001. domdom added a comment. Thanks for your comments, @aaron.ballman. I've addressed the comments and added a test case as suggested. This revealed an issue with the code-gen side of things, so I fixed that and added another test case for that as well.

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

aaron.ballman edited reviewers, added: aaron.ballman; removed: lattner. aaron.ballman added a comment. Can you also add a test case to `TestMiscStmts()` in clang\test\AST\ast-dump-stmt.c that demonstrates we calculate the correct resulting type and don't lose the extra null statements from the A

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom updated this revision to Diff 183043. domdom added a comment. Formatting CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57086/new/ https://reviews.llvm.org/D57086 Files: clang/lib/Parse/ParseStmt.cpp clang/lib/Sema/SemaExpr.cpp clang/test/Sema/statements.c Index: clang/te

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

domdom created this revision. domdom added reviewers: lattner, rsmith. Herald added a subscriber: cfe-commits. Ignore trailing NullStmts in compound expressions when determining the result type and value. This is to match the GCC behavior which ignores semicolons at the end of compound expressio