[cfe-commits] [PATCH] Change counter-intuitive behavior of isDerivedFrom

[Daniel Jasper]

Hi klimek,

http://llvm-reviews.chandlerc.com/D37

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D37?vs=101&id=102#toc

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/ASTMatchFinder.cpp
  unittests/ASTMatchers/ASTMatchersTest.cpp

Index: include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1108,11 +1108,11 @@
 /// \brief Matches C++ classes that are directly or indirectly derived from
 /// a class matching \c Base.
 ///
-/// Note that a class is considered to be also derived from itself.
+/// Note that a class is not considered to be derived from itself.
 ///
-/// Example matches X, Y, Z, C (Base == hasName("X"))
+/// Example matches Y, Z, C (Base == hasName("X"))
 /// \code
-///   class X;                // A class is considered to be derived from itself
+///   class X;
 ///   class Y : public X {};  // directly derived
 ///   class Z : public Y {};  // indirectly derived
 ///   typedef X A;
@@ -1137,6 +1137,18 @@
   return isDerivedFrom(hasName(BaseName));
 }
 
+/// \brief Similar to \c isDerivedFrom(), but also matches classes that directly
+/// match \c Base.
+inline internal::Matcher<CXXRecordDecl> isA(internal::Matcher<NamedDecl> Base) {
+  return anyOf(Base, isDerivedFrom(Base));
+}
+
+/// \brief Overloaded method as shortcut for \c isA(hasName(...)).
+inline internal::Matcher<CXXRecordDecl> isA(StringRef BaseName) {
+  assert(!BaseName.empty());
+  return isA(hasName(BaseName));
+}
+
 /// \brief Matches AST nodes that have child AST nodes that match the
 /// provided matcher.
 ///
Index: lib/ASTMatchers/ASTMatchFinder.cpp
===================================================================
--- lib/ASTMatchers/ASTMatchFinder.cpp
+++ lib/ASTMatchers/ASTMatchFinder.cpp
@@ -277,7 +277,7 @@
   bool VisitTypedefDecl(TypedefDecl *DeclNode) {
     // When we see 'typedef A B', we add name 'B' to the set of names
     // A's canonical type maps to.  This is necessary for implementing
-    // IsDerivedFrom(x) properly, where x can be the name of the base
+    // isDerivedFrom(x) properly, where x can be the name of the base
     // class or any of its aliases.
     //
     // In general, the is-alias-of (as defined by typedefs) relation
@@ -298,7 +298,7 @@
     //      `- E
     //
     // It is wrong to assume that the relation is a chain.  A correct
-    // implementation of IsDerivedFrom() needs to recognize that B and
+    // implementation of isDerivedFrom() needs to recognize that B and
     // E are aliases, even though neither is a typedef of the other.
     // Therefore, we cannot simply walk through one typedef chain to
     // find out whether the type name matches.
@@ -468,13 +468,11 @@
 };
 
 // Returns true if the given class is directly or indirectly derived
-// from a base type with the given name.  A class is considered to be
-// also derived from itself.
+// from a base type with the given name.  A class is not considered to be
+// derived from itself.
 bool MatchASTVisitor::classIsDerivedFrom(const CXXRecordDecl *Declaration,
                                          const Matcher<NamedDecl> &Base,
                                          BoundNodesTreeBuilder *Builder) {
-  if (Base.matches(*Declaration, this, Builder))
-    return true;
   if (!Declaration->hasDefinition())
     return false;
   typedef CXXRecordDecl::base_class_const_iterator BaseIterator;
@@ -523,6 +521,8 @@
     }
     assert(ClassDecl != NULL);
     assert(ClassDecl != Declaration);
+    if (Base.matches(*ClassDecl, this, Builder))
+      return true;
     if (classIsDerivedFrom(ClassDecl, Base, Builder))
       return true;
   }
Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===================================================================
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -101,12 +101,19 @@
   DeclarationMatcher IsDerivedFromX = recordDecl(isDerivedFrom("X"));
 
   EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsDerivedFromX));
-  EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsDerivedFromX));
-  EXPECT_TRUE(matches("class X {};", IsDerivedFromX));
-  EXPECT_TRUE(matches("class X;", IsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X {};", IsDerivedFromX));
+  EXPECT_TRUE(notMatches("class X;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("class Y;", IsDerivedFromX));
   EXPECT_TRUE(notMatches("", IsDerivedFromX));
 
+  DeclarationMatcher IsAX = recordDecl(isA("X"));
+
+  EXPECT_TRUE(matches("class X {}; class Y : public X {};", IsAX));
+  EXPECT_TRUE(matches("class X {};", IsAX));
+  EXPECT_TRUE(matches("class X;", IsAX));
+  EXPECT_TRUE(notMatches("class Y;", IsAX));
+  EXPECT_TRUE(notMatches("", IsAX));
+
   DeclarationMatcher ZIsDerivedFromX =
       recordDecl(hasName("Z"), isDerivedFrom("X"));
   EXPECT_TRUE(
@@ -458,7 +465,6 @@
   DeclarationMatcher NotClassX =
       recordDecl(
           isDerivedFrom("Y"),
-          unless(hasName("Y")),
           unless(hasName("X")));
   EXPECT_TRUE(notMatches("", NotClassX));
   EXPECT_TRUE(notMatches("class Y {};", NotClassX));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37.2.patch
Type: text/x-patch
Size: 4967 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120907/c2483365/attachment.bin>