riccibruno added inline comments.
================
Comment at: lib/AST/Expr.cpp:2562
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+ if (auto *ICE = dyn_cast_or_null<ImplicitCastExpr>(E))
+ return ICE->getSubExpr();
----------------
aaron.ballman wrote:
> Do we really need to accept null arguments? We didn't previously, and this
> increases the overhead of something we do pretty often (though it's probably
> only a negligible performance hit).
I did benchmark it to see if there is any difference but could not measure any
(looking at the generated code this adds a single null check at the start). My
concern was that it could be possible that some AST node has a null child,
perhaps when the AST is malformed. I will however defer to you or Richard since
I probably lack some of the background needed to make this choice.
================
Comment at: lib/AST/Expr.cpp:2705
+template <typename FnTy1, typename FnTy2>
+Expr *IgnoreExprNodes(Expr *E, FnTy1 Fn1, FnTy2 Fn2) {
+ Expr *LastE = nullptr;
----------------
aaron.ballman wrote:
> I wonder if it would make more sense to use a parameter pack here so that
> this can be expanded to an arbitrary combination of single steps? The inner
> part of the while loop would look something like:
> ```
> template <typename ...Func>
> Expr *IgnoreExprNodes(Expr *E, Func&& ...Fns) {
> ...
> while (E != LastE) {
> LastE = E;
> for (auto &&Fn : {Fns...})
> E = std::forward<decltype(Fn)>(Fn)(E);
> }
> ...
> }
> ```
I like it! Thanks for the suggestion. This particular solution did not work
because it requires I believe all the types to be equal for the `auto`
deduction to succeed. However a recursive solution works fine. I looked quickly
at the generated code and it is identical as far as I can tell. Let me know if
there are issues with it.
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
