diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp --- a/clang-tools-extra/clangd/index/remote/Client.cpp +++ b/clang-tools-extra/clangd/index/remote/Client.cpp @@ -46,8 +46,7 @@ } auto Sym = fromProtobuf(Reply.stream_result(), &Strings); if (!Sym) - elog("Received invalid {0}: {1}", ReplyT::descriptor()->name(), - Reply.stream_result().yaml_serialization()); + elog("Received invalid {0}", ReplyT::descriptor()->name()); Callback(*Sym); } SPAN_ATTACH(Tracer, "status", Reader->Finish().ok()); diff --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto --- a/clang-tools-extra/clangd/index/remote/Index.proto +++ b/clang-tools-extra/clangd/index/remote/Index.proto @@ -10,6 +10,8 @@ package clang.clangd.remote; +// Semantics of SymbolIndex match clangd::SymbolIndex with all required +// structures corresponding to their clangd::* counterparts. service SymbolIndex { rpc Lookup(LookupRequest) returns (stream LookupReply) {} @@ -34,7 +36,7 @@ repeated string scopes = 2; bool any_scope = 3; uint32 limit = 4; - bool resricted_for_code_completion = 5; + bool restricted_for_code_completion = 5; repeated string proximity_paths = 6; repeated string preferred_types = 7; } @@ -63,7 +65,49 @@ } } -// FIXME(kirillbobyrev): Properly serialize symbols and refs instead of passing -// YAML. -message Ref { string yaml_serialization = 1; } -message Symbol { string yaml_serialization = 1; } +message Symbol { + string id = 1; + SymbolInfo info = 2; + string name = 3; + SymbolLocation definition = 4; + string scope = 5; + SymbolLocation canonical_declarattion = 6; + int32 references = 7; + uint32 origin = 8; + string signature = 9; + string template_specialization_args = 10; + string completion_snippet_suffix = 11; + string documentation = 12; + string return_type = 13; + string type = 14; + repeated HeaderWithReferences headers = 15; + uint32 flags = 16; +} + +message Ref { + SymbolLocation location = 1; + uint32 kind = 2; +} + +message SymbolInfo { + uint32 kind = 1; + uint32 subkind = 2; + uint32 language = 3; + uint32 properties = 4; +} + +message SymbolLocation { + Position start = 1; + Position end = 2; + string file_uri = 3; +} + +message Position { + uint32 line = 1; + uint32 column = 2; +} + +message HeaderWithReferences { + string header = 1; + uint32 references = 2; +} diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -7,13 +7,90 @@ //===----------------------------------------------------------------------===// #include "Marshalling.h" +#include "Index.pb.h" +#include "Protocol.h" #include "index/Serialization.h" +#include "index/Symbol.h" +#include "index/SymbolID.h" +#include "index/SymbolLocation.h" +#include "index/SymbolOrigin.h" #include "support/Logger.h" +#include "clang/Index/IndexSymbol.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/Support/StringSaver.h" namespace clang { namespace clangd { namespace remote { +namespace { + +clangd::SymbolLocation::Position fromProtobuf(const Position &Message) { + clangd::SymbolLocation::Position Result; + Result.setColumn(static_cast(Message.column())); + Result.setLine(static_cast(Message.line())); + return Result; +} + +Position toProtobuf(const clangd::SymbolLocation::Position &Position) { + remote::Position Result; + Result.set_column(Position.column()); + Result.set_line(Position.line()); + return Result; +} + +clang::index::SymbolInfo fromProtobuf(const SymbolInfo &Message) { + clang::index::SymbolInfo Result; + Result.Kind = static_cast(Message.kind()); + Result.SubKind = static_cast(Message.subkind()); + Result.Lang = static_cast(Message.language()); + Result.Properties = + static_cast(Message.properties()); + return Result; +} + +SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info) { + SymbolInfo Result; + Result.set_kind(static_cast(Info.Kind)); + Result.set_subkind(static_cast(Info.SubKind)); + Result.set_language(static_cast(Info.Lang)); + Result.set_properties(static_cast(Info.Properties)); + return Result; +} + +clangd::SymbolLocation fromProtobuf(const SymbolLocation &Message, + llvm::UniqueStringSaver *Strings) { + clangd::SymbolLocation Location; + Location.Start = fromProtobuf(Message.start()); + Location.End = fromProtobuf(Message.end()); + Location.FileURI = Strings->save(Message.file_uri()).begin(); + return Location; +} + +SymbolLocation toProtobuf(const clangd::SymbolLocation &Location) { + remote::SymbolLocation Result; + *Result.mutable_start() = toProtobuf(Location.Start); + *Result.mutable_end() = toProtobuf(Location.End); + *Result.mutable_file_uri() = Location.FileURI; + return Result; +} + +HeaderWithReferences +toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) { + HeaderWithReferences Result; + Result.set_header(IncludeHeader.IncludeHeader.str()); + Result.set_references(IncludeHeader.References); + return Result; +} + +clangd::Symbol::IncludeHeaderWithReferences +fromProtobuf(const HeaderWithReferences &Message) { + return clangd::Symbol::IncludeHeaderWithReferences{Message.header(), + Message.references()}; +} + +} // namespace + clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request) { clangd::FuzzyFindRequest Result; Result.Query = Request->query(); @@ -22,7 +99,7 @@ Result.AnyScope = Request->any_scope(); if (Request->limit()) Result.Limit = Request->limit(); - Result.RestrictForCodeCompletion = Request->resricted_for_code_completion(); + Result.RestrictForCodeCompletion = Request->restricted_for_code_completion(); for (const auto &Path : Request->proximity_paths()) Result.ProximityPaths.push_back(Path); for (const auto &Type : Request->preferred_types()) @@ -32,21 +109,50 @@ llvm::Optional fromProtobuf(const Symbol &Message, llvm::UniqueStringSaver *Strings) { - auto Result = symbolFromYAML(Message.yaml_serialization(), Strings); - if (!Result) { - elog("Cannot convert Symbol from Protobuf: {}", Result.takeError()); + if (!Message.has_info() || !Message.has_definition() || + !Message.has_canonical_declarattion()) { + elog("Cannot convert Symbol from Protobuf: {}", Message.ShortDebugString()); return llvm::None; } - return *Result; + clangd::Symbol Result; + auto ID = SymbolID::fromStr(Message.id()); + if (!ID) { + elog("Cannot convert parse SymbolID {} from Protobuf: {}", ID.takeError(), + Message.ShortDebugString()); + return llvm::None; + } + Result.ID = *ID; + Result.SymInfo = fromProtobuf(Message.info()); + Result.Name = Message.name(); + Result.Scope = Message.scope(); + Result.Definition = fromProtobuf(Message.definition(), Strings); + Result.CanonicalDeclaration = + fromProtobuf(Message.canonical_declarattion(), Strings); + Result.References = Message.references(); + Result.Origin = static_cast(Message.origin()); + Result.Signature = Message.signature(); + Result.TemplateSpecializationArgs = Message.template_specialization_args(); + Result.CompletionSnippetSuffix = Message.completion_snippet_suffix(); + Result.Documentation = Message.documentation(); + Result.ReturnType = Message.return_type(); + Result.Type = Message.type(); + for (const auto &Header : Message.headers()) { + Result.IncludeHeaders.push_back(fromProtobuf(Header)); + } + Result.Flags = static_cast(Message.flags()); + return Result; } + llvm::Optional fromProtobuf(const Ref &Message, llvm::UniqueStringSaver *Strings) { - auto Result = refFromYAML(Message.yaml_serialization(), Strings); - if (!Result) { - elog("Cannot convert Ref from Protobuf: {}", Result.takeError()); + if (!Message.has_location()) { + elog("Cannot convert Ref from Protobuf: {}", Message.ShortDebugString()); return llvm::None; } - return *Result; + clangd::Ref Result; + Result.Location = fromProtobuf(Message.location(), Strings); + Result.Kind = static_cast(Message.kind()); + return Result; } LookupRequest toProtobuf(const clangd::LookupRequest &From) { @@ -64,7 +170,7 @@ RPCRequest.set_any_scope(From.AnyScope); if (From.Limit) RPCRequest.set_limit(*From.Limit); - RPCRequest.set_resricted_for_code_completion(From.RestrictForCodeCompletion); + RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion); for (const auto &Path : From.ProximityPaths) RPCRequest.add_proximity_paths(Path); for (const auto &Type : From.PreferredTypes) @@ -84,13 +190,34 @@ Symbol toProtobuf(const clangd::Symbol &From) { Symbol Result; - Result.set_yaml_serialization(toYAML(From)); + Result.set_id(From.ID.str()); + *Result.mutable_info() = toProtobuf(From.SymInfo); + Result.set_name(From.Name.str()); + *Result.mutable_definition() = toProtobuf(From.Definition); + Result.set_scope(From.Scope.str()); + *Result.mutable_canonical_declarattion() = + toProtobuf(From.CanonicalDeclaration); + Result.set_references(From.References); + Result.set_origin(static_cast(From.Origin)); + Result.set_signature(From.Signature.str()); + Result.set_template_specialization_args( + From.TemplateSpecializationArgs.str()); + Result.set_completion_snippet_suffix(From.CompletionSnippetSuffix.str()); + Result.set_documentation(From.Documentation.str()); + Result.set_return_type(From.ReturnType.str()); + Result.set_type(From.Type.str()); + for (const auto &Header : From.IncludeHeaders) { + auto *NextHeader = Result.add_headers(); + *NextHeader = toProtobuf(Header); + } + Result.set_flags(static_cast(From.Flags)); return Result; } Ref toProtobuf(const clangd::Ref &From) { Ref Result; - Result.set_yaml_serialization(toYAML(From)); + Result.set_kind(static_cast(From.Kind)); + *Result.mutable_location() = toProtobuf(From.Location); return Result; } diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -22,6 +22,12 @@ endif() endif() +if (CLANGD_ENABLE_REMOTE) + include_directories(${CMAKE_CURRENT_BINARY_DIR}/../index/remote) + add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI=1) + set(REMOTE_TEST_SOURCES remote/MarshallingTests.cpp) +endif() + add_custom_target(ClangdUnitTests) add_unittest(ClangdUnitTests ClangdTests Annotations.cpp @@ -87,6 +93,8 @@ support/TestTracer.cpp support/TraceTests.cpp + ${REMOTE_TEST_SOURCES} + $ ) @@ -116,6 +124,12 @@ LLVMTestingSupport ) +if (CLANGD_ENABLE_REMOTE) + target_link_libraries(ClangdTests + PRIVATE + clangdRemoteMarshalling) +endif() + if (CLANGD_BUILD_XPC) add_subdirectory(xpc) endif () diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h --- a/clang-tools-extra/clangd/unittests/TestTU.h +++ b/clang-tools-extra/clangd/unittests/TestTU.h @@ -70,6 +70,7 @@ ParsedAST build() const; ParseInputs inputs() const; SymbolSlab headerSymbols() const; + RefSlab headerRefs() const; std::unique_ptr index() const; }; diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp --- a/clang-tools-extra/clangd/unittests/TestTU.cpp +++ b/clang-tools-extra/clangd/unittests/TestTU.cpp @@ -113,6 +113,11 @@ AST.getCanonicalIncludes())); } +RefSlab TestTU::headerRefs() const { + auto AST = build(); + return std::get<1>(indexMainDecls(AST)); +} + std::unique_ptr TestTU::index() const { auto AST = build(); auto Idx = std::make_unique(/*UseDex=*/true); diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp @@ -0,0 +1,93 @@ +//===--- MarshallingTests.cpp ------------------------------------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "../TestTU.h" +#include "index/Serialization.h" +#include "index/remote/marshalling/Marshalling.h" +#include "llvm/Support/StringSaver.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace remote { +namespace { + +TEST(RemoteMarshallingTest, SymbolSerialization) { + const auto *Header = R"( + // This is a class. + class Foo { + public: + Foo(); + + int Bar; + private: + double Number; + }; + /// This is a function. + char baz(); + template + T getT (); + )"; + const auto TU = TestTU::withHeaderCode(Header); + const auto Symbols = TU.headerSymbols(); + // Sanity check: there are more than 5 symbols available. + EXPECT_GE(Symbols.size(), 5UL); + llvm::BumpPtrAllocator Arena; + llvm::UniqueStringSaver Strings(Arena); + for (auto &Sym : Symbols) { + const auto ProtobufMeessage = toProtobuf(Sym); + const auto SymToProtobufAndBack = fromProtobuf(ProtobufMeessage, &Strings); + EXPECT_TRUE(SymToProtobufAndBack.hasValue()); + EXPECT_EQ(toYAML(Sym), toYAML(*SymToProtobufAndBack)); + } +} + +TEST(RemoteMarshallingTest, ReferenceSerialization) { + TestTU TU; + TU.HeaderCode = R"( + int foo(); + int GlobalVariable = 42; + class Foo { + public: + Foo(); + + char Symbol = 'S'; + }; + template + T getT() { return T(); } + )"; + TU.Code = R"( + int foo() { + ++GlobalVariable; + + Foo foo = Foo(); + if (foo.Symbol - 'a' == 42) { + foo.Symbol = 'b'; + } + + const auto bar = getT(); + } + )"; + const auto References = TU.headerRefs(); + llvm::BumpPtrAllocator Arena; + llvm::UniqueStringSaver Strings(Arena); + // Sanity check: there are more than 5 references available. + EXPECT_GE(References.numRefs(), 5UL); + for (const auto &SymbolWithRefs : References) { + for (const auto &Ref : SymbolWithRefs.second) { + const auto RefToProtobufAndBack = fromProtobuf(toProtobuf(Ref), &Strings); + EXPECT_TRUE(RefToProtobufAndBack.hasValue()); + EXPECT_EQ(toYAML(Ref), toYAML(*RefToProtobufAndBack)); + } + } +} // namespace + +} // namespace +} // namespace remote +} // namespace clangd +} // namespace clang