[cfe-commits] [Patch] x 3: Matchers for components of loops, a spelling fix, isInteger matcher

Daniel Jasper djasper at google.com
Tue Jul 10 22:26:53 PDT 2012


Thanks for the patches. A few comments:

loop-matchers.patch:

  +/// Example:
  +///     forSmt(hasIncrement(unaryOperator(hasOperatorName("++"))))

Typo: Should be forStmt. Same in boths comments.

  +AST_MATCHER_P(clang::ForStmt, hasIncrement,
internal::Matcher<clang::Stmt>,
  +              InnerMatcher) {

Remove all the "clang::" (here and everywhere else).

  +TEST(For, NegativeForLoopInternals) {
  +  EXPECT_FALSE(matches("void f(){ for (int i = 0; ; ++i); }",

We prefer "EXPECT_TRUE(notMatches(". I think "EXPECT_FALSE(matches(" will
always pass if there are syntax errors.


isInteger.patch:
Use EXPECT_TRUE(notMatches(.


allof.patch:

  +TEST(AllOF, CorrectOverloads) {

nit: AllOf
also: The test name could contain a bit more information. What does it
actually test?


On Wed, Jul 11, 2012 at 1:11 AM, Sam Panzer <panzer at google.com> wrote:

> There are three patches attached here. One adds matchers for the various
> parts of a for loop (initializer, condition, increment), as well as
> extending the hasBody matcher to work for while and do-while loops. The
> second patch adds an isInteger matcher for types.
> The third patch fixes a bug in allOf, where a few of the name chages
> (AllOf --> allOf) had been missed.
>
> All matchers come with unit tests.
>
> Thoughts?
> -Sam
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120711/4c9771b6/attachment.html>


More information about the cfe-commits mailing list