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

[Bruno Ricci via Phabricator via cfe-commits]

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 am not sure that the comment is optimal. This is inside `CompoundStmt` and it is therefore strange to use "if this compound expression", since the compound expression is represented with the `StmtExpr` node.


================
Comment at: clang/include/clang/AST/Stmt.h:1303
+  // Replace the Stmt that would be the result of this compound expression with
+  // another Stmt.
+  void setStmtExpr(Stmt *S) {
----------------
I think it needs at the very least to mention that this is about the GNU extension. Perhaps it would be useful to mention the relation between the `CompoundStmt` node and the `StmtExpr` node ? Also more generally is it not possible to avoid mutating the compound statement node after it has been created ?


================
Comment at: clang/include/clang/AST/Stmt.h:1312
+
+  // Get the Stmt representing the result of this compound expression.
+  Stmt *getStmtExprResult() const {
----------------
Same, I would find it clearer if you mentioned the extension.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57086/new/

https://reviews.llvm.org/D57086