[cfe-commits] [PATCH] C++11 matchers

Daniel Jasper reviews at llvm-reviews.chandlerc.com
Thu Sep 20 07:35:11 PDT 2012



================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:2110
@@ -2088,3 +2109,3 @@
 TEST(Member, MatchesMemberAllocationFunction) {
-  EXPECT_TRUE(matches("namespace std { typedef typeof(sizeof(int)) size_t; }"
+  EXPECT_TRUE(matches("namespace std { typedef decltype(sizeof(int)) size_t; }"
                       "class X { void *operator new(std::size_t); };",
----------------
I'd rather you don't change the content of this test. Rather use matchesConditionally() directly here.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:815
@@ +814,3 @@
+  EXPECT_TRUE(matches("#include <initializer_list>\n"
+                      "void f() { for (auto &a : {1, 2, 3}); }",
+                      forRangeStmt()));
----------------
Add a test to and change comments of both the forStmt() and the forRangeStmt() matchers so that it is clear whether one matches the other or not.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:820
@@ +819,3 @@
+TEST(Matcher, UserDefinedLiteral) {
+  EXPECT_TRUE(matches("constexpr char operator \"\" _inc (const char i)"
+                      "{ return i + 1; } char c = 'a'_inc;",
----------------
I know that we don't do much formatting, but (as I am not yet very familiar with the C++11 feature), I found this quite hard to read. Could you at least add a newline after "}"?

================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:805
@@ +804,3 @@
+  Stmt,
+  CXXNullPtrLiteralExpr> nullPtrLiteralExpr;
+
----------------
I wonder whether this should be nullPtrLiteral; for consistency with the other matchers. That would then (at a later point entail renaming the AST node). Manuel, any thoughts?


http://llvm-reviews.chandlerc.com/D46



More information about the cfe-commits mailing list