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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 20 07:08:28 PST 2019


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





More information about the cfe-commits mailing list