kbobyrev updated this revision to Diff 264633. kbobyrev added a comment. Rebase on top of master.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79862/new/ https://reviews.llvm.org/D79862 Files: clang-tools-extra/clangd/index/remote/Client.cpp clang-tools-extra/clangd/index/remote/Index.proto clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp clang-tools-extra/clangd/unittests/CMakeLists.txt clang-tools-extra/clangd/unittests/TestTU.cpp clang-tools-extra/clangd/unittests/TestTU.h clang-tools-extra/clangd/unittests/remote/CMakeLists.txt clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp @@ -0,0 +1,87 @@ +//===--- 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 { + +class RemoteMarshallingTest : public ::testing::Test { +public: + RemoteMarshallingTest() : Strings(NewArena) { + TU.HeaderCode = R"( + // This is a class. + class Foo { + public: + Foo(); + + int Bar; + private: + double Number; + }; + /// This is a function. + char baz(); + template <typename T> + T getT (); + struct FooBar {}; + )"; + TU.Code = R"( + const int GlobalVariable = 123; + + class ForwardDecl; + char baz() { return 'x'; } + template <typename T> + T getT() { return T(); } + Foo::Foo() : Bar(GlobalVariable), Number(42) { + for (int Counter = 0; Counter < GlobalVariable; ++Counter) { + baz(); + getT<FooBar>(); + } + } + )"; + } + + SymbolSlab symbols() { return TU.symbols(); } + RefSlab refs() { return TU.refs(); } + + llvm::UniqueStringSaver Strings; + +private: + TestTU TU; + llvm::BumpPtrAllocator NewArena; +}; + +TEST_F(RemoteMarshallingTest, SymbolSerialization) { + 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_F(RemoteMarshallingTest, ReferenceSerialization) { + for (const auto &SymbolWithRefs : refs()) { + 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 remote +} // namespace clangd +} // namespace clang Index: clang-tools-extra/clangd/unittests/remote/CMakeLists.txt =================================================================== --- /dev/null +++ clang-tools-extra/clangd/unittests/remote/CMakeLists.txt @@ -0,0 +1,26 @@ +set(LLVM_LINK_COMPONENTS + Support + ) + +get_filename_component(CLANGD_SOURCE_DIR + ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH) +include_directories( + ${CLANGD_SOURCE_DIR} + ) + +include_directories(${CMAKE_CURRENT_BINARY_DIR}/../../index/remote) +add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI=1) + +add_custom_target(ClangdRemoteUnitTests) +add_unittest(ClangdRemoteUnitTests ClangdRemoteTests + MarshallingTests.cpp + + ../TestTU.cpp + ../TestFS.cpp + ) + +target_link_libraries(ClangdRemoteTests + PRIVATE + clangDaemon + clangdRemoteMarshalling + ) Index: clang-tools-extra/clangd/unittests/TestTU.h =================================================================== --- clang-tools-extra/clangd/unittests/TestTU.h +++ clang-tools-extra/clangd/unittests/TestTU.h @@ -70,6 +70,11 @@ ParsedAST build() const; ParseInputs inputs() const; SymbolSlab headerSymbols() const; + RefSlab headerRefs() const; + // Returns all symbols from the header and main file. + SymbolSlab symbols() const; + // Returns all references from the header and main file. + RefSlab refs() const; std::unique_ptr<SymbolIndex> index() const; }; Index: clang-tools-extra/clangd/unittests/TestTU.cpp =================================================================== --- clang-tools-extra/clangd/unittests/TestTU.cpp +++ clang-tools-extra/clangd/unittests/TestTU.cpp @@ -113,6 +113,39 @@ AST.getCanonicalIncludes())); } +RefSlab TestTU::headerRefs() const { + auto AST = build(); + return std::get<1>(indexHeaderSymbols(/*Version=*/"null", AST.getASTContext(), + AST.getPreprocessorPtr(), + AST.getCanonicalIncludes())); +} + +SymbolSlab TestTU::symbols() const { + auto AST = build(); + const auto MainCodeSymbols = std::get<0>(indexMainDecls(AST)); + const auto HeaderSymbols = headerSymbols(); + SymbolSlab::Builder Merger; + for (const auto &Sym : MainCodeSymbols) + Merger.insert(Sym); + for (const auto &Sym : HeaderSymbols) + Merger.insert(Sym); + return std::move(Merger).build(); +} + +RefSlab TestTU::refs() const { + auto AST = build(); + const auto MainCodeRefs = std::get<1>(indexMainDecls(AST)); + const auto HeaderRefs = headerRefs(); + RefSlab::Builder Merger; + for (const auto &SymbolReferences : MainCodeRefs) + for (const auto &Reference : SymbolReferences.second) + Merger.insert(SymbolReferences.first, Reference); + for (const auto &SymbolReferences : HeaderRefs) + for (const auto &Reference : SymbolReferences.second) + Merger.insert(SymbolReferences.first, Reference); + return std::move(Merger).build(); +} + std::unique_ptr<SymbolIndex> TestTU::index() const { auto AST = build(); auto Idx = std::make_unique<FileIndex>(/*UseDex=*/true); Index: clang-tools-extra/clangd/unittests/CMakeLists.txt =================================================================== --- clang-tools-extra/clangd/unittests/CMakeLists.txt +++ clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -120,6 +120,10 @@ add_subdirectory(xpc) endif () +if (CLANGD_ENABLE_REMOTE) + add_subdirectory(remote) +endif() + configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py) Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp =================================================================== --- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ 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<uint32_t>(Message.column())); + Result.setLine(static_cast<uint32_t>(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<clang::index::SymbolKind>(Message.kind()); + Result.SubKind = static_cast<clang::index::SymbolSubKind>(Message.subkind()); + Result.Lang = static_cast<clang::index::SymbolLanguage>(Message.language()); + Result.Properties = + static_cast<clang::index::SymbolPropertySet>(Message.properties()); + return Result; +} + +SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info) { + SymbolInfo Result; + Result.set_kind(static_cast<uint32_t>(Info.Kind)); + Result.set_subkind(static_cast<uint32_t>(Info.SubKind)); + Result.set_language(static_cast<uint32_t>(Info.Lang)); + Result.set_properties(static_cast<uint32_t>(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<clangd::Symbol> 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<clangd::SymbolOrigin>(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<clangd::Symbol::SymbolFlag>(Message.flags()); + return Result; } + llvm::Optional<clangd::Ref> 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<clangd::RefKind>(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<uint32_t>(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<uint32_t>(From.Flags)); return Result; } Ref toProtobuf(const clangd::Ref &From) { Ref Result; - Result.set_yaml_serialization(toYAML(From)); + Result.set_kind(static_cast<uint32_t>(From.Kind)); + *Result.mutable_location() = toProtobuf(From.Location); return Result; } Index: clang-tools-extra/clangd/index/remote/Index.proto =================================================================== --- clang-tools-extra/clangd/index/remote/Index.proto +++ 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; +} Index: clang-tools-extra/clangd/index/remote/Client.cpp =================================================================== --- clang-tools-extra/clangd/index/remote/Client.cpp +++ 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());
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
