kbobyrev updated this revision to Diff 281869.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.
Address post-LGTM comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84894/new/
https://reviews.llvm.org/D84894
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/index/remote/marshalling/Marshalling.h
clang-tools-extra/clangd/index/remote/server/Server.cpp
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ 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 @@
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, 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 @@
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 @@
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/"));
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===================================================================
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -169,6 +169,38 @@
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;
};
Index: clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
===================================================================
--- clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,14 +38,19 @@
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 @@
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
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
@@ -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 {
-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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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);
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
@@ -18,6 +18,8 @@
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 @@
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;
+}
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
@@ -91,11 +91,16 @@
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
