================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3356
@@ +3355,3 @@
+/// statement.
+AST_MATCHER_P(SwitchStmt, eachCase, internal::Matcher<SwitchCase>,
+ InnerMatcher) {
----------------
Daniel Jasper wrote:
> I am not very happy with the naming here:
> 1) This should follow the conventions we already have. Meaning, this should
> be called forEach.. Ideally, we would then also add a corresponding has...
> 2) It is slightly weird, that this is called ..Case, when in fact it matches
> on SwitchCases. I'd vote for either calling this ..SwitchCase or only
> matching on CaseStmts.
Renamed.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3378
@@ +3377,3 @@
+/// the GNU case range extension, matches the constant given in the case
+/// statement.
+AST_MATCHER_P(SwitchCase, hasCaseConstant, internal::Matcher<Expr>,
----------------
Daniel Jasper wrote:
> Please add a usage example.
Done.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3379
@@ +3378,3 @@
+/// statement.
+AST_MATCHER_P(SwitchCase, hasCaseConstant, internal::Matcher<Expr>,
+ InnerMatcher) {
----------------
Daniel Jasper wrote:
> I think this should just be a matcher on CaseStmt (making the dyn_cast)
> unnecessary. Or am I missing something?
I initially wrote this matcher this way so that users could write
forEachSwitchCase(hasCaseConstant()) (which is already quite verbose) without
also using caseStmt(). But I guess this would make it consistent with other
matchers. Changed.
================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:2922
@@ +2921,3 @@
+ EXPECT_TRUE(
+ notMatches("void x() { switch(42); }",
switchStmt(eachCase(caseStmt()))));
+ EXPECT_TRUE(matches("void x() { switch(42) case 42:; }",
----------------
Daniel Jasper wrote:
> There are no tests that verify that "eachCase" binds all matches. We need
> something like:
>
> EXPECT_TRUE(matchAndVerifyResultTrue(
> "void x() { switch (42) { case 1: case 2: break; case 3: default:
> break } }",
> switchStmt(eachCase(caseStmt().bind("x"))),
> new VerifyIdIsBoundTo<CaseStmt>("x", 3)));
I added something like this.
================
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3355
@@ +3354,3 @@
+/// \brief Matches each case or default statement belonging to the given switch
+/// statement.
+AST_MATCHER_P(SwitchStmt, eachCase, internal::Matcher<SwitchCase>,
----------------
Daniel Jasper wrote:
> Please add a usage example (like all the other matchers have).
Done.
http://llvm-reviews.chandlerc.com/D744
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
