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

[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.translateLineCol() should be one-based, also
----------------
std::min(Word.Text.size(), numeric_limits<unsigned>::digits() - 1) to avoid UB :-(


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:572
+  // - backward: 2^(N-1) lines.
+  unsigned WordGain = 1U << Word.Text.size();
+  // Line number for SM.translateLineCol() should be one-based, also
----------------
sammccall wrote:
> std::min(Word.Text.size(), numeric_limits<unsigned>::digits() - 1) to avoid UB :-(
name WordGain is unclear to me: MaxDistance?


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:574
+  // Line number for SM.translateLineCol() should be one-based, also
+  // SM.translateLineCol() cares about line number greater than
+  // number of lines in the file.
----------------
cares about -> can handle?


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:578
+  // - LineMax = WordLine + 1 + 2^(N-1)
+  unsigned LineMin = WordLine + 1 <= WordGain ? 1 : WordLine + 1 - WordGain;
+  unsigned LineMax = WordLine + 1 + WordGain / 2;
----------------
I think this is backwards: min should divide wordgain by two, max should not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891