[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 234727. kbobyrev marked 5 inline comments as done. kbobyrev added a comment. Addressed a bunch of comments to cleanup the patch and replied to ask for clarification of several unresolved comments. CHANGES SINCE LAST ACTION

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:591 // Details: -// - lex the draft code to get all rename candidates, this yields a superset -//of candidates. +// - lex the draft code to get all rename candidates, this yields a

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-19 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:431 +// REQUIRES: Indexed and Lexed are sorted. +// REQUIRES: Indexed and Lexed are sorted. +llvm::Optional> filterIndexResults(ArrayRef Indexed, duplication

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 234719. kbobyrev marked 7 inline comments as done. kbobyrev added a comment. Sorry for a delay: I was trying to work with range patching heuristics and get it to work in generic case of "stale index returns more results than lexer", but in the end I

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-19 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev updated this revision to Diff 234720. kbobyrev added a comment. Move helper function back to the anonymous namespace. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71598/new/ https://reviews.llvm.org/D71598 Files: clang-tools-extra/clangd/refactor/Rename.cpp

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-18 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:217 +SourceLocation positionToSourceLoc(const SourceManager , Position Pos, + StringRef Filename) { this one isn't used anywhere?

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. If we go with the solution proposed by @hokein, it looks like using the current patch is an improvement of what we have now. One big issue with the adding a new ref kind/ref modifier is that it requires modifications to Kythe-based index implementation, something

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment. (also, this should eliminate unwanted changes caused by the index being stale, wouldn't it?) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71598/new/ https://reviews.llvm.org/D71598 ___ cfe-commits mailing list

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment. In D71598#1787806 , @hokein wrote: > (apologies, the FIXME may imply this approach...) > > this approach is based on an assumption: the index results are matched to the > latest file content, but this is not always true in

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. (apologies, the FIXME may imply this approach...) this approach is based on an assumption: the index results are matched to the latest file content, but this is not always true in practice, our index maybe stale (index results came from an old snapshot of the file),

[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev created this revision. kbobyrev added reviewers: ilya-biryukov, kadircet. kbobyrev added a project: clang-tools-extra. Herald added subscribers: usaxena95, arphaman, mgrang, jkorous, MaskRay. When asked for references during cross-file rename, index might return implicit references to