[clang-tools-extra] a262f0f - [clangd] Implement Relations request for remote index
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 30 03:58:39 PDT 2020
Author: Kirill Bobyrev
Date: 2020-07-30T12:57:33+02:00
New Revision: a262f0fea46ce08008f3462c336c3d7107e98b27
URL: https://github.com/llvm/llvm-project/commit/a262f0fea46ce08008f3462c336c3d7107e98b27
DIFF: https://github.com/llvm/llvm-project/commit/a262f0fea46ce08008f3462c336c3d7107e98b27.diff
LOG: [clangd] Implement Relations request for remote index
This is the last missing bit in the core remote index implementation. The only
remaining bits are some API refactorings (replacing Optional with Expected and
being better at reporting errors).
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D84894
Added:
Modified:
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/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/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp
index 5a33fd2eaf14..af58645d1795 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -91,11 +91,16 @@ class IndexClient : public clangd::SymbolIndex {
return streamRPC(Request, &remote::SymbolIndex::Stub::Refs, Callback);
}
- // FIXME(kirillbobyrev): Implement this.
void
- relations(const clangd::RelationsRequest &,
- llvm::function_ref<void(const SymbolID &, const clangd::Symbol &)>)
- const {}
+ relations(const clangd::RelationsRequest &Request,
+ llvm::function_ref<void(const SymbolID &, const clangd::Symbol &)>
+ Callback) const {
+ streamRPC(Request, &remote::SymbolIndex::Stub::Relations,
+ // Unpack protobuf Relation.
+ [&](std::pair<SymbolID, clangd::Symbol> SubjectAndObject) {
+ Callback(SubjectAndObject.first, SubjectAndObject.second);
+ });
+ }
// IndexClient does not take any space since the data is stored on the
// server.
diff --git a/clang-tools-extra/clangd/index/remote/Index.proto b/clang-tools-extra/clangd/index/remote/Index.proto
index 99c4e3329d67..305164ffef77 100644
--- a/clang-tools-extra/clangd/index/remote/Index.proto
+++ b/clang-tools-extra/clangd/index/remote/Index.proto
@@ -18,6 +18,8 @@ service SymbolIndex {
rpc FuzzyFind(FuzzyFindRequest) returns (stream FuzzyFindReply) {}
rpc Refs(RefsRequest) returns (stream RefsReply) {}
+
+ rpc Relations(RelationsRequest) returns (stream RelationsReply) {}
}
message LookupRequest { repeated string ids = 1; }
@@ -114,3 +116,25 @@ message HeaderWithReferences {
string header = 1;
uint32 references = 2;
}
+
+message RelationsRequest {
+ repeated string subjects = 1;
+ uint32 predicate = 2;
+ uint32 limit = 3;
+}
+
+// The response is a stream of reference messages, and one terminating has_more
+// message.
+message RelationsReply {
+ oneof kind {
+ Relation stream_result = 1;
+ bool final_result = 2; // HasMore
+ }
+}
+
+// This struct does not mirror clangd::Relation but rather the arguments of
+// SymbolIndex::relations callback.
+message Relation {
+ string subject_id = 1;
+ Symbol object = 2;
+}
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index b2085bc21f48..270e15347ee0 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -10,6 +10,7 @@
#include "Headers.h"
#include "Index.pb.h"
#include "Protocol.h"
+#include "index/Index.h"
#include "index/Serialization.h"
#include "index/Symbol.h"
#include "index/SymbolID.h"
@@ -33,10 +34,10 @@ namespace remote {
namespace {
-template <typename MessageT>
-llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(MessageT *Message) {
+template <typename IDRange>
+llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(IDRange IDs) {
llvm::DenseSet<SymbolID> Result;
- for (const auto &ID : Message->ids()) {
+ for (const auto &ID : IDs) {
auto SID = SymbolID::fromStr(StringRef(ID));
if (!SID)
return SID.takeError();
@@ -69,7 +70,7 @@ Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
llvm::Expected<clangd::LookupRequest>
Marshaller::fromProtobuf(const LookupRequest *Message) {
clangd::LookupRequest Req;
- auto IDs = getIDs(Message);
+ auto IDs = getIDs(Message->ids());
if (!IDs)
return IDs.takeError();
Req.IDs = std::move(*IDs);
@@ -100,7 +101,7 @@ Marshaller::fromProtobuf(const FuzzyFindRequest *Message) {
llvm::Expected<clangd::RefsRequest>
Marshaller::fromProtobuf(const RefsRequest *Message) {
clangd::RefsRequest Req;
- auto IDs = getIDs(Message);
+ auto IDs = getIDs(Message->ids());
if (!IDs)
return IDs.takeError();
Req.IDs = std::move(*IDs);
@@ -110,6 +111,19 @@ Marshaller::fromProtobuf(const RefsRequest *Message) {
return Req;
}
+llvm::Expected<clangd::RelationsRequest>
+Marshaller::fromProtobuf(const RelationsRequest *Message) {
+ clangd::RelationsRequest Req;
+ auto IDs = getIDs(Message->subjects());
+ if (!IDs)
+ return IDs.takeError();
+ Req.Subjects = std::move(*IDs);
+ Req.Predicate = static_cast<RelationKind>(Message->predicate());
+ 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 "
@@ -178,6 +192,24 @@ llvm::Optional<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
return Result;
}
+llvm::Optional<std::pair<clangd::SymbolID, clangd::Symbol>>
+Marshaller::fromProtobuf(const Relation &Message) {
+ auto SubjectID = SymbolID::fromStr(Message.subject_id());
+ if (!SubjectID) {
+ elog("Cannot convert Relation from protobuf (invalid Subject ID): {0}",
+ SubjectID.takeError());
+ return llvm::None;
+ }
+ if (!Message.has_object()) {
+ elog("Cannot convert Relation from protobuf (missing Object): {0}");
+ return llvm::None;
+ }
+ auto Object = fromProtobuf(Message.object());
+ if (!Object)
+ return llvm::None;
+ return std::make_pair(*SubjectID, *Object);
+}
+
LookupRequest Marshaller::toProtobuf(const clangd::LookupRequest &From) {
LookupRequest RPCRequest;
for (const auto &SymbolID : From.IDs)
@@ -216,6 +248,16 @@ RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) {
return 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<uint32_t>(From.Predicate));
+ if (From.Limit)
+ RPCRequest.set_limit(*From.Limit);
+ return RPCRequest;
+}
+
llvm::Optional<Symbol> Marshaller::toProtobuf(const clangd::Symbol &From) {
Symbol Result;
Result.set_id(From.ID.str());
@@ -274,6 +316,19 @@ llvm::Optional<Ref> Marshaller::toProtobuf(const clangd::Ref &From) {
return Result;
}
+llvm::Optional<Relation> Marshaller::toProtobuf(const clangd::SymbolID &Subject,
+ const clangd::Symbol &Object) {
+ Relation Result;
+ *Result.mutable_subject_id() = Subject.str();
+ auto SerializedObject = toProtobuf(Object);
+ if (!SerializedObject) {
+ elog("Can not convert Relation to protobuf (invalid symbol): {0}", Object);
+ return llvm::None;
+ }
+ *Result.mutable_object() = *SerializedObject;
+ return Result;
+}
+
llvm::Optional<std::string>
Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
assert(LocalIndexRoot);
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
index 5d82cdb7e765..9588f1ef6411 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,14 +38,19 @@ class Marshaller {
Marshaller() = delete;
Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
+ // FIXME(kirillbobyrev): Switch from Optional to Expected.
llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message);
llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message);
+ llvm::Optional<std::pair<clangd::SymbolID, clangd::Symbol>>
+ fromProtobuf(const Relation &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);
+ llvm::Expected<clangd::RelationsRequest>
+ fromProtobuf(const RelationsRequest *Message);
/// toProtobuf() functions serialize native clangd types and strip IndexRoot
/// from the file paths specific to indexing machine. fromProtobuf() functions
@@ -54,9 +59,12 @@ class Marshaller {
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::Optional<Ref> toProtobuf(const clangd::Ref &From);
llvm::Optional<Symbol> toProtobuf(const clangd::Symbol &From);
+ llvm::Optional<Ref> toProtobuf(const clangd::Ref &From);
+ llvm::Optional<Relation> 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
diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp
index c3f9efeb7ec0..077d58ea7db0 100644
--- a/clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -169,6 +169,38 @@ class RemoteIndexServer final : public SymbolIndex::Service {
return grpc::Status::OK;
}
+ grpc::Status Relations(grpc::ServerContext *Context,
+ const RelationsRequest *Request,
+ grpc::ServerWriter<RelationsReply> *Reply) override {
+ trace::Span Tracer(RelationsRequest::descriptor()->name());
+ auto Req = ProtobufMarshaller->fromProtobuf(Request);
+ if (!Req) {
+ elog("Can not parse RelationsRequest from protobuf: {0}",
+ Req.takeError());
+ return grpc::Status::CANCELLED;
+ }
+ unsigned Sent = 0;
+ unsigned FailedToSend = 0;
+ Index->relations(
+ *Req, [&](const SymbolID &Subject, const clangd::Symbol &Object) {
+ auto SerializedItem = ProtobufMarshaller->toProtobuf(Subject, Object);
+ if (!SerializedItem) {
+ ++FailedToSend;
+ return;
+ }
+ RelationsReply NextMessage;
+ *NextMessage.mutable_stream_result() = *SerializedItem;
+ Reply->Write(NextMessage);
+ ++Sent;
+ });
+ RelationsReply LastMessage;
+ LastMessage.set_final_result(true);
+ Reply->Write(LastMessage);
+ SPAN_ATTACH(Tracer, "Sent", Sent);
+ SPAN_ATTACH(Tracer, "Failed to send", FailedToSend);
+ return grpc::Status::OK;
+ }
+
std::unique_ptr<clangd::SymbolIndex> Index;
std::unique_ptr<Marshaller> ProtobufMarshaller;
};
diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index f975d1c35e1e..6abf16a8dd49 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -10,6 +10,7 @@
#include "TestFS.h"
#include "index/Index.h"
#include "index/Ref.h"
+#include "index/Relation.h"
#include "index/Serialization.h"
#include "index/Symbol.h"
#include "index/SymbolID.h"
@@ -18,6 +19,7 @@
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/StringSaver.h"
@@ -38,6 +40,51 @@ const char *testPathURI(llvm::StringRef Path,
return Strings.save(URI.toString()).begin();
}
+clangd::Symbol createSymbol(llvm::StringRef PathPrefix,
+ llvm::UniqueStringSaver &Strings) {
+ clangd::Symbol Sym;
+ Sym.ID = llvm::cantFail(SymbolID::fromStr("057557CEBF6E6B2D"));
+
+ index::SymbolInfo Info;
+ Info.Kind = index::SymbolKind::Function;
+ Info.SubKind = index::SymbolSubKind::AccessorGetter;
+ Info.Lang = index::SymbolLanguage::CXX;
+ Info.Properties = static_cast<index::SymbolPropertySet>(
+ index::SymbolProperty::TemplateSpecialization);
+ Sym.SymInfo = Info;
+
+ Sym.Name = Strings.save("Foo");
+ Sym.Scope = Strings.save("llvm::foo::bar::");
+
+ clangd::SymbolLocation Location;
+ Location.Start.setLine(1);
+ Location.Start.setColumn(15);
+ Location.End.setLine(3);
+ Location.End.setColumn(121);
+ Location.FileURI = testPathURI(PathPrefix.str() + "Definition.cpp", Strings);
+ Sym.Definition = Location;
+
+ Location.Start.setLine(42);
+ Location.Start.setColumn(31);
+ Location.End.setLine(20);
+ Location.End.setColumn(400);
+ Location.FileURI = testPathURI(PathPrefix.str() + "Declaration.h", Strings);
+ Sym.CanonicalDeclaration = Location;
+
+ Sym.References = 9000;
+ Sym.Origin = clangd::SymbolOrigin::Static;
+ Sym.Signature = Strings.save("(int X, char Y, Type T)");
+ Sym.TemplateSpecializationArgs = Strings.save("<int, char, bool, Type>");
+ Sym.CompletionSnippetSuffix =
+ Strings.save("({1: int X}, {2: char Y}, {3: Type T})");
+ Sym.Documentation = Strings.save("This is my amazing Foo constructor!");
+ Sym.ReturnType = Strings.save("Foo");
+
+ Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
+
+ return Sym;
+}
+
TEST(RemoteMarshallingTest, URITranslation) {
llvm::BumpPtrAllocator Arena;
llvm::UniqueStringSaver Strings(Arena);
@@ -86,52 +133,10 @@ TEST(RemoteMarshallingTest, URITranslation) {
}
TEST(RemoteMarshallingTest, SymbolSerialization) {
- clangd::Symbol Sym;
-
- auto ID = SymbolID::fromStr("057557CEBF6E6B2D");
- ASSERT_TRUE(bool(ID));
- Sym.ID = *ID;
-
- index::SymbolInfo Info;
- Info.Kind = index::SymbolKind::Function;
- Info.SubKind = index::SymbolSubKind::AccessorGetter;
- Info.Lang = index::SymbolLanguage::CXX;
- Info.Properties = static_cast<index::SymbolPropertySet>(
- index::SymbolProperty::TemplateSpecialization);
- Sym.SymInfo = Info;
-
llvm::BumpPtrAllocator Arena;
llvm::UniqueStringSaver Strings(Arena);
- Sym.Name = Strings.save("Foo");
- Sym.Scope = Strings.save("llvm::foo::bar::");
-
- clangd::SymbolLocation Location;
- Location.Start.setLine(1);
- Location.Start.setColumn(15);
- Location.End.setLine(3);
- Location.End.setColumn(121);
- Location.FileURI = testPathURI("home/Definition.cpp", Strings);
- Sym.Definition = Location;
-
- Location.Start.setLine(42);
- Location.Start.setColumn(31);
- Location.End.setLine(20);
- Location.End.setColumn(400);
- Location.FileURI = testPathURI("home/Declaration.h", Strings);
- Sym.CanonicalDeclaration = Location;
-
- Sym.References = 9000;
- Sym.Origin = clangd::SymbolOrigin::Static;
- Sym.Signature = Strings.save("(int X, char Y, Type T)");
- Sym.TemplateSpecializationArgs = Strings.save("<int, char, bool, Type>");
- Sym.CompletionSnippetSuffix =
- Strings.save("({1: int X}, {2: char Y}, {3: Type T})");
- Sym.Documentation = Strings.save("This is my amazing Foo constructor!");
- Sym.ReturnType = Strings.save("Foo");
-
- Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
-
+ clangd::Symbol Sym = createSymbol("home/", Strings);
Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/"));
// Check that symbols are exactly the same if the path to indexed project is
@@ -160,20 +165,17 @@ TEST(RemoteMarshallingTest, SymbolSerialization) {
EXPECT_FALSE(ProtobufMarshaller.fromProtobuf(*Serialized));
// Fail with an invalid URI.
- Location.FileURI = "Not A URI";
- Sym.Definition = Location;
+ Sym.Definition.FileURI = "Not A URI";
EXPECT_FALSE(ProtobufMarshaller.toProtobuf(Sym));
// Schemes other than "file" can not be used.
auto UnittestURI = URI::create(testPath("home/SomePath.h"), "unittest");
ASSERT_TRUE(bool(UnittestURI));
- Location.FileURI = Strings.save(UnittestURI->toString()).begin();
- Sym.Definition = Location;
+ Sym.Definition.FileURI = Strings.save(UnittestURI->toString()).begin();
EXPECT_FALSE(ProtobufMarshaller.toProtobuf(Sym));
// Passing root that is not prefix of the original file path.
- Location.FileURI = testPathURI("home/File.h", Strings);
- Sym.Definition = Location;
+ Sym.Definition.FileURI = testPathURI("home/File.h", Strings);
// Check that the symbol is valid and passing the correct path works.
Serialized = ProtobufMarshaller.toProtobuf(Sym);
ASSERT_TRUE(Serialized);
@@ -342,6 +344,55 @@ TEST(RemoteMarshallingTest, RefsRequestFailingSerialization) {
llvm::consumeError(Deserialized.takeError());
}
+TEST(RemoteMarshallingTest, RelationsRequestSerialization) {
+ clangd::RelationsRequest Request;
+ Request.Subjects.insert(
+ llvm::cantFail(SymbolID::fromStr("0000000000000001")));
+ Request.Subjects.insert(
+ llvm::cantFail(SymbolID::fromStr("0000000000000002")));
+
+ Request.Limit = 9000;
+ Request.Predicate = RelationKind::BaseOf;
+
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
+ EXPECT_EQ(static_cast<unsigned>(Serialized.subjects_size()),
+ Request.Subjects.size());
+ EXPECT_EQ(Serialized.limit(), Request.Limit);
+ EXPECT_EQ(static_cast<RelationKind>(Serialized.predicate()),
+ Request.Predicate);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ ASSERT_TRUE(bool(Deserialized));
+ EXPECT_EQ(Deserialized->Subjects, Request.Subjects);
+ ASSERT_TRUE(Deserialized->Limit);
+ EXPECT_EQ(*Deserialized->Limit, Request.Limit);
+ EXPECT_EQ(Deserialized->Predicate, Request.Predicate);
+}
+
+TEST(RemoteMarshallingTest, RelationsRequestFailingSerialization) {
+ RelationsRequest Serialized;
+ Serialized.add_subjects("ZZZZZZZZZZZZZZZZ");
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
+ EXPECT_FALSE(Deserialized);
+ llvm::consumeError(Deserialized.takeError());
+}
+
+TEST(RemoteMarshallingTest, RelationsSerializion) {
+ llvm::BumpPtrAllocator Arena;
+ llvm::UniqueStringSaver Strings(Arena);
+
+ clangd::Symbol Sym = createSymbol("remote/", Strings);
+ SymbolID ID = llvm::cantFail(SymbolID::fromStr("0000000000000002"));
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
+ auto Serialized = ProtobufMarshaller.toProtobuf(ID, Sym);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ ASSERT_TRUE(Deserialized);
+ EXPECT_THAT(Deserialized->first, ID);
+ EXPECT_THAT(Deserialized->second.ID, Sym.ID);
+}
+
TEST(RemoteMarshallingTest, RelativePathToURITranslation) {
Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"",
/*LocalIndexRoot=*/testPath("home/project/"));
More information about the cfe-commits
mailing list