ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:453
+ auto Offset = [&](const Position &P) -> llvm::Expected<size_t> {
+ Position Shifted = {
+ P.line - LastPos.line,
----------------
add `assert(LastPos <= P)` to protect against malformed inputs (e.g. if one of
occurence ranges would have an endpoint that is before the starting point).
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:458
+ positionToOffset(InitialCode.substr(LastOffset), Shifted);
+ if (!ShiftedOffset) {
+ return llvm::make_error<llvm::StringError>(
----------------
NIT: braces are redundant and can be dropped.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:459
+ if (!ShiftedOffset) {
+ return llvm::make_error<llvm::StringError>(
+ llvm::formatv("fail to convert the position {0} to offset ({1})", P,
----------------
Could we simply propagate error?
I believe we mostly choose simpler code over more detailed error messages in
clangd.
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
