hokein updated this revision to Diff 170001.
hokein marked 3 inline comments as done.
hokein added a comment.
Address review comments:
- handle overflowed cases, and added tests
- add getter/setters for line/column and clear all call sides
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53363
Files:
clangd/FindSymbols.cpp
clangd/XRefs.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
clangd/index/SymbolCollector.cpp
clangd/index/YAMLSerialization.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -63,19 +63,19 @@
return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
}
MATCHER_P(DeclRange, Pos, "") {
- return std::tie(arg.CanonicalDeclaration.Start.Line,
- arg.CanonicalDeclaration.Start.Column,
- arg.CanonicalDeclaration.End.Line,
- arg.CanonicalDeclaration.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(arg.CanonicalDeclaration.Start.line(),
+ arg.CanonicalDeclaration.Start.column(),
+ arg.CanonicalDeclaration.End.line(),
+ arg.CanonicalDeclaration.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(DefRange, Pos, "") {
- return std::tie(arg.Definition.Start.Line,
- arg.Definition.Start.Column, arg.Definition.End.Line,
- arg.Definition.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(
+ arg.Definition.Start.line(), arg.Definition.Start.column(),
+ arg.Definition.End.line(), arg.Definition.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
@@ -86,10 +86,10 @@
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
- return std::tie(Pos.Location.Start.Line, Pos.Location.Start.Column,
- Pos.Location.End.Line, Pos.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(Pos.Location.Start.line(), Pos.Location.Start.column(),
+ Pos.Location.End.line(), Pos.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
testing::Matcher<const std::vector<Ref> &>
HaveRanges(const std::vector<Range> Ranges) {
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -16,6 +16,7 @@
#include "index/Merge.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <cstdint>
using testing::_;
using testing::AllOf;
@@ -31,13 +32,28 @@
MATCHER_P(Named, N, "") { return arg.Name == N; }
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
+ arg.Location.End.line(), arg.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+TEST(SymbolLocation, Position) {
+ using Position = SymbolLocation::Position;
+ Position Pos;
+
+ Pos.setLine(1);
+ EXPECT_EQ(1u, Pos.line());
+ Pos.setColumn(2);
+ EXPECT_EQ(2u, Pos.column());
+
+ Pos.setLine((1 << Position::LineBits) + 1); // overflow
+ EXPECT_EQ(Pos.line(), (1u << Position::LineBits) - 1);
+ Pos.setColumn((1 << Position::ColumnBits) + 1); // overflow
+ EXPECT_EQ(Pos.column(), (1u << Position::ColumnBits) - 1);
+}
+
TEST(SymbolSlab, FindAndIterate) {
SymbolSlab::Builder B;
B.insert(symbol("Z"));
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -32,10 +32,10 @@
using testing::UnorderedElementsAre;
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
+ arg.Location.End.line(), arg.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.FileURI == U; }
Index: clangd/index/YAMLSerialization.cpp
===================================================================
--- clangd/index/YAMLSerialization.cpp
+++ clangd/index/YAMLSerialization.cpp
@@ -19,6 +19,7 @@
#include "dex/Dex.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/YAMLTraits.h"
@@ -94,18 +95,42 @@
uint8_t Origin = 0;
};
-template <> struct MappingTraits<SymbolLocation::Position> {
- static void mapping(IO &IO, SymbolLocation::Position &Value) {
- IO.mapRequired("Line", Value.Line);
- IO.mapRequired("Column", Value.Column);
+template <> struct MappingTraits<std::pair<uint32_t, uint32_t>> {
+ static void mapping(IO &IO, std::pair<uint32_t, uint32_t> &Value) {
+ IO.mapRequired("Line", Value.first);
+ IO.mapRequired("Column", Value.second);
}
};
+struct NormalizedPosition {
+ using Position = clang::clangd::SymbolLocation::Position;
+ NormalizedPosition(IO &) {}
+ NormalizedPosition(IO &, const Position &Pos) {
+ static_assert(
+ sizeof(Position) == sizeof(uint32_t),
+ "SymbolLocation::Position structure can not fit into a uint32_t.");
+ P.first = Pos.line();
+ P.second = Pos.column();
+ }
+
+ Position denormalize(IO &) {
+ Position Pos;
+ Pos.setLine(P.first);
+ Pos.setColumn(P.second);
+ return Pos;
+ }
+ std::pair<uint32_t, uint32_t> P;
+};
+
template <> struct MappingTraits<SymbolLocation> {
static void mapping(IO &IO, SymbolLocation &Value) {
IO.mapRequired("FileURI", Value.FileURI);
- IO.mapRequired("Start", Value.Start);
- IO.mapRequired("End", Value.End);
+ MappingNormalization<NormalizedPosition, SymbolLocation::Position> NStart(
+ IO, Value.Start);
+ IO.mapRequired("Start", NStart->P);
+ MappingNormalization<NormalizedPosition, SymbolLocation::Position> NEnd(
+ IO, Value.End);
+ IO.mapRequired("End", NEnd->P);
}
};
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -192,8 +192,8 @@
auto CreatePosition = [&SM](SourceLocation Loc) {
auto LSPLoc = sourceLocToPosition(SM, Loc);
SymbolLocation::Position Pos;
- Pos.Line = LSPLoc.line;
- Pos.Column = LSPLoc.character;
+ Pos.setLine(LSPLoc.line);
+ Pos.setColumn(LSPLoc.character);
return Pos;
};
Index: clangd/index/Serialization.cpp
===================================================================
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -232,17 +232,17 @@
raw_ostream &OS) {
writeVar(Strings.index(Loc.FileURI), OS);
for (const auto &Endpoint : {Loc.Start, Loc.End}) {
- writeVar(Endpoint.Line, OS);
- writeVar(Endpoint.Column, OS);
+ writeVar(Endpoint.line(), OS);
+ writeVar(Endpoint.column(), OS);
}
}
SymbolLocation readLocation(Reader &Data, ArrayRef<StringRef> Strings) {
SymbolLocation Loc;
Loc.FileURI = Data.consumeString(Strings);
for (auto *Endpoint : {&Loc.Start, &Loc.End}) {
- Endpoint->Line = Data.consumeVar();
- Endpoint->Column = Data.consumeVar();
+ Endpoint->setLine(Data.consumeVar());
+ Endpoint->setColumn(Data.consumeVar());
}
return Loc;
}
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -33,10 +33,23 @@
struct SymbolLocation {
// Specify a position (Line, Column) of symbol. Using Line/Column allows us to
// build LSP responses without reading the file content.
+ //
+ // Position is encoded into 32 bits to save space.
+ // If Line/Column is overflow, the value will be their maximum value.
struct Position {
- uint32_t Line = 0; // 0-based
+ void setLine(uint32_t Line);
+ uint32_t line() const;
+ void setColumn(uint32_t Column);
+ uint32_t column() const;
+
+ static constexpr int LineBits = 20;
+ static constexpr int ColumnBits = 12;
+
+ // Clients should use getters and setters to access these members.
+ // FIXME: hide these members.
+ uint32_t Line : LineBits; // 0-based
// Using UTF-16 code units.
- uint32_t Column = 0; // 0-based
+ uint32_t Column : ColumnBits; // 0-based
};
// The URI of the source file where a symbol occurs.
@@ -50,11 +63,13 @@
};
inline bool operator==(const SymbolLocation::Position &L,
const SymbolLocation::Position &R) {
- return std::tie(L.Line, L.Column) == std::tie(R.Line, R.Column);
+ return std::make_tuple(L.line(), L.column()) ==
+ std::make_tuple(R.Line, R.Column);
}
inline bool operator<(const SymbolLocation::Position &L,
const SymbolLocation::Position &R) {
- return std::tie(L.Line, L.Column) < std::tie(R.Line, R.Column);
+ return std::make_tuple(L.line(), L.column()) <
+ std::make_tuple(R.Line, R.Column);
}
inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
return std::tie(L.FileURI, L.Start, L.End) ==
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include "Index.h"
+#include "Logger.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -18,11 +19,30 @@
namespace clangd {
using namespace llvm;
+void SymbolLocation::Position::setLine(uint32_t L) {
+ if (L >= (1 << LineBits)) {
+ log("Set an overflowed Line {0}", L);
+ Line = (1 << LineBits) - 1;
+ return;
+ }
+ Line = L;
+}
+uint32_t SymbolLocation::Position::line() const { return Line; }
+void SymbolLocation::Position::setColumn(uint32_t Col) {
+ if (Col >= (1 << ColumnBits)) {
+ log("Set an overflowed Column {0}", Col);
+ Column = (1 << ColumnBits) - 1;
+ return;
+ }
+ Column = Col;
+}
+uint32_t SymbolLocation::Position::column() const { return Column; }
+
raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
if (!L)
return OS << "(none)";
- return OS << L.FileURI << "[" << L.Start.Line << ":" << L.Start.Column << "-"
- << L.End.Line << ":" << L.End.Column << ")";
+ return OS << L.FileURI << "[" << L.Start.line() << ":" << L.Start.column()
+ << "-" << L.End.line() << ":" << L.End.column() << ")";
}
SymbolID::SymbolID(StringRef USR)
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -57,10 +57,10 @@
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.Line;
- LSPLoc.range.start.character = Loc.Start.Column;
- LSPLoc.range.end.line = Loc.End.Line;
- LSPLoc.range.end.character = Loc.End.Column;
+ LSPLoc.range.start.line = Loc.Start.line();
+ LSPLoc.range.start.character = Loc.Start.column();
+ LSPLoc.range.end.line = Loc.End.line();
+ LSPLoc.range.end.character = Loc.End.column();
return LSPLoc;
}
Index: clangd/FindSymbols.cpp
===================================================================
--- clangd/FindSymbols.cpp
+++ clangd/FindSymbols.cpp
@@ -140,10 +140,10 @@
Location L;
L.uri = URIForFile((*Path));
Position Start, End;
- Start.line = CD.Start.Line;
- Start.character = CD.Start.Column;
- End.line = CD.End.Line;
- End.character = CD.End.Column;
+ Start.line = CD.Start.line();
+ Start.character = CD.Start.column();
+ End.line = CD.End.line();
+ End.character = CD.End.column();
L.range = {Start, End};
SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
std::string Scope = Sym.Scope;
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
