[clang-rename] handle overridden functions correctly

1. Renaming overridden functions only works for two levels of "overriding
   hierarchy". clang-rename should recursively add overridden methods.
2. Make use of forEachOverridden AST Matcher.
3. Fix two tests.

Reviewers: alexfh

Differential Revision: https://reviews.llvm.org/D23009

llvm-svn: 277356

4 files changed, 36 insertions, 19 deletions

diff --git a/clang-tools-extra/clang-rename/USRFindingAction.cpp b/clang-tools-extra/clang-rename/USRFindingAction.cpp
index 88c01a601b7..e020385ead3 100644
--- a/clang-tools-extra/clang-rename/USRFindingAction.cpp
+++ b/clang-tools-extra/clang-rename/USRFindingAction.cpp
@@ -54,7 +54,12 @@ public:
 
   void Find() {
     USRSet.insert(getUSRForDecl(FoundDecl));
-    addUSRsFromOverrideSetsAndCtorDtors();
+    if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) {
+      addUSRsFromOverrideSets(MethodDecl);
+    }
+    if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) {
+      addUSRsOfCtorDtors(RecordDecl);
+    }
     addMatchers();
     Finder.matchAST(Context);
     USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
@@ -63,12 +68,11 @@ public:
 private:
   void addMatchers() {
     const auto CXXMethodDeclMatcher =
-        cxxMethodDecl(isVirtual()).bind("cxxMethodDecl");
+        cxxMethodDecl(forEachOverridden(cxxMethodDecl().bind("cxxMethodDecl")));
     Finder.addMatcher(CXXMethodDeclMatcher, this);
   }
 
-  // FIXME: Implement hasOverriddenMethod and matchesUSR matchers to make
-  // lookups more efficient.
+  // FIXME: Implement matchesUSR matchers to make lookups more efficient.
   virtual void run(const MatchFinder::MatchResult &Result) {
     const auto *VirtualMethod =
         Result.Nodes.getNodeAs<CXXMethodDecl>("cxxMethodDecl");
@@ -83,20 +87,19 @@ private:
     }
   }
 
-  void addUSRsFromOverrideSetsAndCtorDtors() {
-    // If D is CXXRecordDecl we should add all USRs of its constructors.
-    if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) {
-      RecordDecl = RecordDecl->getDefinition();
-      for (const auto *CtorDecl : RecordDecl->ctors()) {
-        USRSet.insert(getUSRForDecl(CtorDecl));
-      }
-      USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
+  void addUSRsOfCtorDtors(const CXXRecordDecl *RecordDecl) {
+    RecordDecl = RecordDecl->getDefinition();
+    for (const auto *CtorDecl : RecordDecl->ctors()) {
+      USRSet.insert(getUSRForDecl(CtorDecl));
     }
-    // If D is CXXMethodDecl we should add all USRs of its overriden methods.
-    if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) {
-      for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
-        USRSet.insert(getUSRForDecl(OverriddenMethod));
-      }
+    USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
+  }
+
+  void addUSRsFromOverrideSets(const CXXMethodDecl *MethodDecl) {
+    USRSet.insert(getUSRForDecl(MethodDecl));
+    for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+      // Recursively visit each OverridenMethod.
+      addUSRsFromOverrideSets(OverriddenMethod);
     }
   }
 
diff --git a/clang-tools-extra/test/clang-rename/FunctionOverride.cpp b/clang-tools-extra/test/clang-rename/FunctionOverride.cpp
new file mode 100644
index 00000000000..476a6bafbb4
--- /dev/null
+++ b/clang-tools-extra/test/clang-rename/FunctionOverride.cpp
@@ -0,0 +1,10 @@
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=318 -new-name=bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+class A { virtual void foo(); };    // CHECK: class A { virtual void bar(); };
+class B : public A { void foo(); }; // CHECK: class B : public A { void bar(); };
+class C : public B { void foo(); }; // CHECK: class C : public B { void bar(); };
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of Foo when changing
+// this file.
diff --git a/clang-tools-extra/test/clang-rename/FunctionWithClassFindByName.cpp b/clang-tools-extra/test/clang-rename/FunctionWithClassFindByName.cpp
index 32681b6714c..7d1f85653d2 100644
--- a/clang-tools-extra/test/clang-rename/FunctionWithClassFindByName.cpp
+++ b/clang-tools-extra/test/clang-rename/FunctionWithClassFindByName.cpp
@@ -1,4 +1,6 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -old-name=Foo -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 void foo() {
 }
diff --git a/clang-tools-extra/test/clang-rename/UserDefinedConversion.cpp b/clang-tools-extra/test/clang-rename/UserDefinedConversion.cpp
index 9f61bf08c42..13bac334929 100644
--- a/clang-tools-extra/test/clang-rename/UserDefinedConversion.cpp
+++ b/clang-tools-extra/test/clang-rename/UserDefinedConversion.cpp
@@ -1,4 +1,6 @@
-// RUN: clang-rename -offset=143 -new-name=Bar %s -- | FileCheck %s
+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=205 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
 class Foo {};             // CHECK: class Bar {};