[clang-tools-extra] 9ad1099 - [clangd] No need to query ctor refs in cross-file rename.

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 2 06:24:09 PST 2020


Author: Haojian Wu
Date: 2020-03-02T15:21:32+01:00
New Revision: 9ad109922450ce3519a5431c72cdc9d4c20a18bf

URL: https://github.com/llvm/llvm-project/commit/9ad109922450ce3519a5431c72cdc9d4c20a18bf
DIFF: https://github.com/llvm/llvm-project/commit/9ad109922450ce3519a5431c72cdc9d4c20a18bf.diff

LOG: [clangd] No need to query ctor refs in cross-file rename.

Summary:
This patch reverts https://github.com/llvm/llvm-project/commit/2c5ee78de113484978450b834498e1b0e2aab5c4,
now kythe (https://github.com/kythe/kythe/issues/4381) supports returning ctors refs as part of class references, so
there is no need to query the ctor refs in the index (this would also
make the results worse, lots of duplications)

Reviewers: sammccall

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

Tags: #clang

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

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 16dfef316a27..54112e09f0f9 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -295,23 +295,6 @@ 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 {};
-    for (const CXXConstructorDecl *Ctor : RD->ctors())
-      Ctors.push_back(Ctor);
-    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>>>
@@ -321,14 +304,6 @@ findOccurrencesOutsideFile(const NamedDecl &RenameDecl,
   trace::Span Tracer("FindOccurrencesOutsideFile");
   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 9906d6b3c29d..55d78a82ab56 100644
--- a/clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ b/clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -798,53 +798,6 @@ 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");


        


More information about the cfe-commits mailing list