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

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ad109922450: [clangd] No need to query ctor refs in 
cross-file rename. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75439/new/

https://reviews.llvm.org/D75439

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
@@ -798,53 +798,6 @@
   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 ,
-  llvm::function_ref Callback) const override {
-  *Out = Req;
-  return false;
-}
-
-bool fuzzyFind(const FuzzyFindRequest &,
-   llvm::function_ref) const override {
-  return false;
-}
-void lookup(const LookupRequest &,
-llvm::function_ref) const override {}
-
-void relations(const RelationsRequest &,
-   llvm::function_ref)
-const override {}
-size_t estimateMemoryUsage() const override { return 0; }
-
-RefsRequest *Out;
-  } RIndex();
-  auto Results = rename({MainCode.point(),
- "NewName",
- AST,
- testPath("main.cc"),
- ,
- {/*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
@@ -295,23 +295,6 @@
   return R;
 }
 
-std::vector getConstructors(const NamedDecl *ND) {
-  std::vector Ctors;
-  if (const auto *RD = dyn_cast(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(D))
-if (const auto *Ctor =
-dyn_cast(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>>
@@ -321,14 +304,6 @@
   trace::Span Tracer("FindOccurrencesOutsideFile");
   RefsRequest RQuest;
   RQuest.IDs.insert(*getSymbolID());
-  // 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())
-RQuest.IDs.insert(*getSymbolID(Ctor));
 
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap> AffectedFiles;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks for following up!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75439/new/

https://reviews.llvm.org/D75439



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

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)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75439

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
@@ -798,53 +798,6 @@
   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 ,
-  llvm::function_ref Callback) const override {
-  *Out = Req;
-  return false;
-}
-
-bool fuzzyFind(const FuzzyFindRequest &,
-   llvm::function_ref) const override {
-  return false;
-}
-void lookup(const LookupRequest &,
-llvm::function_ref) const override {}
-
-void relations(const RelationsRequest &,
-   llvm::function_ref)
-const override {}
-size_t estimateMemoryUsage() const override { return 0; }
-
-RefsRequest *Out;
-  } RIndex();
-  auto Results = rename({MainCode.point(),
- "NewName",
- AST,
- testPath("main.cc"),
- ,
- {/*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
@@ -295,23 +295,6 @@
   return R;
 }
 
-std::vector getConstructors(const NamedDecl *ND) {
-  std::vector Ctors;
-  if (const auto *RD = dyn_cast(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(D))
-if (const auto *Ctor =
-dyn_cast(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>>
@@ -321,14 +304,6 @@
   trace::Span Tracer("FindOccurrencesOutsideFile");
   RefsRequest RQuest;
   RQuest.IDs.insert(*getSymbolID());
-  // 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())
-RQuest.IDs.insert(*getSymbolID(Ctor));
 
   // Absolute file path => rename occurrences in that file.
   llvm::StringMap> AffectedFiles;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits