aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with some nits. Thank you for the cleaned up comments, these are helpful!
================
Comment at: include/clang/AST/Expr.h:749-750
+ /// reaching a fixed point. Skips:
+ /// * ImplicitCastExpr.
+ /// * FullExpr.
Expr *IgnoreImpCasts() LLVM_READONLY;
----------------
I'd remove the full stops from the list elements (same elsewhere).
================
Comment at: include/clang/AST/Expr.h:848
+ /// CK_UncheckedDerivedToBase and CK_NoOp).
Expr *ignoreParenBaseCasts() LLVM_READONLY;
+ const Expr *ignoreParenBaseCasts() const {
----------------
Want to fix up this name to match the coding conventions (in a separate patch,
no review needed)?
================
Comment at: lib/AST/Expr.cpp:2561
+ Expr *E = this;
+ while (true)
+ if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
----------------
I would appreciate some braces around the loop body -- I know they're not
strictly required, but this is not a particularly simple body either.
================
Comment at: lib/AST/Expr.cpp:2562
+ while (true)
+ if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
----------------
`auto *` and same below.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57266/new/
https://reviews.llvm.org/D57266
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
