[PATCH] D73037: Add a way to set traversal mode in clang-query

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

There should also be a mention of this in the release notes (especially if the default behavior winds up changing).



================
Comment at: clang-tools-extra/clang-query/Query.cpp:51
+        "    IgnoreImplicitCastsAndParentheses"
+        "  Omit implicit casts and parens in matching and dumping.\n"
+        "    IgnoreUnlessSpelledInSource     "
----------------
It took me a few tries to understand why there's a leading whitespace here. I'd prefer this to be a trailing whitespace on the previous line as in other cases unless there's some reason we need all the trailing quotes to line up vertically.


================
Comment at: clang-tools-extra/clang-query/Query.cpp:53
+        "    IgnoreUnlessSpelledInSource     "
+        "Omit AST nodes unless spelled in the source.\n"
         "  set output <feature>              "
----------------
We should document explicitly that this is the default (or document `AsIs` as the default if we go that route for now).


================
Comment at: clang-tools-extra/clang-query/QuerySession.h:29
         DetailedASTOutput(false), BindRoot(true), PrintMatcher(false),
-        Terminate(false) {}
+        Terminate(false), TK(ast_type_traits::TK_IgnoreUnlessSpelledInSource) {}
 
----------------
While we want to get here eventually, I think it might make sense to have the default be `AsIs` until there's clear agreement that we want the default traversal mode in AST matchers to be this way. Basically, clang-query's default mode should be the same as the C++ AST matcher default mode.


================
Comment at: clang-tools-extra/unittests/clang-query/QueryParserTest.cpp:114
+
+  Q = parse("set traversal AsIs");
+  ASSERT_TRUE(isa<SetQuery<ast_type_traits::TraversalKind> >(Q));
----------------
Can you also add a test for `set traversal ThisShouldNotWork` or something along those lines?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73037