hokein updated this revision to Diff 244345.
hokein marked 3 inline comments as done.
hokein added a comment.
address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74411/new/
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) {
+ const 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();
+ const 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) {
+ const 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();
+ const 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
