This revision was automatically updated to reflect the committed changes.
Closed by commit rL364283: [clangd] Narrow rename to local symbols. (authored
by hokein, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm
hokein updated this revision to Diff 206185.
hokein added a comment.
More cleanups.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/cla
hokein updated this revision to Diff 206184.
hokein marked 11 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
Files:
clang-tools-e
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:69
+
+// Query the index to get other file where the Decl is referenced.
+llvm::Optional getOtherRefFile(co
hokein updated this revision to Diff 205963.
hokein added a comment.
Improve the tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra
hokein added a comment.
Use the hybrid approach based on our discussion, the implementation should be
fine to review, I'm still working on adding more tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
hokein updated this revision to Diff 205541.
hokein added a comment.
Remove the unrelated changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-
hokein updated this revision to Diff 205540.
hokein marked an inline comment as done.
hokein added a comment.
Switch to a hybrid solution (language semantic + index) to detect "symbol
local".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
sammccall added a comment.
Thanks for exposing the ND, cleaner.
Found another problem :-( not in your code but about this whole approach to
using the index.
Let's talk tomorrow
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:81
+ Req.Limit = 100;
+ if (auto ID = get
hokein added inline comments.
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:83
+// FIXME: we may skip querying the index if D is function-local.
+const NamedDecl *D =
+clang::tooling::getNamedDeclAt(ASTCtx, SourceLocationBeg);
sammccall
hokein updated this revision to Diff 205355.
hokein marked 5 inline comments as done.
hokein added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63426/new/
https://reviews.llvm.org/D63426
Files:
clang-tools-ext
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:79
+
+ if (Index) {
+// Consult the index to determine whether the symbol is used outside of
pull out a function for this?
Comment at: clang-tools-ex
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.
Previously, we perform rename for all kinds of symbols (local, global).
This patch narrows the scope by only renaming sym
13 matches
Mail list logo