hokein created this revision.
hokein added a reviewer: kbobyrev.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Though this is not needed when using clangd's own index, other indexes
(e.g. kythe) need it, as classes and their constructors are different
symbols, otherwise we will miss renaming constructors.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74411
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
@@ -768,6 +768,50 @@
testing::HasSubstr("too many occurrences"));
}
+TEST(CrossFileRename, QueryCtorInIndex) {
+ auto MainCode = Annotations("F^oo f;");
+ auto TU = TestTU::withCode(MainCode.code());
+ TU.HeaderCode = R"cpp(
+ class Foo {
+ public:
+ Foo() = default;
+ };
+ )cpp";
+ auto AST = TU.build();
+
+ RefsRequest Req;
+ class RecordIndex : public SymbolIndex {
+ public:
+ RecordIndex(RefsRequest *R) : Out(R) {}
+ bool refs(const RefsRequest &Req,
+ llvm::function_ref<void(const Ref &)> Callback) const override {
+ *Out = Req;
+ return false;
+ }
+
+ bool fuzzyFind(const FuzzyFindRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override {
+ return false;
+ }
+ void lookup(const LookupRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override {}
+
+ void relations(const RelationsRequest &,
+ llvm::function_ref<void(const SymbolID &, const Symbol &)>)
+ const override {}
+ size_t estimateMemoryUsage() const override { return 0; }
+
+ RefsRequest *Out;
+ } RIndex(&Req);
+ auto Results =
+ rename({MainCode.point(), "NewName", AST, testPath("main.cc"), &RIndex,
+ /*CrossFile=*/true});
+ ASSERT_TRUE(bool(Results)) << Results.takeError();
+ auto HeaderSymbols = TU.headerSymbols();
+ EXPECT_THAT(Req.IDs,
+ testing::Contains(findSymbol(HeaderSymbols, "Foo::Foo").ID));
+}
+
TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
auto MainCode = Annotations("int [[^x]] = 2;");
auto MainFilePath = testPath("main.cc");
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -297,6 +297,22 @@
return R;
}
+std::vector<const CXXConstructorDecl *> getConstructors(const NamedDecl *ND) {
+ std::vector<const CXXConstructorDecl *> Ctors;
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
+ if (!RD->hasUserDeclaredConstructor())
+ return {};
+ Ctors = {RD->ctors().begin(), RD->ctors().end()};
+ for (const auto *D : RD->decls()) {
+ if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+ if (const auto *Ctor =
+ dyn_cast<CXXConstructorDecl>(FTD->getTemplatedDecl()))
+ Ctors.push_back(Ctor);
+ }
+ }
+ return Ctors;
+}
+
// Return all rename occurrences (using the index) outside of the main file,
// grouped by the absolute file path.
llvm::Expected<llvm::StringMap<std::vector<Range>>>
@@ -304,6 +320,14 @@
llvm::StringRef MainFile, const SymbolIndex &Index)
{
RefsRequest RQuest;
RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+ // Classes and their constructors are different symbols, and have different
+ // symbol ID.
+ // When querying references for a class, clangd's own index will also return
+ // references of the corresponding class constructors, but this is not true
+ // for all index backends, e.g. kythe, so we add all constructors to the
query
+ // request.
+ for (const auto *Ctor : getConstructors(&RenameDecl))
+ RQuest.IDs.insert(*getSymbolID(Ctor));
// Absolute file path => rename occurrences in that file.
llvm::StringMap<std::vector<Range>> AffectedFiles;
Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -768,6 +768,50 @@
testing::HasSubstr("too many occurrences"));
}
+TEST(CrossFileRename, QueryCtorInIndex) {
+ auto MainCode = Annotations("F^oo f;");
+ auto TU = TestTU::withCode(MainCode.code());
+ TU.HeaderCode = R"cpp(
+ class Foo {
+ public:
+ Foo() = default;
+ };
+ )cpp";
+ auto AST = TU.build();
+
+ RefsRequest Req;
+ class RecordIndex : public SymbolIndex {
+ public:
+ RecordIndex(RefsRequest *R) : Out(R) {}
+ bool refs(const RefsRequest &Req,
+ llvm::function_ref<void(const Ref &)> Callback) const override {
+ *Out = Req;
+ return false;
+ }
+
+ bool fuzzyFind(const FuzzyFindRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override {
+ return false;
+ }
+ void lookup(const LookupRequest &,
+ llvm::function_ref<void(const Symbol &)>) const override {}
+
+ void relations(const RelationsRequest &,
+ llvm::function_ref<void(const SymbolID &, const Symbol &)>)
+ const override {}
+ size_t estimateMemoryUsage() const override { return 0; }
+
+ RefsRequest *Out;
+ } RIndex(&Req);
+ auto Results =
+ rename({MainCode.point(), "NewName", AST, testPath("main.cc"), &RIndex,
+ /*CrossFile=*/true});
+ ASSERT_TRUE(bool(Results)) << Results.takeError();
+ auto HeaderSymbols = TU.headerSymbols();
+ EXPECT_THAT(Req.IDs,
+ testing::Contains(findSymbol(HeaderSymbols, "Foo::Foo").ID));
+}
+
TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
auto MainCode = Annotations("int [[^x]] = 2;");
auto MainFilePath = testPath("main.cc");
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -297,6 +297,22 @@
return R;
}
+std::vector<const CXXConstructorDecl *> getConstructors(const NamedDecl *ND) {
+ std::vector<const CXXConstructorDecl *> Ctors;
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(ND)) {
+ if (!RD->hasUserDeclaredConstructor())
+ return {};
+ Ctors = {RD->ctors().begin(), RD->ctors().end()};
+ for (const auto *D : RD->decls()) {
+ if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(D))
+ if (const auto *Ctor =
+ dyn_cast<CXXConstructorDecl>(FTD->getTemplatedDecl()))
+ Ctors.push_back(Ctor);
+ }
+ }
+ return Ctors;
+}
+
// Return all rename occurrences (using the index) outside of the main file,
// grouped by the absolute file path.
llvm::Expected<llvm::StringMap<std::vector<Range>>>
@@ -304,6 +320,14 @@
llvm::StringRef MainFile, const SymbolIndex &Index) {
RefsRequest RQuest;
RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+ // Classes and their constructors are different symbols, and have different
+ // symbol ID.
+ // When querying references for a class, clangd's own index will also return
+ // references of the corresponding class constructors, but this is not true
+ // for all index backends, e.g. kythe, so we add all constructors to the query
+ // request.
+ for (const auto *Ctor : getConstructors(&RenameDecl))
+ RQuest.IDs.insert(*getSymbolID(Ctor));
// Absolute file path => rename occurrences in that file.
llvm::StringMap<std::vector<Range>> AffectedFiles;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
