From ce63383e45f4c833dbff6a89b242bfd1d188786e Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 23 Oct 2020 15:26:24 +0200 Subject: [PATCH] [clangd] Drop version from remote index proto names, fix clangd-index-server We only need to version these messages if they actually diverge. Unlike the service, the namespace name isn't part of the wire format. clangd-index-server was broken by 81e5f298c431555d809f898c196945ca879c1150 as the namespace names weren't updated there, this fixes it (by adding them for the service, and not requiring them elsewhere). --- .../clangd/index/remote/Index.proto | 2 +- .../index/remote/marshalling/Marshalling.cpp | 73 +++++++++---------- .../index/remote/marshalling/Marshalling.h | 47 ++++++------ .../clangd/index/remote/server/Server.cpp | 2 +- .../unittests/remote/MarshallingTests.cpp | 6 +- 5 files changed, 62 insertions(+), 68 deletions(-) diff --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto index 654d1eb750f0..7619d0cb2ef3 100644 --- a/clang-tools-extra/clangd/index/remote/Index.proto +++ b/clang-tools-extra/clangd/index/remote/Index.proto @@ -8,7 +8,7 @@ syntax = "proto2"; -package clang.clangd.remote.v1; +package clang.clangd.remote; message LookupRequest { repeated string ids = 1; } diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp index 598d9434023f..296f99cdfa38 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -76,7 +76,7 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot, } llvm::Expected -Marshaller::fromProtobuf(const v1::LookupRequest *Message) { +Marshaller::fromProtobuf(const LookupRequest *Message) { clangd::LookupRequest Req; auto IDs = getIDs(Message->ids()); if (!IDs) @@ -86,7 +86,7 @@ Marshaller::fromProtobuf(const v1::LookupRequest *Message) { } llvm::Expected -Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) { +Marshaller::fromProtobuf(const FuzzyFindRequest *Message) { assert(!RemoteIndexRoot.empty()); clangd::FuzzyFindRequest Result; Result.Query = Message->query(); @@ -110,7 +110,7 @@ Marshaller::fromProtobuf(const v1::FuzzyFindRequest *Message) { } llvm::Expected -Marshaller::fromProtobuf(const v1::RefsRequest *Message) { +Marshaller::fromProtobuf(const RefsRequest *Message) { clangd::RefsRequest Req; auto IDs = getIDs(Message->ids()); if (!IDs) @@ -126,7 +126,7 @@ Marshaller::fromProtobuf(const v1::RefsRequest *Message) { } llvm::Expected -Marshaller::fromProtobuf(const v1::RelationsRequest *Message) { +Marshaller::fromProtobuf(const RelationsRequest *Message) { clangd::RelationsRequest Req; auto IDs = getIDs(Message->subjects()); if (!IDs) @@ -140,8 +140,7 @@ Marshaller::fromProtobuf(const v1::RelationsRequest *Message) { return Req; } -llvm::Expected -Marshaller::fromProtobuf(const v1::Symbol &Message) { +llvm::Expected Marshaller::fromProtobuf(const Symbol &Message) { if (!Message.has_info() || !Message.has_canonical_declaration()) return error("Missing info or declaration."); clangd::Symbol Result; @@ -179,7 +178,7 @@ Marshaller::fromProtobuf(const v1::Symbol &Message) { return Result; } -llvm::Expected Marshaller::fromProtobuf(const v1::Ref &Message) { +llvm::Expected Marshaller::fromProtobuf(const Ref &Message) { if (!Message.has_location()) return error("Missing location."); clangd::Ref Result; @@ -192,7 +191,7 @@ llvm::Expected Marshaller::fromProtobuf(const v1::Ref &Message) { } llvm::Expected> -Marshaller::fromProtobuf(const v1::Relation &Message) { +Marshaller::fromProtobuf(const Relation &Message) { auto SubjectID = SymbolID::fromStr(Message.subject_id()); if (!SubjectID) return SubjectID.takeError(); @@ -204,17 +203,16 @@ Marshaller::fromProtobuf(const v1::Relation &Message) { return std::make_pair(*SubjectID, *Object); } -v1::LookupRequest Marshaller::toProtobuf(const clangd::LookupRequest &From) { - v1::LookupRequest RPCRequest; +LookupRequest Marshaller::toProtobuf(const clangd::LookupRequest &From) { + LookupRequest RPCRequest; for (const auto &SymbolID : From.IDs) RPCRequest.add_ids(SymbolID.str()); return RPCRequest; } -v1::FuzzyFindRequest -Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) { +FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) { assert(!LocalIndexRoot.empty()); - v1::FuzzyFindRequest RPCRequest; + FuzzyFindRequest RPCRequest; RPCRequest.set_query(From.Query); for (const auto &Scope : From.Scopes) RPCRequest.add_scopes(Scope); @@ -233,8 +231,8 @@ Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) { return RPCRequest; } -v1::RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) { - v1::RefsRequest RPCRequest; +RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) { + RefsRequest RPCRequest; for (const auto &ID : From.IDs) RPCRequest.add_ids(ID.str()); RPCRequest.set_filter(static_cast(From.Filter)); @@ -243,9 +241,8 @@ v1::RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) { return RPCRequest; } -v1::RelationsRequest -Marshaller::toProtobuf(const clangd::RelationsRequest &From) { - v1::RelationsRequest RPCRequest; +RelationsRequest Marshaller::toProtobuf(const clangd::RelationsRequest &From) { + RelationsRequest RPCRequest; for (const auto &ID : From.Subjects) RPCRequest.add_subjects(ID.str()); RPCRequest.set_predicate(static_cast(From.Predicate)); @@ -254,8 +251,8 @@ Marshaller::toProtobuf(const clangd::RelationsRequest &From) { return RPCRequest; } -llvm::Expected Marshaller::toProtobuf(const clangd::Symbol &From) { - v1::Symbol Result; +llvm::Expected Marshaller::toProtobuf(const clangd::Symbol &From) { + Symbol Result; Result.set_id(From.ID.str()); *Result.mutable_info() = toProtobuf(From.SymInfo); Result.set_name(From.Name.str()); @@ -290,8 +287,8 @@ llvm::Expected Marshaller::toProtobuf(const clangd::Symbol &From) { return Result; } -llvm::Expected Marshaller::toProtobuf(const clangd::Ref &From) { - v1::Ref Result; +llvm::Expected Marshaller::toProtobuf(const clangd::Ref &From) { + Ref Result; Result.set_kind(static_cast(From.Kind)); auto Location = toProtobuf(From.Location); if (!Location) @@ -300,10 +297,9 @@ llvm::Expected Marshaller::toProtobuf(const clangd::Ref &From) { return Result; } -llvm::Expected -Marshaller::toProtobuf(const clangd::SymbolID &Subject, - const clangd::Symbol &Object) { - v1::Relation Result; +llvm::Expected Marshaller::toProtobuf(const clangd::SymbolID &Subject, + const clangd::Symbol &Object) { + Relation Result; *Result.mutable_subject_id() = Subject.str(); auto SerializedObject = toProtobuf(Object); if (!SerializedObject) @@ -346,23 +342,22 @@ llvm::Expected Marshaller::uriToRelativePath(llvm::StringRef URI) { } clangd::SymbolLocation::Position -Marshaller::fromProtobuf(const v1::Position &Message) { +Marshaller::fromProtobuf(const Position &Message) { clangd::SymbolLocation::Position Result; Result.setColumn(static_cast(Message.column())); Result.setLine(static_cast(Message.line())); return Result; } -v1::Position +Position Marshaller::toProtobuf(const clangd::SymbolLocation::Position &Position) { - remote::v1::Position Result; + remote::Position Result; Result.set_column(Position.column()); Result.set_line(Position.line()); return Result; } -clang::index::SymbolInfo -Marshaller::fromProtobuf(const v1::SymbolInfo &Message) { +clang::index::SymbolInfo Marshaller::fromProtobuf(const SymbolInfo &Message) { clang::index::SymbolInfo Result; Result.Kind = static_cast(Message.kind()); Result.SubKind = static_cast(Message.subkind()); @@ -372,8 +367,8 @@ Marshaller::fromProtobuf(const v1::SymbolInfo &Message) { return Result; } -v1::SymbolInfo Marshaller::toProtobuf(const clang::index::SymbolInfo &Info) { - v1::SymbolInfo Result; +SymbolInfo Marshaller::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)); @@ -382,7 +377,7 @@ v1::SymbolInfo Marshaller::toProtobuf(const clang::index::SymbolInfo &Info) { } llvm::Expected -Marshaller::fromProtobuf(const v1::SymbolLocation &Message) { +Marshaller::fromProtobuf(const SymbolLocation &Message) { clangd::SymbolLocation Location; auto URIString = relativePathToURI(Message.file_path()); if (!URIString) @@ -393,9 +388,9 @@ Marshaller::fromProtobuf(const v1::SymbolLocation &Message) { return Location; } -llvm::Expected +llvm::Expected Marshaller::toProtobuf(const clangd::SymbolLocation &Location) { - remote::v1::SymbolLocation Result; + remote::SymbolLocation Result; auto RelativePath = uriToRelativePath(Location.FileURI); if (!RelativePath) return RelativePath.takeError(); @@ -405,9 +400,9 @@ Marshaller::toProtobuf(const clangd::SymbolLocation &Location) { return Result; } -llvm::Expected Marshaller::toProtobuf( +llvm::Expected Marshaller::toProtobuf( const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) { - v1::HeaderWithReferences Result; + HeaderWithReferences Result; Result.set_references(IncludeHeader.References); const std::string Header = IncludeHeader.IncludeHeader.str(); if (isLiteralInclude(Header)) { @@ -422,7 +417,7 @@ llvm::Expected Marshaller::toProtobuf( } llvm::Expected -Marshaller::fromProtobuf(const v1::HeaderWithReferences &Message) { +Marshaller::fromProtobuf(const HeaderWithReferences &Message) { std::string Header = Message.header(); if (!isLiteralInclude(Header)) { auto URIString = relativePathToURI(Header); diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h index 5be00773080c..e827b4c155a2 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h @@ -38,33 +38,32 @@ public: Marshaller() = delete; Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot); - llvm::Expected fromProtobuf(const v1::Symbol &Message); - llvm::Expected fromProtobuf(const v1::Ref &Message); + llvm::Expected fromProtobuf(const Symbol &Message); + llvm::Expected fromProtobuf(const Ref &Message); llvm::Expected> - fromProtobuf(const v1::Relation &Message); + fromProtobuf(const Relation &Message); llvm::Expected - fromProtobuf(const v1::LookupRequest *Message); + fromProtobuf(const LookupRequest *Message); llvm::Expected - fromProtobuf(const v1::FuzzyFindRequest *Message); - llvm::Expected - fromProtobuf(const v1::RefsRequest *Message); + fromProtobuf(const FuzzyFindRequest *Message); + llvm::Expected fromProtobuf(const RefsRequest *Message); llvm::Expected - fromProtobuf(const v1::RelationsRequest *Message); + fromProtobuf(const RelationsRequest *Message); /// toProtobuf() functions serialize native clangd types and strip IndexRoot /// from the file paths specific to indexing machine. fromProtobuf() functions /// deserialize clangd types and translate relative paths into machine-native /// URIs. - v1::LookupRequest toProtobuf(const clangd::LookupRequest &From); - v1::FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From); - v1::RefsRequest toProtobuf(const clangd::RefsRequest &From); - v1::RelationsRequest toProtobuf(const clangd::RelationsRequest &From); + LookupRequest toProtobuf(const clangd::LookupRequest &From); + FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From); + RefsRequest toProtobuf(const clangd::RefsRequest &From); + RelationsRequest toProtobuf(const clangd::RelationsRequest &From); - llvm::Expected toProtobuf(const clangd::Symbol &From); - llvm::Expected toProtobuf(const clangd::Ref &From); - llvm::Expected toProtobuf(const clangd::SymbolID &Subject, - const clangd::Symbol &Object); + llvm::Expected toProtobuf(const clangd::Symbol &From); + llvm::Expected toProtobuf(const clangd::Ref &From); + llvm::Expected toProtobuf(const clangd::SymbolID &Subject, + const clangd::Symbol &Object); /// Translates \p RelativePath into the absolute path and builds URI for the /// user machine. This translation happens on the client side with the @@ -78,18 +77,18 @@ public: llvm::Expected uriToRelativePath(llvm::StringRef URI); private: - clangd::SymbolLocation::Position fromProtobuf(const v1::Position &Message); - v1::Position toProtobuf(const clangd::SymbolLocation::Position &Position); - clang::index::SymbolInfo fromProtobuf(const v1::SymbolInfo &Message); - v1::SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info); + clangd::SymbolLocation::Position fromProtobuf(const Position &Message); + Position toProtobuf(const clangd::SymbolLocation::Position &Position); + clang::index::SymbolInfo fromProtobuf(const SymbolInfo &Message); + SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info); llvm::Expected - fromProtobuf(const v1::SymbolLocation &Message); - llvm::Expected + fromProtobuf(const SymbolLocation &Message); + llvm::Expected toProtobuf(const clangd::SymbolLocation &Location); - llvm::Expected + llvm::Expected toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader); llvm::Expected - fromProtobuf(const v1::HeaderWithReferences &Message); + fromProtobuf(const HeaderWithReferences &Message); /// RemoteIndexRoot and LocalIndexRoot are absolute paths to the project (on /// remote and local machine respectively) and include a trailing slash. One diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp index df8ef2ba2f47..750c659381af 100644 --- a/clang-tools-extra/clangd/index/remote/server/Server.cpp +++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp @@ -74,7 +74,7 @@ llvm::cl::opt ServerAddress( "server-address", llvm::cl::init("0.0.0.0:50051"), llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051")); -class RemoteIndexServer final : public SymbolIndex::Service { +class RemoteIndexServer final : public v1::SymbolIndex::Service { public: RemoteIndexServer(clangd::SymbolIndex &Index, llvm::StringRef IndexRoot) : Index(Index) { diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp index df4811a8635c..6ef8da59861f 100644 --- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp +++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp @@ -133,7 +133,7 @@ TEST(RemoteMarshallingTest, URITranslation) { // Paths transmitted over the wire can not be absolute, they have to be // relative. - v1::Ref WithAbsolutePath; + Ref WithAbsolutePath; *WithAbsolutePath.mutable_location()->mutable_file_path() = "/usr/local/user/home/HelloWorld.cpp"; Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath); @@ -282,7 +282,7 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) { Sym.IncludeHeaders.pop_back(); Serialized = ProtobufMarshaller.toProtobuf(Sym); ASSERT_TRUE(bool(Serialized)); - v1::HeaderWithReferences InvalidHeader; + HeaderWithReferences InvalidHeader; InvalidHeader.set_header(convert_to_slash("/absolute/path/Header.h")); InvalidHeader.set_references(9000); *Serialized->add_headers() = InvalidHeader; @@ -388,7 +388,7 @@ TEST(RemoteMarshallingTest, RelationsRequestSerialization) { } TEST(RemoteMarshallingTest, RelationsRequestFailingSerialization) { - v1::RelationsRequest Serialized; + RelationsRequest Serialized; Serialized.add_subjects("ZZZZZZZZZZZZZZZZ"); Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/")); auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);