[clangd] Add line and column number to the index symbol.

Summary:
LSP is using Line & column as symbol position, clangd needs to transfer file
offset to Line & column when sending results back to LSP client, which is a high
cost, especially for finding workspace symbol -- we have to read the file
content from disk (if it isn't loaded in memory).

Saving these information in the index will make the clangd life eaiser.

Reviewers: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits

Differential Revision: https://reviews.llvm.org/D45513

llvm-svn: 329997

diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp
index 859f638..c4e969c 100644 (file)
--- a/clang-tools-extra/clangd/index/Index.cpp
+++ b/clang-tools-extra/clangd/index/Index.cpp
@@ -19,7 +19,8 @@ using namespace llvm;
 raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
   if (!L)
     return OS << "(none)";
-  return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << ")";
+  return OS << L.FileURI << "[" << L.Start.Line << ":" << L.Start.Column << "-"
+            << L.End.Line << ":" << L.End.Column << ")";
 }
 
 SymbolID::SymbolID(StringRef USR)

diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h
index 42f23ce..cb32c0f 100644 (file)
--- a/clang-tools-extra/clangd/index/Index.h
+++ b/clang-tools-extra/clangd/index/Index.h
@@ -24,13 +24,27 @@ namespace clang {
 namespace clangd {
 
 struct SymbolLocation {
+  // Specify a position (Line, Column) of symbol. Using Line/Column allows us to
+  // build LSP responses without reading the file content.
+  struct Position {
+    uint32_t Line = 0; // 0-based
+    // Using UTF-16 code units.
+    uint32_t Column = 0; // 0-based
+  };
+
   // The URI of the source file where a symbol occurs.
   llvm::StringRef FileURI;
   // The 0-based offsets of the symbol from the beginning of the source file,
   // using half-open range, [StartOffset, EndOffset).
+  // DO NOT use these fields, as they will be removed immediately.
+  // FIXME(hokein): remove these fields in favor of Position.
   unsigned StartOffset = 0;
   unsigned EndOffset = 0;
 
+  /// The symbol range, using half-open range [Start, End).
+  Position Start;
+  Position End;
+
   operator bool() const { return !FileURI.empty(); }
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);

diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 9a7649b..6e7209d 100644 (file)
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -192,9 +192,24 @@ llvm::Optional<SymbolLocation> getSymbolLocation(
   FileURIStorage = std::move(*U);
   SymbolLocation Result;
   Result.FileURI = FileURIStorage;
-  Result.StartOffset = SM.getFileOffset(NameLoc);
-  Result.EndOffset = Result.StartOffset + clang::Lexer::MeasureTokenLength(
-                                              NameLoc, SM, LangOpts);
+  auto TokenLength = clang::Lexer::MeasureTokenLength(NameLoc, SM, LangOpts);
+
+  auto CreatePosition = [&SM](SourceLocation Loc) {
+    auto FileIdAndOffset = SM.getDecomposedLoc(Loc);
+    auto FileId = FileIdAndOffset.first;
+    auto Offset = FileIdAndOffset.second;
+    SymbolLocation::Position Pos;
+    // Position is 0-based while SourceManager is 1-based.
+    Pos.Line = SM.getLineNumber(FileId, Offset) - 1;
+    // FIXME: Use UTF-16 code units, not UTF-8 bytes.
+    Pos.Column = SM.getColumnNumber(FileId, Offset) - 1;
+    return Pos;
+  };
+
+  Result.Start = CreatePosition(NameLoc);
+  auto EndLoc = NameLoc.getLocWithOffset(TokenLength);
+  Result.End = CreatePosition(EndLoc);
+
   return std::move(Result);
 }
 

diff --git a/clang-tools-extra/clangd/index/SymbolYAML.cpp b/clang-tools-extra/clangd/index/SymbolYAML.cpp
index 307b0af..26b8886 100644 (file)
--- a/clang-tools-extra/clangd/index/SymbolYAML.cpp
+++ b/clang-tools-extra/clangd/index/SymbolYAML.cpp
@@ -43,11 +43,18 @@ struct NormalizedSymbolID {
   std::string HexString;
 };
 
+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<SymbolLocation> {
   static void mapping(IO &IO, SymbolLocation &Value) {
-    IO.mapRequired("StartOffset", Value.StartOffset);
-    IO.mapRequired("EndOffset", Value.EndOffset);
     IO.mapRequired("FileURI", Value.FileURI);
+    IO.mapRequired("Start", Value.Start);
+    IO.mapRequired("End", Value.End);
   }
 };
 

diff --git a/clang-tools-extra/unittests/clangd/Annotations.cpp b/clang-tools-extra/unittests/clangd/Annotations.cpp
index fc9c4cb..6bd81c8 100644 (file)
--- a/clang-tools-extra/unittests/clangd/Annotations.cpp
+++ b/clang-tools-extra/unittests/clangd/Annotations.cpp
@@ -83,15 +83,5 @@ std::vector<Range> Annotations::ranges(llvm::StringRef Name) const {
   return {R.begin(), R.end()};
 }
 
-std::pair<std::size_t, std::size_t>
-Annotations::offsetRange(llvm::StringRef Name) const {
-  auto R = range(Name);
-  llvm::Expected<size_t> Start = positionToOffset(Code, R.start);
-  llvm::Expected<size_t> End = positionToOffset(Code, R.end);
-  assert(Start);
-  assert(End);
-  return {*Start, *End};
-}
-
 } // namespace clangd
 } // namespace clang

diff --git a/clang-tools-extra/unittests/clangd/Annotations.h b/clang-tools-extra/unittests/clangd/Annotations.h
index 799f078..549a21e 100644 (file)
--- a/clang-tools-extra/unittests/clangd/Annotations.h
+++ b/clang-tools-extra/unittests/clangd/Annotations.h
@@ -58,10 +58,6 @@ public:
   // Returns the location of all ranges marked by [[ ]] (or $name[[ ]]).
   std::vector<Range> ranges(llvm::StringRef Name = "") const;
 
-  // The same to `range` method, but returns range in offsets [start, end).
-  std::pair<std::size_t, std::size_t>
-  offsetRange(llvm::StringRef Name = "") const;
-
 private:
   std::string Code;
   llvm::StringMap<llvm::SmallVector<Position, 1>> Points;

diff --git a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
index 9e23ca4..9258b97 100644 (file)
--- a/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -51,13 +51,20 @@ MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
 MATCHER_P(IncludeHeader, P, "") {
   return arg.Detail && arg.Detail->IncludeHeader == P;
 }
-MATCHER_P(DeclRange, Offsets, "") {
-  return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
-      arg.CanonicalDeclaration.EndOffset == Offsets.second;
-}
-MATCHER_P(DefRange, Offsets, "") {
-  return arg.Definition.StartOffset == Offsets.first &&
-         arg.Definition.EndOffset == Offsets.second;
+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);
+}
+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);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
 
@@ -209,7 +216,7 @@ TEST_F(SymbolCollectorTest, Template) {
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
   EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
-                           QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+                           QName("Tmpl"), DeclRange(Header.range()))}));
 }
 
 TEST_F(SymbolCollectorTest, Locations) {
@@ -221,6 +228,9 @@ TEST_F(SymbolCollectorTest, Locations) {
 
     // Declared in header, defined nowhere.
     extern int $zdecl[[Z]];
+
+    void $foodecl[[fo\
+o]]();
   )cpp");
   Annotations Main(R"cpp(
     int $xdef[[X]] = 42;
@@ -234,13 +244,15 @@ TEST_F(SymbolCollectorTest, Locations) {
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
-          AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
-                DefRange(Main.offsetRange("xdef"))),
-          AllOf(QName("Cls"), DeclRange(Header.offsetRange("clsdecl")),
-                DefRange(Main.offsetRange("clsdef"))),
-          AllOf(QName("print"), DeclRange(Header.offsetRange("printdecl")),
-                DefRange(Main.offsetRange("printdef"))),
-          AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl")))));
+          AllOf(QName("X"), DeclRange(Header.range("xdecl")),
+                DefRange(Main.range("xdef"))),
+          AllOf(QName("Cls"), DeclRange(Header.range("clsdecl")),
+                DefRange(Main.range("clsdef"))),
+          AllOf(QName("print"), DeclRange(Header.range("printdecl")),
+                DefRange(Main.range("printdef"))),
+          AllOf(QName("Z"), DeclRange(Header.range("zdecl"))),
+          AllOf(QName("foo"), DeclRange(Header.range("foodecl")))
+          ));
 }
 
 TEST_F(SymbolCollectorTest, References) {
@@ -365,9 +377,9 @@ TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) {
   EXPECT_THAT(
       Symbols,
       UnorderedElementsAre(
-          AllOf(QName("abc_Test"), DeclRange(Header.offsetRange("expansion")),
+          AllOf(QName("abc_Test"), DeclRange(Header.range("expansion")),
                 DeclURI(TestHeaderURI)),
-          AllOf(QName("Test"), DeclRange(Header.offsetRange("spelling")),
+          AllOf(QName("Test"), DeclRange(Header.range("spelling")),
                 DeclURI(TestHeaderURI))));
 }
 
@@ -382,7 +394,8 @@ TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
                      /*ExtraArgs=*/{"-DNAME=name"});
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(AllOf(
-                  QName("name"), DeclRange(Header.offsetRange("expansion")),
+                  QName("name"),
+                  DeclRange(Header.range("expansion")),
                   DeclURI(TestHeaderURI))));
 }
 
@@ -511,9 +524,13 @@ SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
-  StartOffset:     0
-  EndOffset:       1
   FileURI:        file:///path/foo.h
+  Start:
+    Line: 1
+    Column: 0
+  End:
+    Line: 1
+    Column: 1
 CompletionLabel:    'Foo1-label'
 CompletionFilterText:    'filter'
 CompletionPlainInsertText:    'plain'
@@ -531,9 +548,13 @@ SymInfo:
   Kind:            Function
   Lang:            Cpp
 CanonicalDeclaration:
-  StartOffset:     10
-  EndOffset:       12
   FileURI:        file:///path/bar.h
+  Start:
+    Line: 1
+    Column: 0
+  End:
+    Line: 1
+    Column: 1
 CompletionLabel:    'Foo2-label'
 CompletionFilterText:    'filter'
 CompletionPlainInsertText:    'plain'
@@ -542,6 +563,7 @@ CompletionSnippetInsertText:    'snippet'
 )";
 
   auto Symbols1 = SymbolsFromYAML(YAML1);
+
   EXPECT_THAT(Symbols1,
               UnorderedElementsAre(AllOf(
                   QName("clang::Foo1"), Labeled("Foo1-label"), Doc("Foo doc"),
@@ -646,17 +668,17 @@ TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
   EXPECT_THAT(Symbols,
               UnorderedElementsAre(
                   AllOf(QName("C"), DeclURI(TestHeaderURI),
-                        DeclRange(Header.offsetRange("cdecl")),
+                        DeclRange(Header.range("cdecl")),
                         IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI),
-                        DefRange(Header.offsetRange("cdecl"))),
+                        DefRange(Header.range("cdecl"))),
                   AllOf(QName("S"), DeclURI(TestHeaderURI),
-                        DeclRange(Header.offsetRange("sdecl")),
+                        DeclRange(Header.range("sdecl")),
                         IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI),
-                        DefRange(Header.offsetRange("sdecl"))),
+                        DefRange(Header.range("sdecl"))),
                   AllOf(QName("U"), DeclURI(TestHeaderURI),
-                        DeclRange(Header.offsetRange("udecl")),
+                        DeclRange(Header.range("udecl")),
                         IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI),
-                        DefRange(Header.offsetRange("udecl")))));
+                        DefRange(Header.range("udecl")))));
 }
 
 TEST_F(SymbolCollectorTest, ClassForwardDeclarationIsCanonical) {