[clang-tools-extra] 2c5ee78 - [clangd] Query constructors in the index during rename.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-02-13T10:10:12+01:00
New Revision: 2c5ee78de113484978450b834498e1b0e2aab5c4

URL: https://github.com/llvm/llvm-project/commit/2c5ee78de113484978450b834498e1b0e2aab5c4
DIFF: https://github.com/llvm/llvm-project/commit/2c5ee78de113484978450b834498e1b0e2aab5c4.diff

LOG: [clangd] Query constructors in the index during rename.

Summary:
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.

Reviewers: kbobyrev

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74411

Added: 
    

Modified: 
    clang-tools-extra/clangd/refactor/Rename.cpp
    clang-tools-extra/clangd/unittests/RenameTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp b/clang-tools-extra/clangd/refactor/Rename.cpp
index 9946dfe65ec8..59b55624e93b 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -297,6 +297,22 @@ Range toRange(const SymbolLocation &L) {
   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 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
                            llvm::StringRef MainFile, const SymbolIndex &Index) {
   RefsRequest RQuest;
   RQuest.IDs.insert(*getSymbolID(&RenameDecl));
+  // Classes and their constructors are 
diff erent symbols, and have 
diff erent
+  // 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;

diff  --git a/clang-tools-extra/clangd/unittests/RenameTests.cpp b/clang-tools-extra/clangd/unittests/RenameTests.cpp
index 0314a6fabe42..3561ff6438f7 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -768,6 +768,50 @@ TEST(CrossFileRenameTests, DirtyBuffer) {
               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");