[cfe-commits] [PATCH] AST Matchers: function and class template matchers

[Manuel Klimek]

lgtm

On Fri, Aug 17, 2012 at 8:18 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> Hi Manuel,
>
> Thank you for the review!  Please find attached an updated patch.
>
> On Fri, Aug 17, 2012 at 10:58 AM, Manuel Klimek <klimek at google.com> wrote:
>> @@ -385,7 +393,15 @@
>>  ///   void f();
>>  const internal::VariadicDynCastAllOfMatcher<Decl, FunctionDecl> function;
>>
>> +/// \brief Matches C++ function template declarations.
>> +///
>> +/// Example matches f
>> +///   template<class T> void f(T t) {}
>> +const internal::VariadicDynCastAllOfMatcher<
>> +  Decl,
>> +  FunctionTemplateDecl> functionTemplate;
>>
>> +
>>
>> Any reason for the additional newline?
>
> Removed.
>
>> +TEST(DeclarationMatcher, MatchClassTemplate) {
>> +  DeclarationMatcher ClassX = classTemplate(hasName("X"));
>> +  EXPECT_TRUE(notMatches("", ClassX));
>>
>> I don't think we need to test this  - or do you think it provides any
>> information (one problem is that there is some stuff that's declared
>> by default, but it depends on the system we compile on etc).
>
> Removed.
>
>> +  EXPECT_TRUE(notMatches("class X;", ClassX));
>> +  EXPECT_TRUE(notMatches("class X {};", ClassX));
>> +  EXPECT_TRUE(matches("template<class T> class X {};", ClassX));
>> +  EXPECT_TRUE(notMatches("template<class T> class X { };"
>> +                         "template<> class X<int> { int a; };",
>> +              classTemplate(hasName("X"),
>> hasDescendant(field(hasName("a"))))));
>> +}
>> +
>>
>> Can you split up this test much like the other tests below (which are
>> great!). I know that not all the tests currently follow the correct
>> style, but I'd like us to not introduce any new ones there...
>> (What I mean is: have multiple TEST(ClassTemplate, Matches... or
>> DoesNotMatch...))
>
> Done.
>
> Dmitri
>
> --
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/