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

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1000
@@ +999,3 @@
+
+
+/// \brief Matches implicit initializers of init list expressions.
----------------
Can remove the extra newline.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1005
@@ +1004,3 @@
+/// \code
+///   point ptarray[10] = { [2].y = 1.0, [2].x = 2.0, [0].x = 1.0 }; }
+/// \endcode
----------------
I don't think this code is valid because of the }; and the missing semicolon at the end.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1019
@@ +1018,3 @@
+/// parenListExpr()
+///   matches "*this"
+const internal::VariadicDynCastAllOfMatcher<Stmt, ParenListExpr> parenListExpr;
----------------
It may be good to have an example of how this differs from ParenExpr with a list. e.g., I don't think this will match: `int a = 0, b = 1; int i = (a, b);`, will it? Either way, it would be good to have as an example (and possibly test case).

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1646
@@ +1645,3 @@
+///
+/// Example matches a ?: b
+/// \code
----------------
Comment doesn't match code, should either match a ?: b or change the code to use b instead of c.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1648
@@ +1647,3 @@
+/// \code
+///   (a ?: c) + 42
+/// \endcode
----------------
Missing semicolon?

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1654
@@ +1653,3 @@
+
+/// \brief Matches opaque value expressions.
+///
----------------
This is still a bit too brief, because opaque value expressions aren't very commonly used, it's hard to understand what it will actually match. If you can expand the description a bit as to what an opaque value expression actually is, that would be great.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1658
@@ +1657,3 @@
+/// \code
+///   (a ?: c) + 42
+/// \endcode
----------------
Missing semicolon?

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1820
@@ -1729,1 +1819,3 @@
 
+/// \brief Matches predefined identifier expressions [C99 6.4.2.2]
+///
----------------
Missing a period at the end of the sentence.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1822
@@ +1821,3 @@
+///
+/// Example: Matches __func__)
+/// \code
----------------
Spurious ), I hope. ;-)

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1830
@@ +1829,3 @@
+
+/// \brief Matches C99 designated initializer expressions [C99 6.7.8]
+///
----------------
Missing period.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1846
@@ +1845,3 @@
+///   point ptarray[10] = { [2].y = 1.0, [0].x = 1.0 }; };
+///   point ptarray2[10] = { [2].y = 1.0, [2].x = 0.0, [0].x = 1.0 }; }
+/// \endcode
----------------
Neither of these look to be valid code. Both seem to have extra };, and the last one is missing a semicolon.

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3506
@@ +3505,3 @@
+                          internal::Matcher<Expr>, InnerMatcher) {
+  const Expr* const SubExpression =
+      internal::GetSourceExpressionMatcher<NodeType>::get(Node);
----------------
Is this formatting that clang-format produced? The * looks to be in the wrong place.


http://reviews.llvm.org/D17446