aaron.ballman 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();
----------------
riccibruno wrote:
> 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.
If it doesn't seem to have a measurable performance impact, I think it's
reasonable.
================
Comment at: lib/AST/Expr.cpp:2563
+
+ else if (auto *FE = dyn_cast_or_null<FullExpr>(E))
+ return FE->getSubExpr();
----------------
No `else` after a `return`
================
Comment at: lib/AST/Expr.cpp:2580
+
+ else if (auto *NTTP = dyn_cast_or_null<SubstNonTypeTemplateParmExpr>(E))
+ return NTTP->getReplacement();
----------------
No `else` after a `return` (same elsewhere in the patch).
================
Comment at: lib/AST/Expr.cpp:2684
+
+namespace {
+
----------------
Rather than an unnamed namespace, you should prefer static function
declarations per the style guide.
================
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;
----------------
Formatting looks off here, did clang-format produce this?
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
