kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

YAML serialization was used in the Proof of Concept for simplicity.
This patch replaces implements Protobuf (de) serialization of almost all
types that need to be transferred over the protocol.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79862

Files:
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  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

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
@@ -7,13 +7,90 @@
 //===----------------------------------------------------------------------===//
 
 #include "Marshalling.h"
+#include "Index.pb.h"
+#include "Protocol.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
+#include "index/SymbolID.h"
+#include "index/SymbolLocation.h"
+#include "index/SymbolOrigin.h"
 #include "support/Logger.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/StringSaver.h"
 
 namespace clang {
 namespace clangd {
 namespace remote {
 
+namespace {
+
+clangd::SymbolLocation::Position fromProtobuf(const Position &Message) {
+  clangd::SymbolLocation::Position Result;
+  Result.setColumn(static_cast<uint32_t>(Message.column()));
+  Result.setLine(static_cast<uint32_t>(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<clang::index::SymbolKind>(Message.kind());
+  Result.SubKind = static_cast<clang::index::SymbolSubKind>(Message.subkind());
+  Result.Lang = static_cast<clang::index::SymbolLanguage>(Message.language());
+  Result.Properties =
+      static_cast<clang::index::SymbolPropertySet>(Message.properties());
+  return Result;
+}
+
+SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info) {
+  SymbolInfo Result;
+  Result.set_kind(static_cast<uint32_t>(Info.Kind));
+  Result.set_subkind(static_cast<uint32_t>(Info.SubKind));
+  Result.set_language(static_cast<uint32_t>(Info.Lang));
+  Result.set_properties(static_cast<uint32_t>(Info.Properties));
+  return Result;
+}
+
+clangd::SymbolLocation fromProtobuf(const SymbolLocation &Message,
+                                    llvm::UniqueStringSaver *Strings) {
+  clangd::SymbolLocation Location;
+  Location.Start = fromProtobuf(Message.start());
+  Location.End = fromProtobuf(Message.end());
+  Location.FileURI = Strings->save(Message.file_uri()).begin();
+  return Location;
+}
+
+SymbolLocation toProtobuf(const clangd::SymbolLocation &Location) {
+  remote::SymbolLocation Result;
+  *Result.mutable_start() = toProtobuf(Location.Start);
+  *Result.mutable_end() = toProtobuf(Location.End);
+  *Result.mutable_file_uri() = Location.FileURI;
+  return Result;
+}
+
+HeaderWithReferences
+toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) {
+  HeaderWithReferences Result;
+  Result.set_header(IncludeHeader.IncludeHeader.str());
+  Result.set_references(IncludeHeader.References);
+  return Result;
+}
+
+clangd::Symbol::IncludeHeaderWithReferences
+fromProtobuf(const HeaderWithReferences &Message) {
+  return clangd::Symbol::IncludeHeaderWithReferences{Message.header(),
+                                                     Message.references()};
+}
+
+} // namespace
+
 clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request) {
   clangd::FuzzyFindRequest Result;
   Result.Query = Request->query();
@@ -22,7 +99,7 @@
   Result.AnyScope = Request->any_scope();
   if (Request->limit())
     Result.Limit = Request->limit();
-  Result.RestrictForCodeCompletion = Request->resricted_for_code_completion();
+  Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
   for (const auto &Path : Request->proximity_paths())
     Result.ProximityPaths.push_back(Path);
   for (const auto &Type : Request->preferred_types())
@@ -32,21 +109,50 @@
 
 llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message,
                                             llvm::UniqueStringSaver *Strings) {
-  auto Result = symbolFromYAML(Message.yaml_serialization(), Strings);
-  if (!Result) {
-    elog("Cannot convert Symbol from Protobuf: {}", Result.takeError());
+  if (!Message.has_info() || !Message.has_definition() ||
+      !Message.has_canonical_declarattion()) {
+    elog("Cannot convert Symbol from Protobuf: {}", Message.ShortDebugString());
     return llvm::None;
   }
-  return *Result;
+  clangd::Symbol Result;
+  auto ID = SymbolID::fromStr(Message.id());
+  if (!ID) {
+    elog("Cannot convert parse SymbolID {} from Protobuf: {}", ID.takeError(),
+         Message.ShortDebugString());
+    return llvm::None;
+  }
+  Result.ID = *ID;
+  Result.SymInfo = fromProtobuf(Message.info());
+  Result.Name = Message.name();
+  Result.Scope = Message.scope();
+  Result.Definition = fromProtobuf(Message.definition(), Strings);
+  Result.CanonicalDeclaration =
+      fromProtobuf(Message.canonical_declarattion(), Strings);
+  Result.References = Message.references();
+  Result.Origin = static_cast<clangd::SymbolOrigin>(Message.origin());
+  Result.Signature = Message.signature();
+  Result.TemplateSpecializationArgs = Message.template_specialization_args();
+  Result.CompletionSnippetSuffix = Message.completion_snippet_suffix();
+  Result.Documentation = Message.documentation();
+  Result.ReturnType = Message.return_type();
+  Result.Type = Message.type();
+  for (const auto &Header : Message.headers()) {
+    Result.IncludeHeaders.push_back(fromProtobuf(Header));
+  }
+  Result.Flags = static_cast<clangd::Symbol::SymbolFlag>(Message.flags());
+  return Result;
 }
+
 llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message,
                                          llvm::UniqueStringSaver *Strings) {
-  auto Result = refFromYAML(Message.yaml_serialization(), Strings);
-  if (!Result) {
-    elog("Cannot convert Ref from Protobuf: {}", Result.takeError());
+  if (!Message.has_location()) {
+    elog("Cannot convert Ref from Protobuf: {}", Message.ShortDebugString());
     return llvm::None;
   }
-  return *Result;
+  clangd::Ref Result;
+  Result.Location = fromProtobuf(Message.location(), Strings);
+  Result.Kind = static_cast<clangd::RefKind>(Message.kind());
+  return Result;
 }
 
 LookupRequest toProtobuf(const clangd::LookupRequest &From) {
@@ -64,7 +170,7 @@
   RPCRequest.set_any_scope(From.AnyScope);
   if (From.Limit)
     RPCRequest.set_limit(*From.Limit);
-  RPCRequest.set_resricted_for_code_completion(From.RestrictForCodeCompletion);
+  RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
   for (const auto &Path : From.ProximityPaths)
     RPCRequest.add_proximity_paths(Path);
   for (const auto &Type : From.PreferredTypes)
@@ -84,13 +190,34 @@
 
 Symbol toProtobuf(const clangd::Symbol &From) {
   Symbol Result;
-  Result.set_yaml_serialization(toYAML(From));
+  Result.set_id(From.ID.str());
+  *Result.mutable_info() = toProtobuf(From.SymInfo);
+  Result.set_name(From.Name.str());
+  *Result.mutable_definition() = toProtobuf(From.Definition);
+  Result.set_scope(From.Scope.str());
+  *Result.mutable_canonical_declarattion() =
+      toProtobuf(From.CanonicalDeclaration);
+  Result.set_references(From.References);
+  Result.set_origin(static_cast<uint32_t>(From.Origin));
+  Result.set_signature(From.Signature.str());
+  Result.set_template_specialization_args(
+      From.TemplateSpecializationArgs.str());
+  Result.set_completion_snippet_suffix(From.CompletionSnippetSuffix.str());
+  Result.set_documentation(From.Documentation.str());
+  Result.set_return_type(From.ReturnType.str());
+  Result.set_type(From.Type.str());
+  for (const auto &Header : From.IncludeHeaders) {
+    auto *NextHeader = Result.add_headers();
+    *NextHeader = toProtobuf(Header);
+  }
+  Result.set_flags(static_cast<uint32_t>(From.Flags));
   return Result;
 }
 
 Ref toProtobuf(const clangd::Ref &From) {
   Ref Result;
-  Result.set_yaml_serialization(toYAML(From));
+  Result.set_kind(static_cast<uint32_t>(From.Kind));
+  *Result.mutable_location() = toProtobuf(From.Location);
   return Result;
 }
 
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,7 +18,9 @@
   rpc Refs(RefsRequest) returns (stream RefsReply) {}
 }
 
-message LookupRequest { repeated string ids = 1; }
+message LookupRequest {
+  repeated string ids = 1;
+}
 
 // The response is a stream of symbol messages and the terminating message
 // indicating the end of stream.
@@ -34,7 +36,7 @@
   repeated string scopes = 2;
   bool any_scope = 3;
   uint32 limit = 4;
-  bool resricted_for_code_completion = 5;
+  bool restricted_for_code_completion = 5;
   repeated string proximity_paths = 6;
   repeated string preferred_types = 7;
 }
@@ -44,7 +46,7 @@
 message FuzzyFindReply {
   oneof kind {
     Symbol stream_result = 1;
-    bool final_result = 2; // HasMore
+    bool final_result = 2;  // HasMore
   }
 }
 
@@ -59,11 +61,53 @@
 message RefsReply {
   oneof kind {
     Ref stream_result = 1;
-    bool final_result = 2; // HasMore
+    bool final_result = 2;  // HasMore
   }
 }
 
-// FIXME(kirillbobyrev): Properly serialize symbols and refs instead of passing
-// YAML.
-message Ref { string yaml_serialization = 1; }
-message Symbol { string yaml_serialization = 1; }
+message Symbol {
+  string id = 1;
+  SymbolInfo info = 2;
+  string name = 3;
+  SymbolLocation definition = 4;
+  string scope = 5;
+  SymbolLocation canonical_declarattion = 6;
+  int32 references = 7;
+  uint32 origin = 8;
+  string signature = 9;
+  string template_specialization_args = 10;
+  string completion_snippet_suffix = 11;
+  string documentation = 12;
+  string return_type = 13;
+  string type = 14;
+  repeated HeaderWithReferences headers = 15;
+  uint32 flags = 16;
+}
+
+message Ref {
+  SymbolLocation location = 1;
+  uint32 kind = 2;
+}
+
+message SymbolInfo {
+  uint32 kind = 1;
+  uint32 subkind = 2;
+  uint32 language = 3;
+  uint32 properties = 4;
+}
+
+message SymbolLocation {
+  Position start = 1;
+  Position end = 2;
+  string file_uri = 3;
+}
+
+message Position {
+  uint32 line = 1;
+  uint32 column = 2;
+}
+
+message HeaderWithReferences {
+  string header = 1;
+  uint32 references = 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
@@ -46,8 +46,7 @@
       }
       auto Sym = fromProtobuf(Reply.stream_result(), &Strings);
       if (!Sym)
-        elog("Received invalid {0}: {1}", ReplyT::descriptor()->name(),
-             Reply.stream_result().yaml_serialization());
+        elog("Received invalid {0}", ReplyT::descriptor()->name());
       Callback(*Sym);
     }
     SPAN_ATTACH(Tracer, "status", Reader->Finish().ok());
Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -517,17 +517,6 @@
   return Buf;
 }
 
-std::string toYAML(const Ref &R) {
-  std::string Buf;
-  {
-    llvm::raw_string_ostream OS(Buf);
-    llvm::yaml::Output Yout(OS);
-    Ref Reference = R; // copy: Yout<< requires mutability.
-    Yout << Reference;
-  }
-  return Buf;
-}
-
 llvm::Expected<clangd::Symbol>
 symbolFromYAML(StringRef YAML, llvm::UniqueStringSaver *Strings) {
   clangd::Symbol Deserialized;
@@ -540,17 +529,5 @@
   return Deserialized;
 }
 
-llvm::Expected<clangd::Ref> refFromYAML(StringRef YAML,
-                                        llvm::UniqueStringSaver *Strings) {
-  clangd::Ref Deserialized;
-  llvm::yaml::Input YAMLInput(YAML, Strings);
-  if (YAMLInput.error())
-    return llvm::make_error<llvm::StringError>(
-        llvm::formatv("Unable to deserialize Symbol from YAML: {0}", YAML),
-        llvm::inconvertibleErrorCode());
-  YAMLInput >> Deserialized;
-  return Deserialized;
-}
-
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/Serialization.h
===================================================================
--- clang-tools-extra/clangd/index/Serialization.h
+++ clang-tools-extra/clangd/index/Serialization.h
@@ -77,13 +77,10 @@
 std::string toYAML(const Symbol &);
 std::string toYAML(const std::pair<SymbolID, ArrayRef<Ref>> &);
 std::string toYAML(const Relation &);
-std::string toYAML(const Ref &);
 
 // Deserialize a single symbol from YAML.
 llvm::Expected<clangd::Symbol> symbolFromYAML(StringRef YAML,
                                               llvm::UniqueStringSaver *Strings);
-llvm::Expected<clangd::Ref> refFromYAML(StringRef YAML,
-                                        llvm::UniqueStringSaver *Strings);
 
 // Build an in-memory static index from an index file.
 // The size should be relatively small, so data can be managed in memory.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to