[PATCH] D57266: [AST] Update the comments of the various Expr::Ignore* + Related cleanups

[Aaron Ballman via Phabricator via cfe-commits]

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