[PATCH] D90984: Update matchers to be traverse-aware

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 12 13:12:50 PST 2020


aaron.ballman added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115
+
+  if (!Finder->isTraversalAsIs() && (*MatchIt)->isImplicit())
+    return false;
----------------
If the traversal is not `AsIs`, that doesn't mean it's `IgnoreUnlessSpelledInSource` -- it could be `IgnoreImplicitCastsAndParentheses`, right? So I think the logic for some of these AST matchers is incorrect when in `IgnoreImplicitCastsAndParentheses` traversal mode and should be double-checked. We should probably add some extra test coverage for that mode.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4084
+    return false;
+  return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder);
 }
----------------
Huh, I never noticed that we implicitly are ignoring parens and implicit casts for this (without checking the traversal mode or documenting the behavior!). That seems less-than-ideal in some ways. (No change required, I only noticed it because it made me think through whether we need it on the `isa<>` check above, which we don't.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90984



More information about the cfe-commits mailing list