[PATCH] D70441: [clangd] Speed up when building rename edit.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/SourceCode.h:54
+/// Use of UTF-16 may be overridden by kCurrentOffsetEncoding.
+llvm::Expected<size_t> byteOffset(StringRef LineCode, int LSPCharacter);
+
----------------
How this is different from `positionToOffset` with `Pos.line = 0` and `Pos.column = LSPCharacter`?
Why do we need an extra function?


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:441
 
+llvm::Expected<Edit> buildRenameEdit(llvm::StringRef InitialCode,
+                                     const std::vector<Range> &Occurrences,
----------------
We are aiming to convert all ranges in `O(n)` instead of `O(n^2)`, right?
I think this could be simpler and also have guaranteed performance even for long lines if we do it slightly differently. WDYT?

```
assert(llvm::is_sorted(Occurences));
// These two always correspond to the same position.
Position LastPos = Position{0, 0};
size_t LastOffset = 0;

std::vector<pair<size_t, size_t>> Offsets;
for (auto R : Occurences) {
  Position ShiftedStart = R.start - LastPos;
  size_t ShiftedStartOffset = positionToOffset(ShiftedStart, Code.substr(LastOffset);

  LastPos = ShiftedStart;
  LastOffset  = ShiftedStartOffset;
  // Do the same for the end position.
  // ...
}

// Now we can easily replacements.
// ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70441