[clangd] Query constructors in the index during rename.

[hokein]

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.

[kbobyrev]

clang-tools-extra/clangd/refactor/Rename.cpp

326

What about desctructors? Should they always be handled separately?

clang-tools-extra/clangd/unittests/RenameTests.cpp

772

nit: I think most variables are not being modified here, so maybe const everywhere?

[hokein]

address comments.

clang-tools-extra/clangd/refactor/Rename.cpp

326

destructors are fine, there is a type loc of Foo class in ~[[Foo]], so query the class id can find the reference.

[kbobyrev]

Thanks!

[thakis]

Doesn't build on Mac: http://45.33.8.238/mac/7949/step_4.txt

[hokein]

In D74411#1873996, @thakis wrote:

Doesn't build on Mac: http://45.33.8.238/mac/7949/step_4.txt

sorry, it should be fixed in 9f63255a742d7ec542f94e0265eac5147213d9f8.

clang-tools-extra/clangd/refactor/Rename.cpp

Range toRange(const SymbolLocation &L) {
   Range R;
   R.start.line = L.Start.line();
   R.start.character = L.Start.column();
   R.end.line = L.End.line();
   R.end.character = L.End.column();
   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>>>
 findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
                            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;
   // FIXME: Make the limit customizable.
   static constexpr size_t MaxLimitFiles = 50;
   bool HasMore = Index.refs(RQuest, [&](const Ref &R) {
     if (AffectedFiles.size() > MaxLimitFiles)
       return;

clang-tools-extra/clangd/unittests/RenameTests.cpp

TEST(CrossFileRenameTests, DirtyBuffer) {
   } PIndex;
   Results = rename({MainCode.point(), NewName, AST, MainFilePath, &PIndex,
                     /*CrossFile=*/true, GetDirtyBuffer});
   EXPECT_FALSE(Results);
   EXPECT_THAT(llvm::toString(Results.takeError()),
               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");
   auto BarCode = Annotations("int [[x]];");
   auto BarPath = testPath("bar.cc");
   auto TU = TestTU::withCode(MainCode.code());
   // Set a file "bar.cc" on disk.
   TU.AdditionalFiles["bar.cc"] = std::string(BarCode.code());