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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@219622 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/ASTMatchers/ASTMatchersInternal.cpp b/lib/ASTMatchers/ASTMatchersInternal.cpp
index f6fd5ba2a3f6f41fe970886f85131824d83a01a5..c8e5852475c2365a54da46f128ce4f4a4aca2087 100644 (file)
--- a/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -89,18 +89,19 @@ static llvm::ManagedStatic<TrueMatcherImpl> TrueMatcherInstance;
 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(

diff --git a/unittests/ASTMatchers/ASTMatchersTest.cpp b/unittests/ASTMatchers/ASTMatchersTest.cpp
index 2195604799bc971c6a8f7033ab61fbb24665d728..d458431ecbb2bd9d0f018fc47d36972a4fc47ea2 100644 (file)
--- a/unittests/ASTMatchers/ASTMatchersTest.cpp
+++ b/unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -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) {

diff --git a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
index 9011b3f65ecc5c1c87e851512bcf19a3d8e3666f..5483f8f3580407425f89774ec0459ba3cf24fa35 100644 (file)
--- a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -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) {