[PATCH] D70613: Add method to ignore invisible AST nodes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:769-770
+  /// implicit conversions.
+  Expr *IgnoreInvisible();
+  const Expr *IgnoreInvisible() const {
+    return const_cast<Expr *>(this)->IgnoreInvisible();
----------------
`Invisible` is a bit of an odd term because the AST nodes themselves are in fact visible within the AST. I think this means "invisible" as in "not corresponding directly to syntax the user wrote". Bikeshedding some other names:

* IgnoreAllImplicitExprs
* IgnoreAllImplicitNodes
* IgnoreNodesNotWrittenByUser
* IgnoreNotWrittenInSource

other suggestions welcome.



================
Comment at: clang/lib/AST/Expr.cpp:3004-3006
+  Expr *s = this;
+
+  Expr *lasts = nullptr;
----------------
Can remove the spurious newline, and these should be named according to the usual naming conventions (I'd go with `E` and `LastE`, but feel free to pick less terrible names).


================
Comment at: clang/lib/AST/Expr.cpp:3012
+
+    const auto SR = s->getSourceRange();
+
----------------
Drop top-level `const` (I wouldn't be sad if it also lost the `auto` at the same time).


================
Comment at: clang/lib/AST/Expr.cpp:3014
+
+    if (auto C = dyn_cast<CXXConstructExpr>(s)) {
+      if (C->getNumArgs() == 1) {
----------------
`const auto *`


================
Comment at: clang/lib/AST/Expr.cpp:3016
+      if (C->getNumArgs() == 1) {
+        auto A = C->getArg(0);
+        if (A->getSourceRange() == SR || !dyn_cast<CXXTemporaryObjectExpr>(C))
----------------
`const Expr *`


================
Comment at: clang/lib/AST/Expr.cpp:3017
+        auto A = C->getArg(0);
+        if (A->getSourceRange() == SR || !dyn_cast<CXXTemporaryObjectExpr>(C))
+          s = A;
----------------
`!isa<CXXTemporaryObjectExpr>(C)` since you're not using the returned value anyway.


================
Comment at: clang/lib/AST/Expr.cpp:3022
+
+    if (auto C = dyn_cast<CXXMemberCallExpr>(s)) {
+      auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
----------------
`const auto *`


================
Comment at: clang/lib/AST/Expr.cpp:3023
+    if (auto C = dyn_cast<CXXMemberCallExpr>(s)) {
+      auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
+      if (ExprNode->getSourceRange() == SR) {
----------------
`const Expr *`

Are you sure you mean to skip parens here, as those are written in source?


================
Comment at: clang/lib/AST/Expr.cpp:3024
+      auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
+      if (ExprNode->getSourceRange() == SR) {
+        s = ExprNode;
----------------
Elide braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70613