[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-25 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-24 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-24 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-24 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-21 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-19 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-19 Thread Haojian Wu via Phabricator via cfe-commits
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-

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-19 Thread Haojian Wu via Phabricator via cfe-commits
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/

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-18 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-18 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-18 Thread Haojian Wu via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-18 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D63426: [clangd] Narrow rename to local symbols.

2019-06-17 Thread Haojian Wu via Phabricator via cfe-commits
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