riccibruno added inline comments.
================
Comment at: lib/AST/Expr.cpp:2562
+ return ICE->getSubExpr();
+
+ else if (auto *FE = dyn_cast_or_null<FullExpr>(E))
----------------
It is something that is actually possible to audit. I did look at where each of
the skipped node are created and it *seems* that a null child is not possible.
However it is very easy to miss a case and adding the null check give the
various `Expr::Ignore*` the following nice property:
Regardless of the state of the AST, for every expression `E` (null or not).
`Ignore*(E)` produces the correct result (which may be null).
But perhaps the correct invariant to maintain is that these nodes have always a
non-null child ?
I re-ran the benchmarks more carefully and here are the results I got (on my
usual benchmark: `-fyntax-only` on all of Boost, 20 iterations with `perf
stat`):
8.2117 +- 0.0131 seconds (with the null check)
8.2028 +- 0.0058 seconds (without the null check)
================
Comment at: lib/AST/Expr.cpp:2695
+/// Note that a null E is valid; in this case nothing is done.
+template <typename... FnTys> Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
Expr *LastE = nullptr;
----------------
aaron.ballman wrote:
> Formatting looks off here, did clang-format produce this?
It did, though with the `static` specifier `clang-format` adds a new line.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57267/new/
https://reviews.llvm.org/D57267
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
