[clang-tools-extra] r345020 - Fix range length comparison in DraftStore::UpdateDraft when Unicode characters are removed from the document
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 23 04:51:53 PDT 2018
Author: sammccall
Date: Tue Oct 23 04:51:53 2018
New Revision: 345020
URL: http://llvm.org/viewvc/llvm-project?rev=345020&view=rev
Log:
Fix range length comparison in DraftStore::UpdateDraft when Unicode characters are removed from the document
Summary:
See http://lists.llvm.org/pipermail/clangd-dev/2018-October/000171.html for context.
I kept the error (instead of downgrading to a log message) since the range lengths differing does indicate either a bug in the client or server range calculation or the buffers being out of sync (which both seems serious enough to me to be an error). If any existing clients aside from VSCode break they should only break when accidentally typing a Unicode character which should only be a minor nuisance for a little while until the bug is fixed in the respective client.
Patch by Daan De Meyer!
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, ioeric, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D53527
Modified:
clang-tools-extra/trunk/clangd/DraftStore.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
Modified: clang-tools-extra/trunk/clangd/DraftStore.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DraftStore.cpp?rev=345020&r1=345019&r2=345020&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/DraftStore.cpp (original)
+++ clang-tools-extra/trunk/clangd/DraftStore.cpp Tue Oct 23 04:51:53 2018
@@ -77,8 +77,17 @@ DraftStore::updateDraft(PathRef File,
End, Start),
llvm::errc::invalid_argument);
- if (Change.rangeLength &&
- (ssize_t)(*EndIndex - *StartIndex) != *Change.rangeLength)
+ // Since the range length between two LSP positions is dependent on the
+ // contents of the buffer we compute the range length between the start and
+ // end position ourselves and compare it to the range length of the LSP
+ // message to verify the buffers of the client and server are in sync.
+
+ // EndIndex and StartIndex are in bytes, but Change.rangeLength is in UTF-16
+ // code units.
+ ssize_t ComputedRangeLength =
+ lspLength(Contents.substr(*StartIndex, *EndIndex - *StartIndex));
+
+ if (Change.rangeLength && ComputedRangeLength != *Change.rangeLength)
return make_error<StringError>(
formatv("Change's rangeLength ({0}) doesn't match the "
"computed range length ({1}).",
Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=345020&r1=345019&r2=345020&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Oct 23 04:51:53 2018
@@ -67,13 +67,12 @@ static size_t measureUTF16(StringRef U8,
return std::min(Result, U8.size());
}
-// Counts the number of UTF-16 code units needed to represent a string.
// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+size_t lspLength(StringRef Code) {
// A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
// Astral codepoints are encoded as 4 bytes in UTF-8, starting with 11110xxx.
size_t Count = 0;
- iterateCodepoints(U8, [&](int U8Len, int U16Len) {
+ iterateCodepoints(Code, [&](int U8Len, int U16Len) {
Count += U16Len;
return false;
});
@@ -123,7 +122,7 @@ Position offsetToPosition(StringRef Code
size_t StartOfLine = (PrevNL == StringRef::npos) ? 0 : (PrevNL + 1);
Position Pos;
Pos.line = Lines;
- Pos.character = utf16Len(Before.substr(StartOfLine));
+ Pos.character = lspLength(Before.substr(StartOfLine));
return Pos;
}
@@ -139,7 +138,7 @@ Position sourceLocToPosition(const Sourc
if (!Invalid) {
auto ColumnInBytes = SM.getColumnNumber(FID, Offset) - 1;
auto LineSoFar = Code.substr(Offset - ColumnInBytes, ColumnInBytes);
- P.character = utf16Len(LineSoFar);
+ P.character = lspLength(LineSoFar);
}
return P;
}
Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=345020&r1=345019&r2=345020&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Tue Oct 23 04:51:53 2018
@@ -23,6 +23,10 @@ class SourceManager;
namespace clangd {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units).
+size_t lspLength(StringRef Code);
+
/// Turn a [line, column] pair into an offset in Code.
///
/// If P.character exceeds the line length, returns the offset at end-of-line.
Modified: clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp?rev=345020&r1=345019&r2=345020&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp Tue Oct 23 04:51:53 2018
@@ -42,6 +42,16 @@ Range range(const std::pair<int, int> p1
return range;
}
+TEST(SourceCodeTests, lspLength) {
+ EXPECT_EQ(lspLength(""), 0UL);
+ EXPECT_EQ(lspLength("ascii"), 5UL);
+ // BMP
+ EXPECT_EQ(lspLength("â"), 1UL);
+ EXPECT_EQ(lspLength("Â¥"), 1UL);
+ // astral
+ EXPECT_EQ(lspLength("ð"), 2UL);
+}
+
TEST(SourceCodeTests, PositionToOffset) {
// line out of bounds
EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
More information about the cfe-commits
mailing list