[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-29 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX closed this revision. ArcsinX added a comment. Don't know why this didn't close automatically Commit: https://reviews.llvm.org/rGd8ba6b4ab3eceb6bbcdf4371d4ffaab9d1a5cebe Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://revie

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-29 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 294924. ArcsinX added a comment. Fix possible UB at bitwise shift. WordGain => MaxDistance. Fix LineMin and LineMax values. Fix comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://reviews.

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-29 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/XRefs.cpp:572 + // - backward: 2^(N-1) lines. + unsigned WordGain = 1U << Word.Text.size(); + // Line number for SM.translat

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-28 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:589 // but not things like disabled preprocessor sections. if (!(tokenSpelledAt(Tok.location(), TB) || TB.expansionStartingAt(&Tok))) return false; here =

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-28 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 294616. ArcsinX added a comment. Use SourceManager::translateLineCol(), code simplifications. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://reviews.llvm.org/D87891 Files: clang-tools-extra

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-24 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:580 return false; -// No point guessing the same location we started with. -if (Tok.location() == Word.Location) sammccall wrote: > ArcsinX wrote: > > sammccall wrote: >

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D87891#2291838 , @ArcsinX wrote: > Thanks for your reply, I will rethink this patch. FWIW I think if we drop most of the math in favor of using SourceManager's line translation, this doesn't add much complexity and is probab

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-24 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. In D87891#2291760 , @kadircet wrote: > The biggest problem I see is, this is not changing the fact that we are still > traversing the whole file: > > - You do one traversal over the whole file to find `FileLines` > - Then possibly

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-23 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:565 unsigned Line = SM.getSpellingLineNumber(Loc); if (Line > WordLine) + return 1 + Line - WordLine; Since costs are only compared, we can simplify this: return (Line

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-23 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment. Hey! Sorry for the late reply, this has been open in my tabs since day 1 just didn't get a chance to take a look at it. The biggest problem I see is, this is not changing the fact that we are still traversing the whole file: - You do one traversal over the whole file

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-23 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX added a comment. I feel like I'm doing something totally wrong here :) Could someone give me an advice? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://reviews.llvm.org/D87891 __

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-23 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 293665. ArcsinX added a comment. std::pow => bitwise shift. Take care about integers overflow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://reviews.llvm.org/D87891 Files: clang-tools-ex

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-18 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX updated this revision to Diff 292739. ArcsinX added a comment. Fix format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87891/new/ https://reviews.llvm.org/D87891 Files: clang-tools-extra/clangd/XRefs.cpp Index: clang-tools-extra/clang

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

2020-09-18 Thread Aleksandr Platonov via Phabricator via cfe-commits
ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous. Herald added a project: clang. ArcsinX requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. As @kadircet mentions in D84912#2184144