[PATCH] D61837: Make it possible control matcher traversal kind with ASTContext

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/AST/ASTContext.h:562-563
 public:
+  ast_type_traits::TraversalKind GetTraversalKind() const { return Traversal; }
+  void SetTraversalKind(ast_type_traits::TraversalKind TK) { Traversal = TK; }
+
----------------
steveire wrote:
> aaron.ballman wrote:
> > This doesn't match our coding guidelines. Should be `getTraversalKind()`, etc. Same below.
> I think clang still uses uppercase names everywhere. Can you be more specific?
No, Clang doesn't use uppercase names everywhere -- we're consistently inconsistent and it depends mostly on the age of when the code was introduced and what the surrounding code looks like. We still follow the usual coding style guidelines for naming conventions -- stick with the convention used by nearby code if it's already consistent, otherwise follow the coding style rules.


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:716
+/// \endcode
+/// matches the return statement with "ret" bound to "a".
+template <typename T>
----------------
aaron.ballman wrote:
> Copy pasta?
You dropped the interesting bit from the documentation here -- you should add in what the matcher is matching (which makes the preceding "The matcher \code ... \endcode" grammatical again).


================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:718
+template <typename T>
+internal::Matcher<T> traverse(ast_type_traits::TraversalKind TK,
+                              const internal::Matcher<T> &InnerMatcher) {
----------------
steveire wrote:
> aaron.ballman wrote:
> > Is this an API we should be exposing to clang-query as well? Will those users be able to use a string literal for the traversal kind, like they already do for attribute kinds (etc)?
> Yes, I thought about that, but in a follow-up patch. First, I aim to extend the `TraversalKind` enum with `TK_IgnoreInvisble`.
This is new functionality, so why do you want to wait for a follow-up patch (is it somehow more involved)? We typically add support for dynamic matchers at the same time we add support for the static matchers because otherwise the two get frustratingly out of sync.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61837