Author: sbenza Date: Mon Oct 13 12:38:12 2014 New Revision: 219622 URL: http://llvm.org/viewvc/llvm-project?rev=219622&view=rev Log: Fix bug in DynTypedMatcher::constructVariadic() that would cause false negatives.
Summary: Change r219118 fixed the bug for anyOf and eachOf, but it is still present for unless. The variadic wrapper doesn't have enough information to know how to restrict the type. Different operators handle restrict failures in different ways. Reviewers: klimek Subscribers: klimek, cfe-commits Differential Revision: http://reviews.llvm.org/D5731 Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp Modified: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp?rev=219622&r1=219621&r2=219622&view=diff ============================================================================== --- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp (original) +++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp Mon Oct 13 12:38:12 2014 @@ -89,18 +89,19 @@ static llvm::ManagedStatic<TrueMatcherIm DynTypedMatcher DynTypedMatcher::constructVariadic( VariadicOperatorFunction Func, std::vector<DynTypedMatcher> InnerMatchers) { assert(InnerMatchers.size() > 0 && "Array must not be empty."); - DynTypedMatcher Result = InnerMatchers[0]; - // Use the least derived type as the restriction for the wrapper. - // This allows mismatches to be resolved on the inner matchers. - for (const DynTypedMatcher &M : InnerMatchers) { - assert(Result.SupportedKind.isSame(M.SupportedKind) && - "SupportedKind must match!"); - Result.RestrictKind = - ast_type_traits::ASTNodeKind::getMostDerivedCommonAncestor( - Result.RestrictKind, M.RestrictKind); - } - Result.Implementation = new VariadicMatcher(Func, std::move(InnerMatchers)); - return Result; + assert(std::all_of(InnerMatchers.begin(), InnerMatchers.end(), + [&InnerMatchers](const DynTypedMatcher &M) { + return InnerMatchers[0].SupportedKind.isSame(M.SupportedKind); + }) && + "SupportedKind must match!"); + + // We must relax the restrict kind here. + // The different operators might deal differently with a mismatch. + // Make it the same as SupportedKind, since that is the broadest type we are + // allowed to accept. + return DynTypedMatcher(InnerMatchers[0].SupportedKind, + InnerMatchers[0].SupportedKind, + new VariadicMatcher(Func, std::move(InnerMatchers))); } DynTypedMatcher DynTypedMatcher::trueMatcher( Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp?rev=219622&r1=219621&r2=219622&view=diff ============================================================================== --- cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp (original) +++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTest.cpp Mon Oct 13 12:38:12 2014 @@ -591,6 +591,11 @@ TEST(DeclarationMatcher, MatchNot) { EXPECT_TRUE(matches("class X { class Z {}; };", ClassXHasNotClassY)); EXPECT_TRUE(notMatches("class X { class Y {}; class Z {}; };", ClassXHasNotClassY)); + + DeclarationMatcher NamedNotRecord = + namedDecl(hasName("Foo"), unless(recordDecl())); + EXPECT_TRUE(matches("void Foo(){}", NamedNotRecord)); + EXPECT_TRUE(notMatches("struct Foo {};", NamedNotRecord)); } TEST(DeclarationMatcher, HasDescendant) { Modified: cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp?rev=219622&r1=219621&r2=219622&view=diff ============================================================================== --- cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp (original) +++ cfe/trunk/unittests/ASTMatchers/Dynamic/RegistryTest.cpp Mon Oct 13 12:38:12 2014 @@ -380,6 +380,13 @@ TEST_F(RegistryTest, VariadicOp) { EXPECT_FALSE(matches("class Bar{ int Foo; };", D)); EXPECT_TRUE(matches("class OtherBar{ int Foo; };", D)); + + D = constructMatcher( + "namedDecl", constructMatcher("hasName", std::string("Foo")), + constructMatcher("unless", constructMatcher("recordDecl"))) + .getTypedMatcher<Decl>(); + EXPECT_TRUE(matches("void Foo(){}", D)); + EXPECT_TRUE(notMatches("struct Foo {};", D)); } TEST_F(RegistryTest, Errors) { _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
