[clang-tools-extra] f49a7ad - [clangd] Add marshalling code for all request types
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 27 05:16:23 PDT 2020
Author: Kirill Bobyrev
Date: 2020-07-27T14:16:03+02:00
New Revision: f49a7ad8c0854a01b945c27de2fd313b9013ae0d
URL: https://github.com/llvm/llvm-project/commit/f49a7ad8c0854a01b945c27de2fd313b9013ae0d
DIFF: https://github.com/llvm/llvm-project/commit/f49a7ad8c0854a01b945c27de2fd313b9013ae0d.diff
LOG: [clangd] Add marshalling code for all request types
Summary:
Only FuzzyFindRequest is implemented via Marshaller even though other requests
also follow a similar pattern. Unify them under the marshalling umbrella and
make the server requests even more uniform to complement D84499.
Reviewers: kadircet
Reviewed By: kadircet
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits, sammccall
Tags: #clang
Differential Revision: https://reviews.llvm.org/D84525
Added:
Modified:
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
clang-tools-extra/clangd/index/remote/server/Server.cpp
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index b6c83c974072..b2085bc21f48 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -17,6 +17,7 @@
#include "index/SymbolOrigin.h"
#include "support/Logger.h"
#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallString.h"
@@ -30,6 +31,22 @@ namespace clang {
namespace clangd {
namespace remote {
+namespace {
+
+template <typename MessageT>
+llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(MessageT *Message) {
+ llvm::DenseSet<SymbolID> Result;
+ for (const auto &ID : Message->ids()) {
+ auto SID = SymbolID::fromStr(StringRef(ID));
+ if (!SID)
+ return SID.takeError();
+ Result.insert(*SID);
+ }
+ return Result;
+}
+
+} // namespace
+
Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
llvm::StringRef LocalIndexRoot)
: Strings(Arena) {
@@ -49,27 +66,50 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
}
-clangd::FuzzyFindRequest
-Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
+llvm::Expected<clangd::LookupRequest>
+Marshaller::fromProtobuf(const LookupRequest *Message) {
+ clangd::LookupRequest Req;
+ auto IDs = getIDs(Message);
+ if (!IDs)
+ return IDs.takeError();
+ Req.IDs = std::move(*IDs);
+ return Req;
+}
+
+llvm::Expected<clangd::FuzzyFindRequest>
+Marshaller::fromProtobuf(const FuzzyFindRequest *Message) {
assert(RemoteIndexRoot);
clangd::FuzzyFindRequest Result;
- Result.Query = Request->query();
- for (const auto &Scope : Request->scopes())
+ Result.Query = Message->query();
+ for (const auto &Scope : Message->scopes())
Result.Scopes.push_back(Scope);
- Result.AnyScope = Request->any_scope();
- if (Request->limit())
- Result.Limit = Request->limit();
- Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
- for (const auto &Path : Request->proximity_paths()) {
+ Result.AnyScope = Message->any_scope();
+ if (Message->limit())
+ Result.Limit = Message->limit();
+ Result.RestrictForCodeCompletion = Message->restricted_for_code_completion();
+ for (const auto &Path : Message->proximity_paths()) {
llvm::SmallString<256> LocalPath = llvm::StringRef(*RemoteIndexRoot);
llvm::sys::path::append(LocalPath, Path);
Result.ProximityPaths.push_back(std::string(LocalPath));
}
- for (const auto &Type : Request->preferred_types())
+ for (const auto &Type : Message->preferred_types())
Result.ProximityPaths.push_back(Type);
return Result;
}
+llvm::Expected<clangd::RefsRequest>
+Marshaller::fromProtobuf(const RefsRequest *Message) {
+ clangd::RefsRequest Req;
+ auto IDs = getIDs(Message);
+ if (!IDs)
+ return IDs.takeError();
+ Req.IDs = std::move(*IDs);
+ Req.Filter = static_cast<RefKind>(Message->filter());
+ if (Message->limit())
+ Req.Limit = Message->limit();
+ return Req;
+}
+
llvm::Optional<clangd::Symbol> Marshaller::fromProtobuf(const Symbol &Message) {
if (!Message.has_info() || !Message.has_canonical_declaration()) {
elog("Cannot convert Symbol from protobuf (missing info, definition or "
@@ -157,8 +197,7 @@ FuzzyFindRequest Marshaller::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, *LocalIndexRoot,
- ""))
+ if (llvm::sys::path::replace_path_prefix(RelativePath, *LocalIndexRoot, ""))
RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
RelativePath, llvm::sys::path::Style::posix));
}
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
index 9129cff24db5..5d82cdb7e765 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,10 +38,15 @@ class Marshaller {
Marshaller() = delete;
Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
- clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request);
llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message);
llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message);
+ llvm::Expected<clangd::LookupRequest>
+ fromProtobuf(const LookupRequest *Message);
+ llvm::Expected<clangd::FuzzyFindRequest>
+ fromProtobuf(const FuzzyFindRequest *Message);
+ llvm::Expected<clangd::RefsRequest> fromProtobuf(const RefsRequest *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
diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp
index 07b1c736b672..7bf47a288e79 100644
--- a/clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -9,6 +9,7 @@
#include "index/Index.h"
#include "index/Serialization.h"
#include "index/remote/marshalling/Marshalling.h"
+#include "support/Logger.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Path.h"
@@ -59,14 +60,12 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status Lookup(grpc::ServerContext *Context,
const LookupRequest *Request,
grpc::ServerWriter<LookupReply> *Reply) override {
- clangd::LookupRequest Req;
- for (const auto &ID : Request->ids()) {
- auto SID = SymbolID::fromStr(StringRef(ID));
- if (!SID)
- return grpc::Status::CANCELLED;
- Req.IDs.insert(*SID);
+ auto Req = ProtobufMarshaller->fromProtobuf(Request);
+ if (!Req) {
+ elog("Can not parse LookupRequest from protobuf: {0}", Req.takeError());
+ return grpc::Status::CANCELLED;
}
- Index->lookup(Req, [&](const clangd::Symbol &Sym) {
+ Index->lookup(*Req, [&](const clangd::Symbol &Sym) {
auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
if (!SerializedSymbol)
return;
@@ -83,8 +82,13 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status FuzzyFind(grpc::ServerContext *Context,
const FuzzyFindRequest *Request,
grpc::ServerWriter<FuzzyFindReply> *Reply) override {
- const auto Req = ProtobufMarshaller->fromProtobuf(Request);
- bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
+ auto Req = ProtobufMarshaller->fromProtobuf(Request);
+ if (!Req) {
+ elog("Can not parse FuzzyFindRequest from protobuf: {0}",
+ Req.takeError());
+ return grpc::Status::CANCELLED;
+ }
+ bool HasMore = Index->fuzzyFind(*Req, [&](const clangd::Symbol &Sym) {
auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
if (!SerializedSymbol)
return;
@@ -100,14 +104,12 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
grpc::ServerWriter<RefsReply> *Reply) override {
- clangd::RefsRequest Req;
- for (const auto &ID : Request->ids()) {
- auto SID = SymbolID::fromStr(StringRef(ID));
- if (!SID)
- return grpc::Status::CANCELLED;
- Req.IDs.insert(*SID);
+ auto Req = ProtobufMarshaller->fromProtobuf(Request);
+ if (!Req) {
+ elog("Can not parse RefsRequest from protobuf: {0}", Req.takeError());
+ return grpc::Status::CANCELLED;
}
- bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
+ bool HasMore = Index->refs(*Req, [&](const clangd::Ref &Reference) {
auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
if (!SerializedRef)
return;
diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index 147601b665c4..f975d1c35e1e 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -18,6 +18,7 @@
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/StringSaver.h"
#include "gmock/gmock.h"
@@ -271,6 +272,30 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
}
+TEST(RemoteMarshallingTest, LookupRequestSerialization) {
+ clangd::LookupRequest Request;
+ Request.IDs.insert(llvm::cantFail(SymbolID::fromStr("0000000000000001")));
+ Request.IDs.insert(llvm::cantFail(SymbolID::fromStr("0000000000000002")));
+
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
+ EXPECT_EQ(static_cast<unsigned>(Serialized.ids_size()), Request.IDs.size());
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ ASSERT_TRUE(bool(Deserialized));
+ EXPECT_EQ(Deserialized->IDs, Request.IDs);
+}
+
+TEST(RemoteMarshallingTest, LookupRequestFailingSerialization) {
+ clangd::LookupRequest Request;
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
+ Serialized.add_ids("Invalid Symbol ID");
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ EXPECT_FALSE(Deserialized);
+ llvm::consumeError(Deserialized.takeError());
+}
+
TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
clangd::FuzzyFindRequest Request;
Request.ProximityPaths = {testPath("local/Header.h"),
@@ -280,11 +305,43 @@ TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
auto Serialized = ProtobufMarshaller.toProtobuf(Request);
EXPECT_EQ(Serialized.proximity_paths_size(), 2);
auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
- EXPECT_THAT(Deserialized.ProximityPaths,
+ ASSERT_TRUE(bool(Deserialized));
+ EXPECT_THAT(Deserialized->ProximityPaths,
testing::ElementsAre(testPath("remote/Header.h"),
testPath("remote/subdir/OtherHeader.h")));
}
+TEST(RemoteMarshallingTest, RefsRequestSerialization) {
+ clangd::RefsRequest Request;
+ Request.IDs.insert(llvm::cantFail(SymbolID::fromStr("0000000000000001")));
+ Request.IDs.insert(llvm::cantFail(SymbolID::fromStr("0000000000000002")));
+
+ Request.Limit = 9000;
+ Request.Filter = RefKind::Spelled | RefKind::Declaration;
+
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
+ EXPECT_EQ(static_cast<unsigned>(Serialized.ids_size()), Request.IDs.size());
+ EXPECT_EQ(Serialized.limit(), Request.Limit);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ ASSERT_TRUE(bool(Deserialized));
+ EXPECT_EQ(Deserialized->IDs, Request.IDs);
+ ASSERT_TRUE(Deserialized->Limit);
+ EXPECT_EQ(*Deserialized->Limit, Request.Limit);
+ EXPECT_EQ(Deserialized->Filter, Request.Filter);
+}
+
+TEST(RemoteMarshallingTest, RefsRequestFailingSerialization) {
+ clangd::RefsRequest Request;
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
+ Serialized.add_ids("Invalid Symbol ID");
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ EXPECT_FALSE(Deserialized);
+ llvm::consumeError(Deserialized.takeError());
+}
+
TEST(RemoteMarshallingTest, RelativePathToURITranslation) {
Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"",
/*LocalIndexRoot=*/testPath("home/project/"));
More information about the cfe-commits
mailing list