hokein 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);
+
----------------
ilya-biryukov wrote:
> How this is different from `positionToOffset` with `Pos.line = 0` and
> `Pos.column = LSPCharacter`?
> Why do we need an extra function?
removed now, not needed.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:441
+llvm::Expected<Edit> buildRenameEdit(llvm::StringRef InitialCode,
+ const std::vector<Range> &Occurrences,
----------------
ilya-biryukov wrote:
> 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.
> // ...
> ```
agree, this is smart. note that we need to pay the cost of sorting, but I think
it is totally ok as the number is relatively small.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70441/new/
https://reviews.llvm.org/D70441
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
