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

[Daniel Jasper]

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 <djasper at google.com> 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 <panzer at google.com> wrote:
>
>> Here is the fixed-up set of patches:
>>
>>
>> On Tue, Jul 10, 2012 at 10:26 PM, Daniel Jasper <djasper at google.com>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 <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/20120712/7921cadf/attachment.html>