Re: [PATCH] D17446: ASTMatchers: add new statement/decl matchers

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: docs/LibASTMatchersReference.html:512
@@ +511,3 @@
+<tr><td>Matcher&lt;<a 
href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html";>Stmt</a>&gt;</td><td
 class="name" onclick="toggle('addrLabelExpr0')"><a 
name="addrLabelExpr0Anchor">addrLabelExpr</a></td><td>Matcher&lt;<a 
href="http://clang.llvm.org/doxygen/classclang_1_1AddrLabelExpr.html";>AddrLabelExpr</a>&gt;...</td></tr>
+<tr><td colspan="4" class="doc" id="addrLabelExpr0"><pre>Matches address of 
label statements (GNU extension).
+
----------------
We name matchers after the AST node they represent. If we change the name of 
the AST node to include GNU, then I think it would make sense. I'm not certain 
whether we want to rename the AST nodes though (it would be a larger discussion 
than this review), so I would say we shouldn't put gnu in the matcher names, 
but call it out in the documentation (as the author is now doing).

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1012
@@ +1011,3 @@
+/// \brief Matches paren list expressions.
+/// ParenListExprs don't have a predefined type and are used for latter 
parsing.
+/// In final AST, they can be met in template declarations.
----------------
late parsing instead of latter parsing?

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1013
@@ +1012,3 @@
+/// ParenListExprs don't have a predefined type and are used for latter 
parsing.
+/// In final AST, they can be met in template declarations.
+///
----------------
In the final AST (insert the).

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1022
@@ +1021,3 @@
+///
+/// Given
+/// \code
----------------
Combine the two examples into one. e.g.,
```
template<typename T> class X {
  void f() {
    X x(*this);
    int a = 0, b = 1; int i = (a, b);
   }
};
```

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1614
@@ +1613,3 @@
+
+/// \brief Matches GNU statement expression.
+///
----------------
Matches a statement expression (GNU extension).

(This way we can consistently search for GNU extension.)


http://reviews.llvm.org/D17446



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits