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

Reply via email to