I fixed the mentioned variable name and changed the isInteger matcher to use the provided macro (sorry I missed that earlier). Submitted as r160115. Thanks for these patches!
On Wed, Jul 11, 2012 at 11:06 PM, Daniel Jasper <[email protected]> wrote: > +TEST(AllOF, CorrectOverloads) { > + const char program[] = > + "struct T { }; int f(int, T*); void g(int x) { T t; f(x, &t); }"; > > Should be Program (upper case). Everything else looks good IMO. > > > On Wed, Jul 11, 2012 at 10:55 PM, Sam Panzer <[email protected]> wrote: > >> Here is the fixed-up set of patches: >> >> >> On Tue, Jul 10, 2012 at 10:26 PM, Daniel Jasper <[email protected]>wrote: >> >>> Thanks for the patches. A few comments: >>> >>> loop-matchers.patch: >>> >>> +/// Example: >>> +/// forSmt(hasIncrement(unaryOperator(hasOperatorName("++")))) >>> >>> >> Done. >> >> >>> 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). >>> >> >> Done. >> >> >>> >>> +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. >>> >>> >> Done. >> >> >>> >>> isInteger.patch: >>> Use EXPECT_TRUE(notMatches(. >>> >>> >> Done. >> >> >>> >>> allof.patch: >>> >>> +TEST(AllOF, CorrectOverloads) { >>> >>> nit: AllOf >>> also: The test name could contain a bit more information. What does it >>> actually test? >>> >> >> When I first tried to use allOf with three parameters, I got a compiler >> error complaining about an undefined symbol 'AllOf'. I created this test to >> try to catch the original error, which is triggered if the test case is >> added but the spelling change in ASTMatchers.h is not. I renamed the test >> AllOf.AllOverloadsWork, which I think is a little clearer. >> >> >>> >>> >>> On Wed, Jul 11, 2012 at 1:11 AM, Sam Panzer <[email protected]> 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 >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
