Author: hokein Date: Fri Apr 13 01:30:39 2018 New Revision: 329997 URL: http://llvm.org/viewvc/llvm-project?rev=329997&view=rev Log: [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 Modified: clang-tools-extra/trunk/clangd/index/Index.cpp clang-tools-extra/trunk/clangd/index/Index.h clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp clang-tools-extra/trunk/unittests/clangd/Annotations.cpp clang-tools-extra/trunk/unittests/clangd/Annotations.h clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/index/Index.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Index.cpp (original) +++ clang-tools-extra/trunk/clangd/index/Index.cpp Fri Apr 13 01:30:39 2018 @@ -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) Modified: clang-tools-extra/trunk/clangd/index/Index.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h (original) +++ clang-tools-extra/trunk/clangd/index/Index.h Fri Apr 13 01:30:39 2018 @@ -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 &); Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Apr 13 01:30:39 2018 @@ -192,9 +192,24 @@ llvm::Optional<SymbolLocation> getSymbol 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); } Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Fri Apr 13 01:30:39 2018 @@ -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); } }; Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.cpp?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp Fri Apr 13 01:30:39 2018 @@ -83,15 +83,5 @@ std::vector<Range> Annotations::ranges(l 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 Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.h?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/Annotations.h (original) +++ clang-tools-extra/trunk/unittests/clangd/Annotations.h Fri Apr 13 01:30:39 2018 @@ -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; Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=329997&r1=329996&r2=329997&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Fri Apr 13 01:30:39 2018 @@ -51,13 +51,20 @@ MATCHER_P(DefURI, P, "") { return arg.De 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, SymbolFormed 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, SymbolFormed /*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, AvoidUsingFw 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) { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
