[clang-rename] handle overridden functions correctly

[omtcyfz]

Summary

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.

[alexfh]

test/clang-rename/FunctionOverride.cpp

4

I guess, this test will pass even if clang-rename doesn't do any changes, since the pattern in the CHECK line will match with itself. There are multiple ways to fix this. For instance, you can anchor the pattern to the start of line (// CHECK: {{ˆ}}class A ...) or add | sed 's,//.*,//' to the pipeline before | FileCheck %s.

[alexfh]

LG

clang-rename/USRFindingAction.cpp

 class AdditionalUSRFinder : public MatchFinder::MatchCallback {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
                              std::vector<std::string> *USRs)
     : FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
 
   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());
   }
 
 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");
     bool Found = false;
     for (const auto &OverriddenMethod : VirtualMethod->overridden_methods()) {
       if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end()) {
         Found = true;
       }
     }
     if (Found) {
       USRSet.insert(getUSRForDecl(VirtualMethod));
     }
   }
 
-  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()));
-    }
-    // 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));
-      }
+  void addUSRsOfCtorDtors(const CXXRecordDecl *RecordDecl) {
+
+  void addUSRsFromOverrideSets(const CXXMethodDecl *MethodDecl) {
+    USRSet.insert(getUSRForDecl(MethodDecl));
+      // Recursively visit each OverridenMethod.
+      addUSRsFromOverrideSets(OverriddenMethod);
     }
   }
 
   const Decl *FoundDecl;
   ASTContext &Context;
   std::vector<std::string> *USRs;
   std::set<std::string> USRSet;
   MatchFinder Finder;

test/clang-rename/FunctionOverride.cpp

+// 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.