diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp index b46883193076..35ce84068f40 100644 --- a/clang-tools-extra/clangd/index/remote/Client.cpp +++ b/clang-tools-extra/clangd/index/remote/Client.cpp @@ -29,16 +29,6 @@ class IndexClient : public clangd::SymbolIndex { using StreamingCall = std::unique_ptr> ( remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &); - template - RequestT serializeRequest(ClangdRequestT Request) const { - return toProtobuf(Request); - } - - template <> - FuzzyFindRequest serializeRequest(clangd::FuzzyFindRequest Request) const { - return toProtobuf(Request, ProjectRoot); - } - template bool streamRPC(ClangdRequestT Request, @@ -46,24 +36,23 @@ class IndexClient : public clangd::SymbolIndex { CallbackT Callback) const { bool FinalResult = false; trace::Span Tracer(RequestT::descriptor()->name()); - const auto RPCRequest = serializeRequest(Request); + const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request); grpc::ClientContext Context; std::chrono::system_clock::time_point Deadline = std::chrono::system_clock::now() + DeadlineWaitingTime; Context.set_deadline(Deadline); auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest); - llvm::BumpPtrAllocator Arena; - llvm::UniqueStringSaver Strings(Arena); ReplyT Reply; while (Reader->Read(&Reply)) { if (!Reply.has_stream_result()) { FinalResult = Reply.final_result(); continue; } - auto Response = - fromProtobuf(Reply.stream_result(), &Strings, ProjectRoot); - if (!Response) + auto Response = ProtobufMarshaller->fromProtobuf(Reply.stream_result()); + if (!Response) { elog("Received invalid {0}", ReplyT::descriptor()->name()); + continue; + } Callback(*Response); } SPAN_ATTACH(Tracer, "status", Reader->Finish().ok()); @@ -74,7 +63,9 @@ public: IndexClient( std::shared_ptr Channel, llvm::StringRef ProjectRoot, std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000)) - : Stub(remote::SymbolIndex::NewStub(Channel)), ProjectRoot(ProjectRoot), + : Stub(remote::SymbolIndex::NewStub(Channel)), + ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"", + /*LocalIndexRoot=*/ProjectRoot)), DeadlineWaitingTime(DeadlineTime) {} void lookup(const clangd::LookupRequest &Request, @@ -105,7 +96,7 @@ public: private: std::unique_ptr Stub; - std::string ProjectRoot; + std::unique_ptr ProtobufMarshaller; // Each request will be terminated if it takes too long. std::chrono::milliseconds DeadlineWaitingTime; }; diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp index db895bf039ba..059c42ee0eaa 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp @@ -30,101 +30,28 @@ 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; -} - -llvm::Optional -fromProtobuf(const SymbolLocation &Message, llvm::UniqueStringSaver *Strings, - llvm::StringRef IndexRoot) { - clangd::SymbolLocation Location; - auto URIString = relativePathToURI(Message.file_path(), IndexRoot); - if (!URIString) - return llvm::None; - Location.FileURI = Strings->save(*URIString).begin(); - Location.Start = fromProtobuf(Message.start()); - Location.End = fromProtobuf(Message.end()); - return Location; -} - -llvm::Optional -toProtobuf(const clangd::SymbolLocation &Location, llvm::StringRef IndexRoot) { - remote::SymbolLocation Result; - auto RelativePath = uriToRelativePath(Location.FileURI, IndexRoot); - if (!RelativePath) - return llvm::None; - *Result.mutable_file_path() = *RelativePath; - *Result.mutable_start() = toProtobuf(Location.Start); - *Result.mutable_end() = toProtobuf(Location.End); - return Result; -} - -llvm::Optional -toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader, - llvm::StringRef IndexRoot) { - HeaderWithReferences Result; - Result.set_references(IncludeHeader.References); - const std::string Header = IncludeHeader.IncludeHeader.str(); - if (isLiteralInclude(Header)) { - Result.set_header(Header); - return Result; +Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot, + llvm::StringRef LocalIndexRoot) + : Strings(Arena) { + if (!RemoteIndexRoot.empty()) { + assert(llvm::sys::path::is_absolute(RemoteIndexRoot)); + assert(RemoteIndexRoot == + llvm::sys::path::convert_to_slash(RemoteIndexRoot)); + assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator())); + this->RemoteIndexRoot = RemoteIndexRoot.str(); } - auto RelativePath = uriToRelativePath(Header, IndexRoot); - if (!RelativePath) - return llvm::None; - Result.set_header(*RelativePath); - return Result; -} - -llvm::Optional -fromProtobuf(const HeaderWithReferences &Message, - llvm::UniqueStringSaver *Strings, llvm::StringRef IndexRoot) { - std::string Header = Message.header(); - if (Header.front() != '<' && Header.front() != '"') { - auto URIString = relativePathToURI(Header, IndexRoot); - if (!URIString) - return llvm::None; - Header = *URIString; + if (!LocalIndexRoot.empty()) { + assert(llvm::sys::path::is_absolute(LocalIndexRoot)); + assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot)); + assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator())); + this->LocalIndexRoot = LocalIndexRoot.str(); } - return clangd::Symbol::IncludeHeaderWithReferences{Strings->save(Header), - Message.references()}; + assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty()); } -} // namespace - -clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request, - llvm::StringRef IndexRoot) { +clangd::FuzzyFindRequest +Marshaller::fromProtobuf(const FuzzyFindRequest *Request) { + assert(LocalIndexRoot); clangd::FuzzyFindRequest Result; Result.Query = Request->query(); for (const auto &Scope : Request->scopes()) @@ -134,7 +61,7 @@ clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request, Result.Limit = Request->limit(); Result.RestrictForCodeCompletion = Request->restricted_for_code_completion(); for (const auto &Path : Request->proximity_paths()) { - llvm::SmallString<256> LocalPath = llvm::StringRef(IndexRoot); + llvm::SmallString<256> LocalPath = llvm::StringRef(*LocalIndexRoot); llvm::sys::path::append(LocalPath, Path); Result.ProximityPaths.push_back(std::string(LocalPath)); } @@ -143,34 +70,35 @@ clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request, return Result; } -llvm::Optional fromProtobuf(const Symbol &Message, - llvm::UniqueStringSaver *Strings, - llvm::StringRef IndexRoot) { - if (!Message.has_info() || !Message.has_definition() || - !Message.has_canonical_declaration()) { - elog("Cannot convert Symbol from Protobuf: {0}", - Message.ShortDebugString()); +llvm::Optional Marshaller::fromProtobuf(const Symbol &Message) { + if (!Message.has_info() || !Message.has_canonical_declaration()) { + elog("Cannot convert Symbol from protobuf (missing info, definition or " + "declaration): {0}", + Message.DebugString()); return llvm::None; } clangd::Symbol Result; auto ID = SymbolID::fromStr(Message.id()); if (!ID) { - elog("Cannot parse SymbolID {0} given Protobuf: {1}", ID.takeError(), - Message.ShortDebugString()); + elog("Cannot parse SymbolID {0} given protobuf: {1}", ID.takeError(), + Message.DebugString()); return llvm::None; } Result.ID = *ID; Result.SymInfo = fromProtobuf(Message.info()); Result.Name = Message.name(); Result.Scope = Message.scope(); - auto Definition = fromProtobuf(Message.definition(), Strings, IndexRoot); - if (!Definition) - return llvm::None; - Result.Definition = *Definition; - auto Declaration = - fromProtobuf(Message.canonical_declaration(), Strings, IndexRoot); - if (!Declaration) + if (Message.has_definition()) { + auto Definition = fromProtobuf(Message.definition()); + if (Definition) + Result.Definition = *Definition; + } + auto Declaration = fromProtobuf(Message.canonical_declaration()); + if (!Declaration) { + elog("Cannot convert Symbol from protobuf (invalid declaration): {0}", + Message.DebugString()); return llvm::None; + } Result.CanonicalDeclaration = *Declaration; Result.References = Message.references(); Result.Origin = static_cast(Message.origin()); @@ -181,39 +109,44 @@ llvm::Optional fromProtobuf(const Symbol &Message, Result.ReturnType = Message.return_type(); Result.Type = Message.type(); for (const auto &Header : Message.headers()) { - auto SerializedHeader = fromProtobuf(Header, Strings, IndexRoot); + auto SerializedHeader = fromProtobuf(Header); if (SerializedHeader) Result.IncludeHeaders.push_back(*SerializedHeader); + else + elog("Cannot convert HeaderWithIncludes from protobuf: {0}", + Header.DebugString()); } Result.Flags = static_cast(Message.flags()); return Result; } -llvm::Optional fromProtobuf(const Ref &Message, - llvm::UniqueStringSaver *Strings, - llvm::StringRef IndexRoot) { +llvm::Optional Marshaller::fromProtobuf(const Ref &Message) { if (!Message.has_location()) { - elog("Cannot convert Ref from Protobuf: {}", Message.ShortDebugString()); + elog("Cannot convert Ref from protobuf (missing location): {0}", + Message.DebugString()); return llvm::None; } clangd::Ref Result; - auto Location = fromProtobuf(Message.location(), Strings, IndexRoot); - if (!Location) + auto Location = fromProtobuf(Message.location()); + if (!Location) { + elog("Cannot convert Ref from protobuf (invalid location): {0}", + Message.DebugString()); return llvm::None; + } Result.Location = *Location; Result.Kind = static_cast(Message.kind()); return Result; } -LookupRequest toProtobuf(const clangd::LookupRequest &From) { +LookupRequest Marshaller::toProtobuf(const clangd::LookupRequest &From) { LookupRequest RPCRequest; for (const auto &SymbolID : From.IDs) RPCRequest.add_ids(SymbolID.str()); return RPCRequest; } -FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From, - llvm::StringRef IndexRoot) { +FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) { + assert(RemoteIndexRoot); FuzzyFindRequest RPCRequest; RPCRequest.set_query(From.Query); for (const auto &Scope : From.Scopes) @@ -224,7 +157,8 @@ FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From, RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion); for (const auto &Path : From.ProximityPaths) { llvm::SmallString<256> RelativePath = llvm::StringRef(Path); - if (llvm::sys::path::replace_path_prefix(RelativePath, IndexRoot, "")) + if (llvm::sys::path::replace_path_prefix(RelativePath, *RemoteIndexRoot, + "")) RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash( RelativePath, llvm::sys::path::Style::posix)); } @@ -233,7 +167,7 @@ FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From, return RPCRequest; } -RefsRequest toProtobuf(const clangd::RefsRequest &From) { +RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) { RefsRequest RPCRequest; for (const auto &ID : From.IDs) RPCRequest.add_ids(ID.str()); @@ -243,18 +177,28 @@ RefsRequest toProtobuf(const clangd::RefsRequest &From) { return RPCRequest; } -Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot) { +llvm::Optional 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()); - auto Definition = toProtobuf(From.Definition, IndexRoot); - if (Definition) + if (*From.Definition.FileURI) { + auto Definition = toProtobuf(From.Definition); + if (!Definition) { + elog("Can not convert Symbol to protobuf (invalid definition) {0}: {1}", + From, From.Definition); + return llvm::None; + } *Result.mutable_definition() = *Definition; + } Result.set_scope(From.Scope.str()); - auto Declaration = toProtobuf(From.CanonicalDeclaration, IndexRoot); - if (Declaration) - *Result.mutable_canonical_declaration() = *Declaration; + auto Declaration = toProtobuf(From.CanonicalDeclaration); + if (!Declaration) { + elog("Can not convert Symbol to protobuf (invalid declaration) {0}: {1}", + From, From.CanonicalDeclaration); + return llvm::None; + } + *Result.mutable_canonical_declaration() = *Declaration; Result.set_references(From.References); Result.set_origin(static_cast(From.Origin)); Result.set_signature(From.Signature.str()); @@ -265,9 +209,12 @@ Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot) { Result.set_return_type(From.ReturnType.str()); Result.set_type(From.Type.str()); for (const auto &Header : From.IncludeHeaders) { - auto Serialized = toProtobuf(Header, IndexRoot); - if (!Serialized) + auto Serialized = toProtobuf(Header); + if (!Serialized) { + elog("Can not convert IncludeHeaderWithReferences to protobuf: {0}", + Header.IncludeHeader); continue; + } auto *NextHeader = Result.add_headers(); *NextHeader = *Serialized; } @@ -275,50 +222,38 @@ Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot) { return Result; } -// FIXME(kirillbobyrev): A reference without location is invalid. -// llvm::Optional here and on the server side? -Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot) { +llvm::Optional Marshaller::toProtobuf(const clangd::Ref &From) { Ref Result; Result.set_kind(static_cast(From.Kind)); - auto Location = toProtobuf(From.Location, IndexRoot); - if (Location) - *Result.mutable_location() = *Location; + auto Location = toProtobuf(From.Location); + if (!Location) { + elog("Can not convert Reference to protobuf (invalid location) {0}: {1}", + From, From.Location); + return llvm::None; + } + *Result.mutable_location() = *Location; return Result; } -llvm::Optional relativePathToURI(llvm::StringRef RelativePath, - llvm::StringRef IndexRoot) { +llvm::Optional +Marshaller::relativePathToURI(llvm::StringRef RelativePath) { + assert(LocalIndexRoot); assert(RelativePath == llvm::sys::path::convert_to_slash( RelativePath, llvm::sys::path::Style::posix)); - assert(IndexRoot == llvm::sys::path::convert_to_slash(IndexRoot)); - assert(IndexRoot.endswith(llvm::sys::path::get_separator())); - if (RelativePath.empty()) - return std::string(); + if (RelativePath.empty()) { + return llvm::None; + } if (llvm::sys::path::is_absolute(RelativePath)) { - elog("Remote index client got absolute path from server: {0}", - RelativePath); return llvm::None; } - if (llvm::sys::path::is_relative(IndexRoot)) { - elog("Remote index client got a relative path as index root: {0}", - IndexRoot); - return llvm::None; - } - llvm::SmallString<256> FullPath = IndexRoot; + llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot); llvm::sys::path::append(FullPath, RelativePath); auto Result = URI::createFile(FullPath); return Result.toString(); } -llvm::Optional uriToRelativePath(llvm::StringRef URI, - llvm::StringRef IndexRoot) { - assert(IndexRoot.endswith(llvm::sys::path::get_separator())); - assert(IndexRoot == llvm::sys::path::convert_to_slash(IndexRoot)); - assert(!IndexRoot.empty()); - if (llvm::sys::path::is_relative(IndexRoot)) { - elog("Index root {0} is not absolute path", IndexRoot); - return llvm::None; - } +llvm::Optional Marshaller::uriToRelativePath(llvm::StringRef URI) { + assert(RemoteIndexRoot); auto ParsedURI = URI::parse(URI); if (!ParsedURI) { elog("Remote index got bad URI from client {0}: {1}", URI, @@ -326,14 +261,10 @@ llvm::Optional uriToRelativePath(llvm::StringRef URI, return llvm::None; } if (ParsedURI->scheme() != "file") { - elog("Remote index got URI with scheme other than \"file\" {0}: {1}", URI, - ParsedURI->scheme()); return llvm::None; } llvm::SmallString<256> Result = ParsedURI->body(); - if (!llvm::sys::path::replace_path_prefix(Result, IndexRoot, "")) { - elog("Can not get relative path from the URI {0} given the index root {1}", - URI, IndexRoot); + if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, "")) { return llvm::None; } // Make sure the result has UNIX slashes. @@ -341,6 +272,94 @@ llvm::Optional uriToRelativePath(llvm::StringRef URI, llvm::sys::path::Style::posix); } +clangd::SymbolLocation::Position +Marshaller::fromProtobuf(const Position &Message) { + clangd::SymbolLocation::Position Result; + Result.setColumn(static_cast(Message.column())); + Result.setLine(static_cast(Message.line())); + return Result; +} + +Position +Marshaller::toProtobuf(const clangd::SymbolLocation::Position &Position) { + remote::Position Result; + Result.set_column(Position.column()); + Result.set_line(Position.line()); + return Result; +} + +clang::index::SymbolInfo Marshaller::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 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)); + Result.set_properties(static_cast(Info.Properties)); + return Result; +} + +llvm::Optional +Marshaller::fromProtobuf(const SymbolLocation &Message) { + clangd::SymbolLocation Location; + auto URIString = relativePathToURI(Message.file_path()); + if (!URIString) + return llvm::None; + Location.FileURI = Strings.save(*URIString).begin(); + Location.Start = fromProtobuf(Message.start()); + Location.End = fromProtobuf(Message.end()); + return Location; +} + +llvm::Optional +Marshaller::toProtobuf(const clangd::SymbolLocation &Location) { + remote::SymbolLocation Result; + auto RelativePath = uriToRelativePath(Location.FileURI); + if (!RelativePath) + return llvm::None; + *Result.mutable_file_path() = *RelativePath; + *Result.mutable_start() = toProtobuf(Location.Start); + *Result.mutable_end() = toProtobuf(Location.End); + return Result; +} + +llvm::Optional Marshaller::toProtobuf( + const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) { + HeaderWithReferences Result; + Result.set_references(IncludeHeader.References); + const std::string Header = IncludeHeader.IncludeHeader.str(); + if (isLiteralInclude(Header)) { + Result.set_header(Header); + return Result; + } + auto RelativePath = uriToRelativePath(Header); + if (!RelativePath) + return llvm::None; + Result.set_header(*RelativePath); + return Result; +} + +llvm::Optional +Marshaller::fromProtobuf(const HeaderWithReferences &Message) { + std::string Header = Message.header(); + if (Header.front() != '<' && Header.front() != '"') { + auto URIString = relativePathToURI(Header); + if (!URIString) + return llvm::None; + Header = *URIString; + } + return clangd::Symbol::IncludeHeaderWithReferences{Strings.save(Header), + Message.references()}; +} + } // namespace remote } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h index 86cc8fa272fc..9129cff24db5 100644 --- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h +++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h @@ -5,31 +5,6 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===----------------------------------------------------------------------===// -// -// Marshalling provides translation between native clangd types into the -// Protobuf-generated classes. Most translations are 1-to-1 and wrap variables -// into appropriate Protobuf types. -// -// A notable exception is URI translation. Because paths to files are different -// on indexing machine and client machine -// ("/remote/machine/projects/llvm-project/llvm/include/HelloWorld.h" versus -// "/usr/local/username/llvm-project/llvm/include/HelloWorld.h"), they need to -// be converted appropriately. Remote machine strips the prefix from the -// absolute path and passes paths relative to the project root over the wire -// ("include/HelloWorld.h" in this example). The indexed project root is passed -// to the remote server. Client receives this relative path and constructs a URI -// that points to the relevant file in the filesystem. The relative path is -// appended to IndexRoot to construct the full path and build the final URI. -// -// Index root is the absolute path to the project and includes a trailing slash. -// The relative path passed over the wire has unix slashes. -// -// 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. -// -//===----------------------------------------------------------------------===// #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REMOTE_MARSHALLING_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REMOTE_MARSHALLING_H @@ -43,33 +18,76 @@ namespace clang { namespace clangd { namespace remote { -clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request, - llvm::StringRef IndexRoot); -llvm::Optional fromProtobuf(const Symbol &Message, - llvm::UniqueStringSaver *Strings, - llvm::StringRef IndexRoot); -llvm::Optional fromProtobuf(const Ref &Message, - llvm::UniqueStringSaver *Strings, - llvm::StringRef IndexRoot); +// Marshalling provides an interface for translattion between native clangd +// types into the Protobuf-generated classes. Most translations are 1-to-1 and +// wrap variables into appropriate Protobuf types. +// +/// A notable exception is URI translation. Because paths to files are different +/// on indexing machine and client machine +/// ("/remote/machine/projects/llvm-project/llvm/include/HelloWorld.h" versus +/// "/usr/local/username/llvm-project/llvm/include/HelloWorld.h"), they need to +/// be converted appropriately. Remote machine strips the prefix +/// (RemoteIndexRoot) from the absolute path and passes paths relative to the +/// project root over the wire ("include/HelloWorld.h" in this example). The +/// indexed project root is passed to the remote server. Client receives this +/// relative path and constructs a URI that points to the relevant file in the +/// filesystem. The relative path is appended to LocalIndexRoot to construct the +/// full path and build the final URI. +class Marshaller { +public: + Marshaller() = delete; + Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot); -LookupRequest toProtobuf(const clangd::LookupRequest &From); -FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From, - llvm::StringRef IndexRoot); -RefsRequest toProtobuf(const clangd::RefsRequest &From); + clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request); + llvm::Optional fromProtobuf(const Symbol &Message); + llvm::Optional fromProtobuf(const Ref &Message); -Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot); -Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot); + /// 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. + LookupRequest toProtobuf(const clangd::LookupRequest &From); + FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From); + RefsRequest toProtobuf(const clangd::RefsRequest &From); -/// Translates \p RelativePath into the absolute path and builds URI for the -/// user machine. This translation happens on the client side with the -/// \p RelativePath received from remote index server and \p IndexRoot is -/// provided by the client. -llvm::Optional relativePathToURI(llvm::StringRef RelativePath, - llvm::StringRef IndexRoot); -/// Translates a URI from the server's backing index to a relative path suitable -/// to send over the wire to the client. -llvm::Optional uriToRelativePath(llvm::StringRef URI, - llvm::StringRef IndexRoot); + llvm::Optional toProtobuf(const clangd::Ref &From); + llvm::Optional toProtobuf(const clangd::Symbol &From); + + /// Translates \p RelativePath into the absolute path and builds URI for the + /// user machine. This translation happens on the client side with the + /// \p RelativePath received from remote index server and \p IndexRoot is + /// provided by the client. + /// + /// The relative path passed over the wire has unix slashes. + llvm::Optional relativePathToURI(llvm::StringRef RelativePath); + /// Translates a URI from the server's backing index to a relative path + /// suitable to send over the wire to the client. + llvm::Optional uriToRelativePath(llvm::StringRef URI); + +private: + 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::Optional + fromProtobuf(const SymbolLocation &Message); + llvm::Optional + toProtobuf(const clangd::SymbolLocation &Location); + llvm::Optional + toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader); + llvm::Optional + 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 + /// of them can be missing (if the machines are different they don't know each + /// other's specifics and will only do one-way translation), but both can not + /// be missing at the same time. + llvm::Optional RemoteIndexRoot; + llvm::Optional LocalIndexRoot; + llvm::BumpPtrAllocator Arena; + llvm::UniqueStringSaver Strings; +}; } // namespace remote } // namespace clangd diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp index fecd72806cbc..07b1c736b672 100644 --- a/clang-tools-extra/clangd/index/remote/server/Server.cpp +++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp @@ -50,7 +50,9 @@ public: : Index(std::move(Index)) { llvm::SmallString<256> NativePath = IndexRoot; llvm::sys::path::native(NativePath); - IndexedProjectRoot = std::string(NativePath); + ProtobufMarshaller = std::unique_ptr(new Marshaller( + /*RemoteIndexRoot=*/llvm::StringRef(NativePath), + /*LocalIndexRoot=*/"")); } private: @@ -65,9 +67,11 @@ private: Req.IDs.insert(*SID); } Index->lookup(Req, [&](const clangd::Symbol &Sym) { + auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym); + if (!SerializedSymbol) + return; LookupReply NextMessage; - *NextMessage.mutable_stream_result() = - toProtobuf(Sym, IndexedProjectRoot); + *NextMessage.mutable_stream_result() = *SerializedSymbol; Reply->Write(NextMessage); }); LookupReply LastMessage; @@ -79,11 +83,13 @@ private: grpc::Status FuzzyFind(grpc::ServerContext *Context, const FuzzyFindRequest *Request, grpc::ServerWriter *Reply) override { - const auto Req = fromProtobuf(Request, IndexedProjectRoot); + const auto Req = ProtobufMarshaller->fromProtobuf(Request); bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) { + auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym); + if (!SerializedSymbol) + return; FuzzyFindReply NextMessage; - *NextMessage.mutable_stream_result() = - toProtobuf(Sym, IndexedProjectRoot); + *NextMessage.mutable_stream_result() = *SerializedSymbol; Reply->Write(NextMessage); }); FuzzyFindReply LastMessage; @@ -102,8 +108,11 @@ private: Req.IDs.insert(*SID); } bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) { + auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference); + if (!SerializedRef) + return; RefsReply NextMessage; - *NextMessage.mutable_stream_result() = toProtobuf(Reference, IndexRoot); + *NextMessage.mutable_stream_result() = *SerializedRef; Reply->Write(NextMessage); }); RefsReply LastMessage; @@ -113,7 +122,7 @@ private: } std::unique_ptr Index; - std::string IndexedProjectRoot; + std::unique_ptr ProtobufMarshaller; }; void runServer(std::unique_ptr Index, diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp index a14ff13150d3..6cc08144bef8 100644 --- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp +++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp @@ -13,6 +13,7 @@ #include "index/Serialization.h" #include "index/Symbol.h" #include "index/SymbolID.h" +#include "index/SymbolLocation.h" #include "index/remote/marshalling/Marshalling.h" #include "clang/Index/IndexSymbol.h" #include "llvm/ADT/SmallString.h" @@ -39,29 +40,35 @@ const char *testPathURI(llvm::StringRef Path, TEST(RemoteMarshallingTest, URITranslation) { llvm::BumpPtrAllocator Arena; llvm::UniqueStringSaver Strings(Arena); + Marshaller ProtobufMarshaller( + testPath("remote/machine/projects/llvm-project/"), + testPath("home/my-projects/llvm-project/")); clangd::Ref Original; Original.Location.FileURI = testPathURI("remote/machine/projects/llvm-project/clang-tools-extra/" "clangd/unittests/remote/MarshallingTests.cpp", Strings); - auto Serialized = - toProtobuf(Original, testPath("remote/machine/projects/llvm-project/")); - EXPECT_EQ(Serialized.location().file_path(), + auto Serialized = ProtobufMarshaller.toProtobuf(Original); + EXPECT_TRUE(Serialized); + EXPECT_EQ(Serialized->location().file_path(), "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp"); - const std::string LocalIndexPrefix = testPath("local/machine/project/"); - auto Deserialized = fromProtobuf(Serialized, &Strings, - testPath("home/my-projects/llvm-project/")); + auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); - EXPECT_EQ(Deserialized->Location.FileURI, - testPathURI("home/my-projects/llvm-project/clang-tools-extra/" - "clangd/unittests/remote/MarshallingTests.cpp", - Strings)); + EXPECT_STREQ(Deserialized->Location.FileURI, + testPathURI("home/my-projects/llvm-project/clang-tools-extra/" + "clangd/unittests/remote/MarshallingTests.cpp", + Strings)); + + // Can't have empty paths. + *Serialized->mutable_location()->mutable_file_path() = std::string(); + Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); + EXPECT_FALSE(Deserialized); clangd::Ref WithInvalidURI; - // Invalid URI results in empty path. + // Invalid URI results in serialization failure. WithInvalidURI.Location.FileURI = "This is not a URI"; - Serialized = toProtobuf(WithInvalidURI, testPath("home/")); - EXPECT_EQ(Serialized.location().file_path(), ""); + Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI); + EXPECT_FALSE(Serialized); // Can not use URIs with scheme different from "file". auto UnittestURI = @@ -69,15 +76,15 @@ TEST(RemoteMarshallingTest, URITranslation) { EXPECT_TRUE(bool(UnittestURI)); WithInvalidURI.Location.FileURI = Strings.save(UnittestURI->toString()).begin(); - Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/")); - EXPECT_EQ(Serialized.location().file_path(), ""); + Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI); + EXPECT_FALSE(Serialized); + // Paths transmitted over the wire can not be absolute, they have to be + // relative. Ref WithAbsolutePath; *WithAbsolutePath.mutable_location()->mutable_file_path() = "/usr/local/user/home/HelloWorld.cpp"; - Deserialized = fromProtobuf(WithAbsolutePath, &Strings, LocalIndexPrefix); - // Paths transmitted over the wire can not be absolute, they have to be - // relative. + Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath); EXPECT_FALSE(Deserialized); } @@ -128,48 +135,63 @@ TEST(RemoteMarshallingTest, SymbolSerialization) { Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion; + Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/")); + // Check that symbols are exactly the same if the path to indexed project is // the same on indexing machine and the client. - auto Serialized = toProtobuf(Sym, testPath("home/")); - auto Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/")); + auto Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_TRUE(Serialized); + auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized)); // Serialized paths are relative and have UNIX slashes. - EXPECT_EQ(convert_to_slash(Serialized.definition().file_path(), + EXPECT_EQ(convert_to_slash(Serialized->definition().file_path(), llvm::sys::path::Style::posix), - Serialized.definition().file_path()); + Serialized->definition().file_path()); EXPECT_TRUE( - llvm::sys::path::is_relative(Serialized.definition().file_path())); + llvm::sys::path::is_relative(Serialized->definition().file_path())); + + // Missing definition is OK. + Sym.Definition = clangd::SymbolLocation(); + Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_TRUE(Serialized); + Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); + EXPECT_TRUE(Deserialized); + + // Relative path is absolute. + *Serialized->mutable_canonical_declaration()->mutable_file_path() = + convert_to_slash("/path/to/Declaration.h"); + Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); + EXPECT_FALSE(Deserialized); // Fail with an invalid URI. Location.FileURI = "Not A URI"; Sym.Definition = Location; - Serialized = toProtobuf(Sym, testPath("home/")); - Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/")); - EXPECT_FALSE(Deserialized); + Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_FALSE(Serialized); // Schemes other than "file" can not be used. auto UnittestURI = URI::create(testPath("home/SomePath.h"), "unittest"); EXPECT_TRUE(bool(UnittestURI)); Location.FileURI = Strings.save(UnittestURI->toString()).begin(); Sym.Definition = Location; - Serialized = toProtobuf(Sym, testPath("home/")); - Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/")); - EXPECT_FALSE(Deserialized); + Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_FALSE(Serialized); // Passing root that is not prefix of the original file path. Location.FileURI = testPathURI("home/File.h", Strings); Sym.Definition = Location; // Check that the symbol is valid and passing the correct path works. - Serialized = toProtobuf(Sym, testPath("home/")); - Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/")); + Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_TRUE(Serialized); + Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); - EXPECT_EQ(Deserialized->Definition.FileURI, - testPathURI("home/File.h", Strings)); + EXPECT_STREQ(Deserialized->Definition.FileURI, + testPathURI("home/File.h", Strings)); // Fail with a wrong root. - Serialized = toProtobuf(Sym, testPath("nothome/")); - Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/")); - EXPECT_FALSE(Deserialized); + Marshaller WrongMarshaller(testPath("nothome/"), testPath("home/")); + Serialized = WrongMarshaller.toProtobuf(Sym); + EXPECT_FALSE(Serialized); } TEST(RemoteMarshallingTest, RefSerialization) { @@ -188,9 +210,12 @@ TEST(RemoteMarshallingTest, RefSerialization) { "llvm-project/llvm/clang-tools-extra/clangd/Protocol.h", Strings); Ref.Location = Location; - auto Serialized = toProtobuf(Ref, testPath("llvm-project/")); - auto Deserialized = - fromProtobuf(Serialized, &Strings, testPath("llvm-project/")); + Marshaller ProtobufMarshaller(testPath("llvm-project/"), + testPath("llvm-project/")); + + auto Serialized = ProtobufMarshaller.toProtobuf(Ref); + EXPECT_TRUE(Serialized); + auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); EXPECT_EQ(toYAML(Ref), toYAML(*Deserialized)); } @@ -242,10 +267,13 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) { InvalidHeaders.end()); Sym.IncludeHeaders = AllHeaders; - auto Serialized = toProtobuf(Sym, convert_to_slash("/")); - EXPECT_EQ(static_cast(Serialized.headers_size()), + Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/")); + + auto Serialized = ProtobufMarshaller.toProtobuf(Sym); + EXPECT_EQ(static_cast(Serialized->headers_size()), ValidHeaders.size()); - auto Deserialized = fromProtobuf(Serialized, &Strings, convert_to_slash("/")); + EXPECT_TRUE(Serialized); + auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized); EXPECT_TRUE(Deserialized); Sym.IncludeHeaders = ValidHeaders; @@ -257,35 +285,38 @@ TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) { Request.ProximityPaths = {testPath("remote/Header.h"), testPath("remote/subdir/OtherHeader.h"), testPath("notremote/File.h"), "Not a Path."}; - auto Serialized = toProtobuf(Request, testPath("remote/")); + Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/")); + auto Serialized = ProtobufMarshaller.toProtobuf(Request); EXPECT_EQ(Serialized.proximity_paths_size(), 2); - auto Deserialized = fromProtobuf(&Serialized, testPath("home/")); + auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized); EXPECT_THAT(Deserialized.ProximityPaths, testing::ElementsAre(testPath("home/Header.h"), testPath("home/subdir/OtherHeader.h"))); } TEST(RemoteMarshallingTest, RelativePathToURITranslation) { - EXPECT_TRUE(relativePathToURI("lib/File.cpp", testPath("home/project/"))); + Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"", + /*LocalIndexRoot=*/testPath("home/project/")); + EXPECT_TRUE(ProtobufMarshaller.relativePathToURI("lib/File.cpp")); // RelativePath can not be absolute. - EXPECT_FALSE(relativePathToURI("/lib/File.cpp", testPath("home/project/"))); - // IndexRoot has to be absolute path. - EXPECT_FALSE(relativePathToURI("lib/File.cpp", "home/project/")); + EXPECT_FALSE(ProtobufMarshaller.relativePathToURI("/lib/File.cpp")); + // RelativePath can not be empty. + EXPECT_FALSE(ProtobufMarshaller.relativePathToURI(std::string())); } TEST(RemoteMarshallingTest, URIToRelativePathTranslation) { llvm::BumpPtrAllocator Arena; llvm::UniqueStringSaver Strings(Arena); - EXPECT_TRUE( - uriToRelativePath(testPathURI("home/project/lib/File.cpp", Strings), - testPath("home/project/"))); - // IndexRoot has to be absolute path. - EXPECT_FALSE(uriToRelativePath( - testPathURI("home/project/lib/File.cpp", Strings), "home/project/")); - // IndexRoot has to be be a prefix of the file path. - EXPECT_FALSE( - uriToRelativePath(testPathURI("home/project/lib/File.cpp", Strings), - testPath("home/other/project/"))); + Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/testPath("remote/project/"), + /*LocalIndexRoot=*/""); + EXPECT_TRUE(ProtobufMarshaller.uriToRelativePath( + testPathURI("remote/project/lib/File.cpp", Strings))); + // RemoteIndexRoot has to be be a prefix of the file path. + Marshaller WrongMarshaller( + /*RemoteIndexRoot=*/testPath("remote/other/project/"), + /*LocalIndexRoot=*/""); + EXPECT_FALSE(WrongMarshaller.uriToRelativePath( + testPathURI("remote/project/lib/File.cpp", Strings))); } } // namespace