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
>>
>>
>

Attachment: allof.patch
Description: Binary data

Attachment: isInteger.patch
Description: Binary data

Attachment: loop-matchers.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to