lgtm
On Fri, Aug 17, 2012 at 8:18 PM, Dmitri Gribenko <[email protected]> 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 <[email protected]> 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 <[email protected]>*/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
