[PATCH] D111242: Add `TypeLoc`-related matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Nice! Just some minor notes.



================
Comment at: clang/docs/LibASTMatchersReference.html:636
 
-<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>></td><td class="name" onclick="toggle('blockDecl0')"><a name="blockDecl0Anchor">blockDecl</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1BlockDecl.html">BlockDecl</a>>...</td></tr>
+<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Decl.html">Decl</a>></td><td class="name" onclick="toggle('blockDecl0')"><a name="blockDecl0Anchor">blockDecl</a></td><td>Matcher<BlockDecl>...</td></tr>
 <tr><td colspan="4" class="doc" id="blockDecl0"><pre>Matches block declarations.
----------------
Here and below. These changes look wrong. Did the script encounter an error?


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6358
+/// \code
+///   int* x = 0;
+/// \endcode
----------------
I think you need a const here (to the right) to be considered a qualified type loc.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6400
+/// pointerTypeLoc(hasPointeeLoc(loc(asString("int"))))
+///   matches `int*`.
+AST_MATCHER_P(PointerTypeLoc, hasPointeeLoc, TypeLocMatcher, PointeeMatcher) {
----------------
I think `int` not `int*`?


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6470
+    const DeclRefExpr &Node, unsigned int Index,
+    std::function<bool(const TemplateArgumentLoc &)> Matcher) {
+  llvm::ArrayRef<TemplateArgumentLoc> ArgLocs = Node.template_arguments();
----------------
why the use of std::function? Seems easier just to take `Finder` and `Builder` directly as arguments?


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6526-6528
+/// varDecl(hasTypeLoc(elaboratedTypeLoc(hasNamedTypeLoc(
+///   templateSpecializationTypeLoc(hasAnyTemplateArgumentLoc(
+///   templateArgumentLoc()))))))
----------------
Maybe simplify this -- I think a less detailed matcher would still be a useful illustration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111242