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

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno marked 8 inline comments as done.
riccibruno added inline comments.


================
Comment at: include/clang/AST/Expr.h:749-750
+  /// reaching a fixed point. Skips:
+  /// * ImplicitCastExpr.
+  /// * FullExpr.
   Expr *IgnoreImpCasts() LLVM_READONLY;
----------------
aaron.ballman wrote:
> I'd remove the full stops from the list elements (same elsewhere).
Will do.


================
Comment at: include/clang/AST/Expr.h:848
+  ///   CK_UncheckedDerivedToBase and CK_NoOp).
   Expr *ignoreParenBaseCasts() LLVM_READONLY;
+  const Expr *ignoreParenBaseCasts() const {
----------------
aaron.ballman wrote:
> Want to fix up this name to match the coding conventions (in a separate patch, no review needed)?
Yeah I only see a few uses of `ignoreParenBaseCasts` so this should be easy to do.


================
Comment at: lib/AST/Expr.cpp:2561
+  Expr *E = this;
+  while (true)
+    if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
----------------
aaron.ballman wrote:
> 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.
Will do before landing. I am getting rid of all of these loops in the next patch anyway but I agree that braces are better here.


================
Comment at: lib/AST/Expr.cpp:2562
+  while (true)
+    if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+      E = ICE->getSubExpr();
----------------
aaron.ballman wrote:
> `auto *` and same below.
Yep.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57266/new/

https://reviews.llvm.org/D57266