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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 23 12:45:49 PST 2019


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with a few minor nits.



================
Comment at: clang/include/clang/AST/ASTContext.h:3009
   class ParentMap;
-  std::unique_ptr<ParentMap> Parents;
+  std::map<ast_type_traits::TraversalKind, std::unique_ptr<ParentMap>> Parents;
 
----------------
Do we need to use `std::map` here, or could we use an `IndexedMap` or `DenseMap`?

(`std::map` tends to perform poorly under some circumstances and the keys in this case look like they should be small and dense, but if we can't use one of the more specialized containers for some reason, it's fine to stick with `std::map`.)


================
Comment at: clang/lib/AST/ASTContext.cpp:120
+ASTContext::traverseIgnored(const ast_type_traits::DynTypedNode &N) const {
+  if (auto *E = N.get<Expr>()) {
+    return ast_type_traits::DynTypedNode::create(*traverseIgnored(E));
----------------
Because `N` is const, does its `get<>` method return a `const` pointer? If so, switch to `const auto *` for clarity.


================
Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:222
                               BoundNodesTreeBuilder *Builder) const {
-  if (RestrictKind.isBaseOf(DynNode.getNodeKind()) &&
-      Implementation->dynMatches(DynNode, Finder, Builder)) {
+  TraversalKindScope raii(Finder->getASTContext(),
+                          Implementation->TraversalKind());
----------------
`raii` -> `RAII` per naming conventions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61837





More information about the cfe-commits mailing list