aaron.ballman added inline comments.

================
Comment at: clang/unittests/Tooling/StencilTest.cpp:63
   ASTContext &Context = AstUnit->getASTContext();
-  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  auto Matches = ast_matchers::match(
+      traverse(ast_type_traits::TK_AsIs, wrapMatcher(Matcher)), Context);
----------------
steveire wrote:
> aaron.ballman wrote:
> > Was this change made because you didn't want to accept the traversal mode 
> > as a parameter to `matchStmt` and force each caller to decide which mode 
> > they use? Or is there some other reason why this change was needed?
> Yes, the test currently expects `AsIs`. If someone wanted to change that in 
> the future, that is an issue for future work.
You say "the test" as though there's only one. There are numerous tests which 
use `matchStmt`. Do *all* of the tests require `AsIs`? (I think that's an 
example of what's causing some of the confusion @shafik and I had and I'm 
trying to ensure his question gets answered and that I'm fully understanding 
your changes.) From spot-checking, it looks like there are tests using 
`matchStmt` that don't need `AsIs` traversal and that this sort of cleanup work 
could happen in the future, but for now you're going with the easiest approach 
instead of expanding the test coverage for ignoring implicit nodes; is that 
correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72531



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to