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

[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 >= WordLine) ? (Line - WordLine) : 2 * (WordLine - Line)


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:568
     if (Line < WordLine)
-      return 2 + llvm::Log2_64(WordLine - Line);
+      return 2 + WordLine - Line;
     return 0;
----------------
this has changed the relative ordering, if we're dropping the log then +1 should now become multiplication by two.
(Or was this intended?)


================
Comment at: clang-tools-extra/clangd/XRefs.cpp:573
   // Search bounds are based on word length: 2^N lines forward.
-  unsigned BestCost = Word.Text.size() + 1;
+  unsigned BestCost = 1 << std::min(Word.Text.size(), sizeof(unsigned) * 8 - 1);
+  auto Code = SM.getBuffer(File)->getBuffer();
----------------
The initial value of BestCost was chosen so that no-result would be worse than any cost in the eligible range, but better than any cost outside it.
If you're going to truncate the search region, you can just initialize to -1 (i.e. max).

(FWIW, I guess `sizeof(unsigned) * 8 - 1` -> `std::numeric_limits<unsigned>::digits - 1` would be more obvious)



================
Comment at: clang-tools-extra/clangd/XRefs.cpp:575
+  auto Code = SM.getBuffer(File)->getBuffer();
+  unsigned FileLines = std::count(Code.begin(), Code.end(), '\n');
+  auto TruncUnsigned = [](unsigned U) {
----------------
I think simplest is to use SourceMgr's line table.

```
int MinLine = (signed)Line - Word.Text.size()/2 , MaxLine = Line + Word.Text.size();
SourceLocation LocMin = SM.translateLineCol(File, std::max(1, MinLine), 1);
SourceLocation LocMax = SM.translateLineCol(File, MaxLine); // past-end ok
```


================
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)
----------------
why can't this happen anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891