[clang-rename] improve USRFindingAction

[omtcyfz]

Summary

• improve templated class renaming, namely add capabilities of finding partial and full specializations
• fix virtual function and their overriding functions renaming

[alexfh]

test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp

2

It makes sense to move RUN lines to the end of the file (and add -fno-delayed-template-parsing, if windows buildbots start complaining).

test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp

3

This test looks exactly as the one above except for the offset. Instead of duplicating tests, we can run clang-rename multiple times and use different -check-prefix with FileCheck. In this case moving the RUN lines to the bottom makes even more sense.

Also, instructions for updating -offset are incorrect, if you have multiple different offsets.

[omtcyfz]

test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp

3

Will it work if I add multiple invocations while not adding different -check-prefix? The replacements are the same across the runs.

[omtcyfz]

test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp

2

It makes sense to move RUN lines to the end of the file

You mean for all tests?

[omtcyfz]

Removed dump() call used for debugging purposes.
Removed unneeded header inclusion in USRFindingAction.cpp.

[alexfh]

test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp

2

Yes, but it's better to modify unrelated tests in a separate patch.

test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp

4

Yep, should also work with the same check prefix, if the replacements are the same.

[omtcyfz]

Merged multiple identical tests into a single test with multiple clang-rename invocations.

[omtcyfz]

test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp

4

Done.

[alexfh]

LG

clang-rename/USRFinder.cpp

public:
   bool VisitTypeLoc(const TypeLoc Loc) {
     const auto TypeBeginLoc = Loc.getBeginLoc();
     const auto TypeEndLoc = Lexer::getLocForEndOfToken(
         TypeBeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
     if (const auto *TemplateTypeParm =
             dyn_cast<TemplateTypeParmType>(Loc.getType())) {
       return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
     }
+    if (const auto *TemplateSpecType =
+            dyn_cast<TemplateSpecializationType>(Loc.getType())) {
+      return setResult(TemplateSpecType->getTemplateName().getAsTemplateDecl(),
+                       TypeBeginLoc, TypeEndLoc);
+    }
     return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
                      TypeEndLoc);
   }
 
   // Other:
 
   const NamedDecl *getNamedDecl() { return Result; }
 

clang-rename/USRFindingAction.cpp

 namespace clang {
 namespace rename {
 
 namespace {
 // \brief NamedDeclFindingConsumer should delegate finding USRs of given Decl to
 // AdditionalUSRFinder. AdditionalUSRFinder adds USRs of ctor and dtor if given
 // Decl refers to class and adds USRs of all overridden methods if Decl refers
 // to virtual method.
-class AdditionalUSRFinder : public MatchFinder::MatchCallback {
+class AdditionalUSRFinder : public RecursiveASTVisitor<AdditionalUSRFinder> {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
                              std::vector<std::string> *USRs)
-    : FoundDecl(FoundDecl), Context(Context), USRs(USRs), USRSet(), Finder() {}
+    : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
-    USRSet.insert(getUSRForDecl(FoundDecl));
+    // Fill OverriddenMethods and PartialSpecs storages.
+    TraverseDecl(Context.getTranslationUnitDecl());
     if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(FoundDecl)) {
-      addUSRsFromOverrideSets(MethodDecl);
+      addUSRsOfOverridenFunctions(MethodDecl);
+      for (const auto &OverriddenMethod : OverriddenMethods) {
+        OverriddenMethod->dump();
+        if (checkIfOverriddenFunctionAscends(OverriddenMethod)) {
+          USRSet.insert(getUSRForDecl(OverriddenMethod));
         }
-    if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) {
-      addUSRsOfCtorDtors(RecordDecl);
       }
-    addMatchers();
-    Finder.matchAST(Context);
+    } else if (const auto *RecordDecl = dyn_cast<CXXRecordDecl>(FoundDecl)) {
+      handleCXXRecordDecl(RecordDecl);
+    } else if (const auto *TemplateDecl =
+               dyn_cast<ClassTemplateDecl>(FoundDecl)) {
+      handleClassTemplateDecl(TemplateDecl);
+    } else {
+      USRSet.insert(getUSRForDecl(FoundDecl));
+    }
     USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
   }
 
+  bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
+    if (MethodDecl->isVirtual()) {
+      OverriddenMethods.push_back(MethodDecl);
+    }
+    return true;
+  }
+
+  bool VisitClassTemplatePartialSpecializationDecl(
+      const ClassTemplatePartialSpecializationDecl *PartialSpec) {
+    PartialSpecs.push_back(PartialSpec);
+    return true;
+  }
+
 private:
-  void addMatchers() {
-    const auto CXXMethodDeclMatcher =
-        cxxMethodDecl(forEachOverridden(cxxMethodDecl().bind("cxxMethodDecl")));
-    Finder.addMatcher(CXXMethodDeclMatcher, this);
+  void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
+    RecordDecl = RecordDecl->getDefinition();
+    if (const auto *ClassTemplateSpecDecl
+            = dyn_cast<ClassTemplateSpecializationDecl>(RecordDecl)) {
+      handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
+    }
+    addUSRsOfCtorDtors(RecordDecl);
   }
 
-  // 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;
-      }
+  void handleClassTemplateDecl(const ClassTemplateDecl *TemplateDecl) {
+    for (const auto *Specialization : TemplateDecl->specializations()) {
+      addUSRsOfCtorDtors(Specialization);
+    for (const auto *PartialSpec : PartialSpecs) {
+      if (PartialSpec->getSpecializedTemplate() == TemplateDecl) {
+        addUSRsOfCtorDtors(PartialSpec);
       }
-    if (Found) {
-      USRSet.insert(getUSRForDecl(VirtualMethod));
     }
+    addUSRsOfCtorDtors(TemplateDecl->getTemplatedDecl());
   }
 
   void addUSRsOfCtorDtors(const CXXRecordDecl *RecordDecl) {
     RecordDecl = RecordDecl->getDefinition();
     for (const auto *CtorDecl : RecordDecl->ctors()) {
       USRSet.insert(getUSRForDecl(CtorDecl));
     }
     USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
+    USRSet.insert(getUSRForDecl(RecordDecl));
   }
 
-  void addUSRsFromOverrideSets(const CXXMethodDecl *MethodDecl) {
+  void addUSRsOfOverridenFunctions(const CXXMethodDecl *MethodDecl) {
     USRSet.insert(getUSRForDecl(MethodDecl));
     for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
       // Recursively visit each OverridenMethod.
-      addUSRsFromOverrideSets(OverriddenMethod);
+      addUSRsOfOverridenFunctions(OverriddenMethod);
+    }
+  }
+
+  bool checkIfOverriddenFunctionAscends(const CXXMethodDecl *MethodDecl) {
+    for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+      if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end()) {
+        return true;
+      }
+      return checkIfOverriddenFunctionAscends(OverriddenMethod);
     }
+    return false;
   }
 
   const Decl *FoundDecl;
   ASTContext &Context;
   std::vector<std::string> *USRs;
   std::set<std::string> USRSet;
-  MatchFinder Finder;
+  std::vector<const CXXMethodDecl*> OverriddenMethods;
+  std::vector<const ClassTemplatePartialSpecializationDecl*> PartialSpecs;
 };
 } // namespace
 
 struct NamedDeclFindingConsumer : public ASTConsumer {
   void HandleTranslationUnit(ASTContext &Context) override {
     const auto &SourceMgr = Context.getSourceManager();
     // The file we look for the USR in will always be the main source file.
     const auto Point = SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())

test/clang-rename/ComplexFunctionOverride.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=307 -new-name=bar %t.cpp -i -- -std=c++11
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+struct A {
+  virtual void foo();   // CHECK: virtual void bar();
+};
+
+struct B : A {
+  void foo() override;  // CHECK: void bar() override;
+};
+
+struct C : B {
+  void foo() override;  // CHECK: void bar() override;
+};
+
+struct D : B {
+  void foo() override;  // CHECK: void bar() override;
+};
+
+struct E : D {
+  void foo() override;  // CHECK: void bar() override;
+};

test/clang-rename/TemplateClassInstantiationFindByDeclaration.cpp

 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=287 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=159 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
 template <typename T>
-class Foo {               // CHECK: class Bar;
+class Foo {               // CHECK: class Bar {
 public:
   T foo(T arg, T& ref, T* ptr) {
     T value;
     int number = 42;
     value = (T)number;
     value = static_cast<T>(number);
     return value;
   }

test/clang-rename/TemplateClassInstantiationFindByTypeUse.cpp

 // RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=703 -new-name=Bar %t.cpp -i --
+// RUN: clang-rename -offset=575 -new-name=Bar %t.cpp -i --
 // RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
 
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename classes with templates
-// correctly.
-// XFAIL: *
-
 template <typename T>
-class Foo {               // CHECK: class Bar;
+class Foo {               // CHECK: class Bar {
 public:
   T foo(T arg, T& ref, T* ptr) {
     T value;
     int number = 42;
     value = (T)number;
     value = static_cast<T>(number);
     return value;
   }

test/clang-rename/TemplateClassInstantiationFindByUninstantiatedType.cpp

+// RUN: cat %s > %t.cpp
+// RUN: clang-rename -offset=440 -new-name=Bar %t.cpp -i --
+// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
+
+template <typename T>
+class Foo {               // CHECK: class Bar {
+public:
+  T foo(T arg, T& ref, T* ptr) {
+    T value;
+    int number = 42;
+    value = (T)number;
+    value = static_cast<T>(number);
+    return value;
+  }
+  static void foo(T value) {}
+  T member;
+};
+
+template <typename T>
+void func() {
+  Foo<T> obj;             // CHECK: Bar<T> obj;
+  obj.member = T();
+  Foo<T>::foo();          // CHECK: Bar<T>::foo();
+}
+
+int main() {
+  Foo<int> i;             // CHECK: Bar<int> i;
+  i.member = 0;
+  Foo<int>::foo(0);       // CHECK: Bar<int>::foo(0);
+
+  Foo<bool> b;            // CHECK: Bar<bool> b;
+  b.member = false;
+  Foo<bool>::foo(false);  // CHECK: Bar<bool>::foo(false);
+
+  return 0;
+}
+
+// Use grep -FUbo 'Foo' <file> to get the correct offset of foo when changing
+// this file.