kbobyrev created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
This patch introduces new canonicalization rules which are used for AST-based
rename in Clangd. By comparing two canonical declarations of inspected nodes,
Clangd determines whether both of them belong to the same entity user would
like to rename. Such functionality is relatively concise compared to the
Clang-Rename API that is used right now. It also helps to overcome the
limitations that Clang-Rename originally had and helps to eliminate several
classes of bugs.
Clangd AST-based rename currently relies on Clangd-Rename functionality, which
has a better test coverage and hence provides some correctness guarantees. This
pattch decouples two rename features and therefore introduces new challenges to
the Clangd maintenance. To prevent regressions and improve stability of Clangd
rename feature, there are new unittests that come with this change.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D71880
Files:
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/RenameTests.cpp
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -88,10 +88,87 @@
}
TEST(RenameTest, WithinFileRename) {
- // rename is runnning on all "^" points, and "[[]]" ranges point to the
- // identifier that is being renamed.
+ // For each "^" this test moves cursor to its location and applies renaming
+ // while checking that all identifiers enclosed in [[]] ranges are handled
+ // correctly.
+ // TODO(kirillbobyrev): Add more tests.
llvm::StringRef Tests[] = {
- // Function.
+ // Templated static method instantiation.
+ R"cpp(
+ template<typename T>
+ class Foo {
+ public:
+ static T [[f^oo]]() {}
+ }
+
+ void bar() {
+ Foo<int>::[[f^oo]]();
+ }
+ )cpp",
+
+ // Templated method instantiation.
+ R"cpp(
+ template<typename T>
+ class Foo {
+ public:
+ T [[f^oo]]() {}
+ }
+
+ void bar() {
+ Foo<int>().[[f^oo]]();
+ }
+ )cpp",
+
+ // Class template (partial) specialization forward declarations.
+ R"cpp(
+ template<typename T, typename U=bool>
+ class [[Foo^]];
+
+ template<typename T, typename U>
+ class [[Foo^]] {};
+
+ template<typename T=int, typename U>
+ class [[Foo^]];
+ )cpp",
+
+ // Class template (full) specialization forward declaration.
+ R"cpp(
+ template<typename T=float, typename U=int>
+ class [[Foo^]];
+
+ template<typename T, typename U>
+ class [[Foo^]] {};
+ )cpp",
+
+ // Function template specialization forward declaration.
+ R"cpp(
+ template<typename T=int, typename U=bool>
+ U [[foo^]]();
+
+ template<typename T, typename U>
+ U [[foo^]]() {};
+ )cpp",
+
+ // Function template specialization forward declaration.
+ R"cpp(
+ template<typename T, typename U>
+ U [[foo^]]() {};
+
+ template<typename T=int, typename U=bool>
+ U [[foo^]]();
+ )cpp",
+
+ // Function template specialization forward declaration without function
+ // definition.
+ R"cpp(
+ template<typename T=int, typename U=bool>
+ U [[foo^]]();
+
+ template<typename T, typename U>
+ U [[foo^]]();
+ )cpp",
+
+ // Simple recursive function.
R"cpp(
void [[foo^]]() {
[[fo^o]]();
@@ -116,15 +193,16 @@
}
)cpp",
- // Rename class, including constructor/destructor.
+ // Class, its constructor and destructor.
R"cpp(
class [[F^oo]] {
+ public:
[[F^oo]]();
- ~[[Foo]]();
+ ~[[Fo^o]]();
void foo(int x);
};
- [[Foo]]::[[Fo^o]]() {}
- void [[Foo]]::foo(int x) {}
+ [[Fo^o]]::[[Fo^o]]() {}
+ void [[Fo^o]]::foo(int x) {}
)cpp",
// Class in template argument.
@@ -175,9 +253,9 @@
class [[F^oo]]<T*> {};
void test() {
- [[Foo]]<int> x;
- [[Foo]]<bool> y;
- [[Foo]]<int*> z;
+ [[F^oo]]<int> x;
+ [[Fo^o]]<bool> y;
+ [[Foo^]]<int*> z;
}
)cpp",
@@ -303,7 +381,7 @@
void qoo() {
[[foo]]();
- boo([[foo]]());
+ boo([[fo^o]]());
M1();
boo(M1());
M2([[foo]]());
@@ -396,7 +474,7 @@
}
)cpp",
- // template class in template argument list.
+ // Template class in template argument list.
R"cpp(
template<typename T>
class [[Fo^o]] {};
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -209,6 +209,57 @@
llvm::inconvertibleErrorCode());
}
+const Decl *getRenameRootDecl(const ClassTemplateSpecializationDecl *D) {
+ return D->getSpecializedTemplate()->getTemplatedDecl();
+}
+const Decl *getRenameRootDecl(const TemplateDecl *D) {
+ return D->getTemplatedDecl();
+}
+const Decl *getRenameRootDecl(const CXXMethodDecl *D) {
+ auto *Result = D;
+ if (const auto *InstantiatedMethod = D->getInstantiatedFromMemberFunction())
+ Result = llvm::cast<CXXMethodDecl>(InstantiatedMethod);
+ while (Result->isVirtual() && Result->size_overridden_methods())
+ Result = *Result->overridden_methods().begin();
+ return Result;
+}
+const Decl *getRenameRootDecl(const FunctionDecl *D) {
+ const auto *Definition = D->getDefinition();
+ auto *Candidate = Definition ? Definition : D->getMostRecentDecl();
+ return Candidate->isTemplateInstantiation()
+ ? Candidate->getPrimaryTemplate()->getTemplatedDecl()
+ : Candidate;
+}
+
+// TODO(kirillbobyrev): Replace clang-rename's USRFindingAction with this
+// routine to get a better test coverage and reduce technical debt.
+// TODO(kirillbobyrev): Write documentation.
+const Decl *getRenameRootDecl(const Decl *D) {
+ auto *Candidate = D;
+ // TODO(kirillbobyrev): Should this only happen once?
+ if (const auto *RD = llvm::dyn_cast<RecordDecl>(D)) {
+ const auto *Definition = RD->getDefinition();
+ Candidate = Definition ? Definition : RD->getMostRecentDecl();
+ }
+ if (const auto *Template = llvm::dyn_cast<TemplateDecl>(Candidate)) {
+ return getRenameRootDecl(Template);
+ } else if (const auto *ClassTemplateSpecialization =
+ llvm::dyn_cast<ClassTemplateSpecializationDecl>(Candidate)) {
+ return getRenameRootDecl(ClassTemplateSpecialization);
+ } else if (const auto *Constructor =
+ llvm::dyn_cast<CXXConstructorDecl>(Candidate)) {
+ return getRenameRootDecl(Constructor->getParent());
+ } else if (const auto *Destructor =
+ llvm::dyn_cast<CXXDestructorDecl>(Candidate)) {
+ return getRenameRootDecl(Destructor->getParent());
+ } else if (const auto *Method = llvm::dyn_cast<CXXMethodDecl>(Candidate)) {
+ return getRenameRootDecl(Method);
+ } else if (const auto *Function = llvm::dyn_cast<FunctionDecl>(Candidate)) {
+ return getRenameRootDecl(Function);
+ }
+ return Candidate;
+}
+
// Return all rename occurrences in the main file.
std::vector<SourceLocation> findOccurrencesWithinFile(ParsedAST &AST,
const NamedDecl &ND) {
@@ -217,14 +268,7 @@
// get the primary template maunally.
// getUSRsForDeclaration will find other related symbols, e.g. virtual and its
// overriddens, primary template and all explicit specializations.
- // FIXME: Get rid of the remaining tooling APIs.
- const auto RenameDecl =
- ND.getDescribedTemplate() ? ND.getDescribedTemplate() : &ND;
- std::vector<std::string> RenameUSRs =
- tooling::getUSRsForDeclaration(RenameDecl, AST.getASTContext());
- llvm::DenseSet<SymbolID> TargetIDs;
- for (auto &USR : RenameUSRs)
- TargetIDs.insert(SymbolID(USR));
+ const auto *RenameDeclRoot = getRenameRootDecl(&ND);
std::vector<SourceLocation> Results;
for (Decl *TopLevelDecl : AST.getLocalTopLevelDecls()) {
@@ -232,8 +276,7 @@
if (Ref.Targets.empty())
return;
for (const auto *Target : Ref.Targets) {
- auto ID = getSymbolID(Target);
- if (!ID || TargetIDs.find(*ID) == TargetIDs.end())
+ if (getRenameRootDecl(Target) != RenameDeclRoot)
return;
}
Results.push_back(Ref.NameLoc);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
