rsmith added a comment. Do you have evidence that this complexity is worthwhile? (I wouldn't imagine we have very many `ForStmt`s per translation unit, so saving 16 bytes for each of them seems unlikely to matter.)
================ Comment at: include/clang/AST/Stmt.h:1863 + Expr *getCond() { + return reinterpret_cast<Expr *>(getTrailingObjects<Stmt *>()[condOffset()]); + } ---------------- Please `static_cast` rather than `reinterpret_cast` throughout. (If the `Stmt` base class weren't at offset 0 within `Expr`, we'd want to apply the adjustment.) ================ Comment at: include/clang/AST/Stmt.h:1871 + void setCond(Expr *Cond) { + getTrailingObjects<Stmt *>()[condOffset()] = reinterpret_cast<Stmt *>(Cond); + } ---------------- No need for an explicit cast from `Expr` to base class `Stmt`. ================ Comment at: test/Import/for-stmt/test.cpp:3-8 // CHECK: ForStmt // CHECK-NEXT: <<NULL>> // CHECK-NEXT: <<NULL>> -// CHECK-NEXT: <<NULL>> -// CHECK-NEXT: <<NULL>> // CHECK-NEXT: NullStmt ---------------- This seems like a (minor) problem: `-ast-dump` no longer shows what the things it's dumping mean. Maybe add the has_init / has_var flags to the dump of the `ForStmt`, or add labels to the substatements, or something? Repository: rC Clang https://reviews.llvm.org/D53717 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits