[PATCH] D53717: [AST] Only store the needed data in ForStmt.
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 26 13:35:28 PDT 2018
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
More information about the cfe-commits
mailing list