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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
