[PATCH] D53717: [AST] Only store the needed data in ForStmt.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: include/clang/AST/Stmt.h:1863
+  Expr *getCond() {
+    return reinterpret_cast<Expr *>(getTrailingObjects<Stmt *>()[condOffset()]);
+  }
----------------
riccibruno wrote:
> rsmith wrote:
> > 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.)
> I would gladly get rid of these ugly `reinterpret_cast`,
> but unfortunately the definition of `Expr` is not available in `Stmt.h`.
> 
> As you say these `reinterpret_cast` only work now because of the layout.
> 
> A solution would be to move `Expr` and `Stmt` to something like `StmtBase.h`
> and include this in `Stmt.h`. This would allow us to get of these casts between
> `Stmt` and `Expr`. Of the 91 `reinterpret_cast` in all of the `Stmt*.h` it seems that
> at least 2/3 of them could be removed.
Ugh, I see, thanks. Yes, pulling `Expr` and `Stmt` out into a `StmtBase.h` or similar seems like a good approach to me.


Repository:
  rC Clang

https://reviews.llvm.org/D53717