[clangd-remote] Replace YAML serialization with proper Protobuf messages

[kbobyrev]

Summary

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.

[sammccall]

It's tedious, but we really should have tests for this now.

clang-tools-extra/clangd/index/remote/Index.proto

68–113

somewhere in this file there should be a hint that semantics of all fields matches those in the structs in Index/

70

sigh, I wish we'd managed to get rid of this struct by now :-(
Maybe we can still do it before the 11 release

[kbobyrev]

Add tests for Protobuf (de)serialization.

[kbobyrev]

Simplify test setup and rebase on top of master.

[sammccall]

clang-tools-extra/clangd/unittests/TestTU.h

1

This header provides a widely used abstraction and has a clear scope, please don't dump things here.

94

I don't think the API of this class has been thought through well enough to share.

Why do you need it? Can't you just use TestTU::headerSymbols() (if you need Refs and such as well, just copy its implementation - it's 2 lines long)

[kbobyrev]

Don't complicate SymbolCollectorTest, use TestTU.

[kbobyrev]

Inline HeaderCode and MainCode.

[kbobyrev]

Rebase on top of master.

[sammccall]

Just more stuff about simplifying the tests :-(

This is really just about converting between a struct and a proto, if we have to add indexing-related fixtures to shared libraries something seems to have gone a bit off the rails.
In the worst case we could just create a few symbols with fields populated by hand, it's probably easier to see what parts of the code are being covered by the test...

clang-tools-extra/clangd/unittests/TestTU.cpp

128

can you structure the tests to avoid doing this (untested) merge?

Why do we need the example symbols split over multiple files at all?
If we need such symbols can we just construct them manually instead?

clang-tools-extra/clangd/unittests/TestTU.h

78

please don't add these for a single test, we can do this inline in the test unless it becomes a common thing

clang-tools-extra/clangd/unittests/remote/CMakeLists.txt

3

Can we conditionally include these tests into the main ClangdTests, instead of setting up another set of targets?

19

I don't think including the same source files in multiple targets is a great path to go down.

clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

21

Using inheritance to define test *cases* is a bit of a trap, and not particularly idiomatic.
Can we just write a function indexSampleData() that returns a SlabTuple, and call it from the tests?

63

similarly i'd consider just inlining the stringsaver/arena into the tests, seems easier to read

67

if you want to define the symbols indirectly like this, you probably also want to assert that there are more than 5 or something, to guard against the test fixture being broken

[kbobyrev]

Address some comments.

clang-tools-extra/clangd/unittests/remote/CMakeLists.txt

3

I can do that, although I saw that XPC has their own set of tests if I am correct, so I thought this should be OK.

Conditionally including tests into the main ClangdTests is not super easy because... CMake stuff. But I. can look into that.

[kbobyrev]

Move remote tests into all Clangd tests

clang-tools-extra/clangd/unittests/remote/CMakeLists.txt

3

Actually, nevermind, it looks pretty straightforward.

[kbobyrev]

Move a comment to the line it corresponds to.

[sammccall]

Thanks! Sorry about the hassle with the tests.

clang-tools-extra/clangd/unittests/remote/CMakeLists.txt

3

Yeah we have to pay the cost of *some* cmake stuff either way, this looks like a bit less work.

XPC... yeah, I wish there was less CMake stuff going on there. But in practice, the majority of maintenance in the two dirs has been disjoint sets of people, so it has its advantages to...

[kbobyrev]

In D79862#2044073, @sammccall wrote:

Thanks! Sorry about the hassle with the tests.

No worries, I think it looks cleaner and simpler which is important.

Thanks for the review!

clang-tools-extra/clangd/index/remote/Client.cpp

bool streamRPC(ClangdRequestT Request,
     ReplyT Reply;
     while (Reader->Read(&Reply)) {
       if (!Reply.has_stream_result()) {
         FinalResult = Reply.final_result();
         continue;
       }
       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());
     return FinalResult;
   }
 
 public:
   IndexClient(std::shared_ptr<grpc::Channel> Channel)

clang-tools-extra/clangd/index/remote/Index.proto

 //===--- Index.proto - Remote index Protocol Buffers definition -----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 syntax = "proto3";
 
 package clang.clangd.remote;
 
+// Semantics of SymbolIndex match clangd::SymbolIndex with all required
+// structures corresponding to their clangd::* counterparts.
 service SymbolIndex {
   rpc Lookup(LookupRequest) returns (stream LookupReply) {}
 
   rpc FuzzyFind(FuzzyFindRequest) returns (stream FuzzyFindReply) {}
 
   rpc Refs(RefsRequest) returns (stream RefsReply) {}
 }
 
 message LookupRequest { repeated string ids = 1; }
 
 // The response is a stream of symbol messages and the terminating message
 // indicating the end of stream.
 message LookupReply {
   oneof kind {
     Symbol stream_result = 1;
     bool final_result = 2;
   }
 }
 
 message FuzzyFindRequest {
   string query = 1;
   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;
 }
 
 // The response is a stream of symbol messages, and one terminating has_more
 // message.
 message FuzzyFindReply {
   oneof kind {

 // message.
 message RefsReply {
   oneof kind {
     Ref stream_result = 1;
     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;
+}

clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp

 //===--- Marshalling.cpp -----------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #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();
   for (const auto &Scope : Request->scopes())
     Result.Scopes.push_back(Scope);
   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())
     Result.ProximityPaths.push_back(Type);
   return Result;
 }
 
 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;
+  }
+  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;
   }
-  return *Result;
+  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) {
   LookupRequest RPCRequest;
   for (const auto &SymbolID : From.IDs)
     RPCRequest.add_ids(SymbolID.str());
   return RPCRequest;
 }
 
 FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From) {
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)
     RPCRequest.add_scopes(Scope);
   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)
     RPCRequest.add_preferred_types(Type);
   return RPCRequest;
 }
 
 RefsRequest toProtobuf(const clangd::RefsRequest &From) {
   RefsRequest RPCRequest;
   for (const auto &ID : From.IDs)
     RPCRequest.add_ids(ID.str());
   RPCRequest.set_filter(static_cast<uint32_t>(From.Filter));
   if (From.Limit)
     RPCRequest.set_limit(*From.Limit);
   return RPCRequest;
 }
 
 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;
 }
 
 } // namespace remote
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/CMakeLists.txt

target_link_libraries(ClangdTests
   LLVMSupport
   LLVMTestingSupport
   )
 
 if (CLANGD_BUILD_XPC)
   add_subdirectory(xpc)
 endif ()
 
+if (CLANGD_ENABLE_REMOTE)
+  add_subdirectory(remote)
+endif()
+
 configure_lit_site_cfg(
   ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
   ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py)

clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

namespace nx {
         Kind_Fine
       };
     }
   )");
   EXPECT_TRUE(shouldCollect("nx::Top_Level"));
   EXPECT_TRUE(shouldCollect("nx::Kind_Fine"));
 }
 
-class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
-public:
-  SymbolIndexActionFactory(SymbolCollector::Options COpts,
-                           CommentHandler *PragmaHandler)
-      : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
-
-  std::unique_ptr<FrontendAction> create() override {
-    class IndexAction : public ASTFrontendAction {
-    public:
-      IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
-                  const index::IndexingOptions &Opts,
-                  CommentHandler *PragmaHandler)
-          : DataConsumer(std::move(DataConsumer)), Opts(Opts),
-            PragmaHandler(PragmaHandler) {}
-
-      std::unique_ptr<ASTConsumer>
-      CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
-        if (PragmaHandler)
-          CI.getPreprocessor().addCommentHandler(PragmaHandler);
-        return createIndexingASTConsumer(DataConsumer, Opts,
-                                         CI.getPreprocessorPtr());
-      }
-
-      bool BeginInvocation(CompilerInstance &CI) override {
-        // Make the compiler parse all comments.
-        CI.getLangOpts().CommentOpts.ParseAllComments = true;
-        return true;
-      }
-
-    private:
-      std::shared_ptr<index::IndexDataConsumer> DataConsumer;
-      index::IndexingOptions Opts;
-      CommentHandler *PragmaHandler;
-    };
-    index::IndexingOptions IndexOpts;
-    IndexOpts.SystemSymbolFilter =
-        index::IndexingOptions::SystemSymbolFilterKind::All;
-    IndexOpts.IndexFunctionLocals = false;
-    Collector = std::make_shared<SymbolCollector>(COpts);
-    return std::make_unique<IndexAction>(Collector, std::move(IndexOpts),
-                                         PragmaHandler);
-  }
-
-  std::shared_ptr<SymbolCollector> Collector;
-  SymbolCollector::Options COpts;
-  CommentHandler *PragmaHandler;
-};
-
-class SymbolCollectorTest : public ::testing::Test {
-public:
-  SymbolCollectorTest()
-      : InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
-        TestHeaderName(testPath("symbol.h")),
-        TestFileName(testPath("symbol.cc")) {
-    TestHeaderURI = URI::create(TestHeaderName).toString();
-    TestFileURI = URI::create(TestFileName).toString();
-  }
-
-  // Note that unlike TestTU, no automatic header guard is added.
-  // HeaderCode should start with #pragma once to be treated as modular.
-  bool runSymbolCollector(llvm::StringRef HeaderCode, llvm::StringRef MainCode,
-                          const std::vector<std::string> &ExtraArgs = {}) {
-    llvm::IntrusiveRefCntPtr<FileManager> Files(
-        new FileManager(FileSystemOptions(), InMemoryFileSystem));
-
-    auto Factory = std::make_unique<SymbolIndexActionFactory>(
-        CollectorOpts, PragmaHandler.get());
-
-    std::vector<std::string> Args = {"symbol_collector", "-fsyntax-only",
-                                     "-xc++", "-include", TestHeaderName};
-    Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
-    // This allows to override the "-xc++" with something else, i.e.
-    // -xobjective-c++.
-    Args.push_back(TestFileName);
-
-    tooling::ToolInvocation Invocation(
-        Args, Factory->create(), Files.get(),
-        std::make_shared<PCHContainerOperations>());
-
-    InMemoryFileSystem->addFile(TestHeaderName, 0,
-                                llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-    InMemoryFileSystem->addFile(TestFileName, 0,
-                                llvm::MemoryBuffer::getMemBuffer(MainCode));
-    Invocation.run();
-    Symbols = Factory->Collector->takeSymbols();
-    Refs = Factory->Collector->takeRefs();
-    Relations = Factory->Collector->takeRelations();
-    return true;
-  }
-
-protected:
-  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem;
-  std::string TestHeaderName;
-  std::string TestHeaderURI;
-  std::string TestFileName;
-  std::string TestFileURI;
-  SymbolSlab Symbols;
-  RefSlab Refs;
-  RelationSlab Relations;
-  SymbolCollector::Options CollectorOpts;
-  std::unique_ptr<CommentHandler> PragmaHandler;
-};
-
 TEST_F(SymbolCollectorTest, CollectSymbols) {
   const std::string Header = R"(
     class Foo {
       Foo() {}
       Foo(int a) {}
       void f();
       friend void f1();
       friend class Friend;

 }
 
 TEST_F(SymbolCollectorTest, SpelledReferences) {
   struct {
     llvm::StringRef Header;
     llvm::StringRef Main;
     llvm::StringRef TargetSymbolName;
   } TestCases[] = {
       {
           R"cpp(
         struct Foo;
         #define MACRO Foo
       )cpp",
           R"cpp(
         struct $spelled[[Foo]] {
           $spelled[[Foo]]();
           ~$spelled[[Foo]]();
         };
         $spelled[[Foo]] Variable1;
         $implicit[[MACRO]] Variable2;
       )cpp",
           "Foo",
       },
       {
           R"cpp(
         class Foo {
         public:
           Foo() = default;
         };
       )cpp",
           R"cpp(
         void f() { Foo $implicit[[f]]; f = $spelled[[Foo]]();}
       )cpp",
           "Foo::Foo" /// constructor.
       },
   };
   CollectorOpts.RefFilter = RefKind::All;
   CollectorOpts.RefsInHeaders = false;
-  for (const auto& T : TestCases) {
+  for (const auto &T : TestCases) {
     Annotations Header(T.Header);
     Annotations Main(T.Main);
     // Reset the file system.
     InMemoryFileSystem = new llvm::vfs::InMemoryFileSystem;
     runSymbolCollector(Header.code(), Main.code());
 
     const auto SpelledRanges = Main.ranges("spelled");
     const auto ImplicitRanges = Main.ranges("implicit");

clang-tools-extra/clangd/unittests/TestTU.h

 //===--- TestTU.h - Scratch source files for testing -------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Many tests for indexing, code completion etc are most naturally expressed
 // using code examples.
 // TestTU lets test define these examples in a common way without dealing with
 // the mechanics of VFS and compiler interactions, and then easily grab the
 // AST, particular symbols, etc.
 //
 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
 
 #include "Compiler.h"
 #include "ParsedAST.h"
+#include "TestFS.h"
 #include "index/Index.h"
+#include "index/SymbolCollector.h"
 #include "support/Path.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Index/IndexingOptions.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/StringMap.h"
 #include "gtest/gtest.h"
 #include <string>
 #include <utility>
 #include <vector>
 
 namespace clang {
 namespace clangd {
struct TestTU {
   // Simulate a header guard of the header (using an #import directive).
   bool ImplicitHeaderGuard = true;
 
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   ParsedAST build() const;
   ParseInputs inputs() const;
   SymbolSlab headerSymbols() const;
   std::unique_ptr<SymbolIndex> index() const;
 };
 
+class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
+public:
+  SymbolIndexActionFactory(SymbolCollector::Options COpts,
+                           CommentHandler *PragmaHandler)
+      : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
+
+  std::unique_ptr<FrontendAction> create() override;
+
+  std::shared_ptr<SymbolCollector> Collector;
+  SymbolCollector::Options COpts;
+  CommentHandler *PragmaHandler;
+};
+
+class SymbolCollectorTest : public ::testing::Test {
+public:
+  SymbolCollectorTest()
+      : InMemoryFileSystem(new llvm::vfs::InMemoryFileSystem),
+        TestHeaderName(testPath("symbol.h")),
+        TestFileName(testPath("symbol.cc")) {
+    TestHeaderURI = URI::create(TestHeaderName).toString();
+    TestFileURI = URI::create(TestFileName).toString();
+  }
+
+  // Note that unlike TestTU, no automatic header guard is added.
+  // HeaderCode should start with #pragma once to be treated as modular.
+  bool runSymbolCollector(llvm::StringRef HeaderCode, llvm::StringRef MainCode,
+                          const std::vector<std::string> &ExtraArgs = {});
+
+protected:
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem;
+  std::string TestHeaderName;
+  std::string TestHeaderURI;
+  std::string TestFileName;
+  std::string TestFileURI;
+  SymbolSlab Symbols;
+  RefSlab Refs;
+  RelationSlab Relations;
+  SymbolCollector::Options CollectorOpts;
+  std::unique_ptr<CommentHandler> PragmaHandler;
+};
+
 // Look up an index symbol by qualified name, which must be unique.
 const Symbol &findSymbol(const SymbolSlab &, llvm::StringRef QName);
 // Look up an AST symbol by qualified name, which must be unique and top-level.
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName);
 // Look up an AST symbol that satisfies \p Filter.
 const NamedDecl &findDecl(ParsedAST &AST,
                           std::function<bool(const NamedDecl &)> Filter);
 // Look up an AST symbol by unqualified name, which must be unique.
 const NamedDecl &findUnqualifiedDecl(ParsedAST &AST, llvm::StringRef Name);
 
 } // namespace clangd
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H

clang-tools-extra/clangd/unittests/TestTU.cpp

Idx->updatePreamble(testPath(Filename), /*Version=*/"null",
                       AST.getASTContext(), AST.getPreprocessorPtr(),
                       AST.getCanonicalIncludes());
   Idx->updateMain(testPath(Filename), AST);
   return std::move(Idx);
 }
 
 const Symbol &findSymbol(const SymbolSlab &Slab, llvm::StringRef QName) {
   const Symbol *Result = nullptr;
   for (const Symbol &S : Slab) {
     if (QName != (S.Scope + S.Name).str())
       continue;
     if (Result) {
       ADD_FAILURE() << "Multiple symbols named " << QName << ":\n"
                     << *Result << "\n---\n"
                     << S;
       assert(false && "QName is not unique");
     }
const NamedDecl &findUnqualifiedDecl(ParsedAST &AST, llvm::StringRef Name) {
   return findDecl(AST, [Name](const NamedDecl &ND) {
     if (auto *ID = ND.getIdentifier())
       if (ID->getName() == Name)
         return true;
     return false;
   });
 }
 
+std::unique_ptr<FrontendAction> SymbolIndexActionFactory::create() {
+  class IndexAction : public ASTFrontendAction {
+  public:
+    IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
+                const index::IndexingOptions &Opts,
+                CommentHandler *PragmaHandler)
+        : DataConsumer(std::move(DataConsumer)), Opts(Opts),
+          PragmaHandler(PragmaHandler) {}
+
+    std::unique_ptr<ASTConsumer>
+    CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
+      if (PragmaHandler)
+        CI.getPreprocessor().addCommentHandler(PragmaHandler);
+      return createIndexingASTConsumer(DataConsumer, Opts,
+                                       CI.getPreprocessorPtr());
+    }
+
+    bool BeginInvocation(CompilerInstance &CI) override {
+      // Make the compiler parse all comments.
+      CI.getLangOpts().CommentOpts.ParseAllComments = true;
+      return true;
+    }
+
+  private:
+    std::shared_ptr<index::IndexDataConsumer> DataConsumer;
+    index::IndexingOptions Opts;
+    CommentHandler *PragmaHandler;
+  };
+  index::IndexingOptions IndexOpts;
+  IndexOpts.SystemSymbolFilter =
+      index::IndexingOptions::SystemSymbolFilterKind::All;
+  IndexOpts.IndexFunctionLocals = false;
+  Collector = std::make_shared<SymbolCollector>(COpts);
+  return std::make_unique<IndexAction>(Collector, std::move(IndexOpts),
+                                       PragmaHandler);
+}
+
+bool SymbolCollectorTest::runSymbolCollector(
+    llvm::StringRef HeaderCode, llvm::StringRef MainCode,
+    const std::vector<std::string> &ExtraArgs) {
+  llvm::IntrusiveRefCntPtr<FileManager> Files(
+      new FileManager(FileSystemOptions(), InMemoryFileSystem));
+
+  auto Factory = std::make_unique<SymbolIndexActionFactory>(
+      CollectorOpts, PragmaHandler.get());
+
+  std::vector<std::string> Args = {"symbol_collector", "-fsyntax-only", "-xc++",
+                                   "-include", TestHeaderName};
+  Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+  // This allows to override the "-xc++" with something else, i.e.
+  // -xobjective-c++.
+  Args.push_back(TestFileName);
+
+  tooling::ToolInvocation Invocation(
+      Args, Factory->create(), Files.get(),
+      std::make_shared<PCHContainerOperations>());
+
+  InMemoryFileSystem->addFile(TestHeaderName, 0,
+                              llvm::MemoryBuffer::getMemBuffer(HeaderCode));
+  InMemoryFileSystem->addFile(TestFileName, 0,
+                              llvm::MemoryBuffer::getMemBuffer(MainCode));
+  Invocation.run();
+  Symbols = Factory->Collector->takeSymbols();
+  Refs = Factory->Collector->takeRefs();
+  Relations = Factory->Collector->takeRelations();
+  return true;
+}
+
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/unittests/remote/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS
+  Support
+  )
+
+get_filename_component(CLANGD_SOURCE_DIR
+  ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH)
+include_directories(
+  ${CLANGD_SOURCE_DIR}
+  )
+
+include_directories(${CMAKE_CURRENT_BINARY_DIR}/../../index/remote)
+add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI=1)
+
+add_custom_target(ClangdRemoteUnitTests)
+add_unittest(ClangdRemoteUnitTests ClangdRemoteTests
+  MarshallingTests.cpp
+
+  ../TestTU.cpp
+  ../TestFS.cpp
+  )
+
+target_link_libraries(ClangdRemoteTests
+  PRIVATE
+  clangDaemon
+  clangdRemoteMarshalling
+  )

clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp

+//===--- MarshallingTests.cpp ------------------------------------*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../TestTU.h"
+#include "index/Serialization.h"
+#include "index/remote/marshalling/Marshalling.h"
+#include "llvm/Support/StringSaver.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace remote {
+namespace {
+
+class RemoteMarshallingTest : public SymbolCollectorTest {
+public:
+  RemoteMarshallingTest() : Strings(NewArena) {
+    const std::string Header = R"(
+    // This is a class.
+    class Foo {
+    public:
+      Foo();
+
+      int Bar;
+    private:
+      double Number;
+    };
+    /// This is a function.
+    char baz();
+    template <typename T>
+    T getT ();
+    struct FooBar {};
+    )";
+    const std::string Main = R"(
+    const int GlobalVariable = 123;
+
+    class ForwardDecl;
+    char baz() { return 'x'; }
+    template <typename T>
+    T getT() { return T(); }
+    Foo::Foo() : Bar(GlobalVariable), Number(42) {
+      for (int Counter = 0; Counter < GlobalVariable; ++Counter) {
+        baz();
+        getT<FooBar>();
+      }
+    }
+    )";
+    SymbolCollectorTest::runSymbolCollector(Header, Main);
+  }
+
+  llvm::UniqueStringSaver Strings;
+
+private:
+  llvm::BumpPtrAllocator NewArena;
+};
+
+TEST_F(RemoteMarshallingTest, SymbolSerialization) {
+  for (auto &Sym : Symbols) {
+    const auto ProtobufMeessage = toProtobuf(Sym);
+    const auto SymToProtobufAndBack = fromProtobuf(ProtobufMeessage, &Strings);
+    EXPECT_TRUE(SymToProtobufAndBack.hasValue());
+    EXPECT_EQ(toYAML(Sym), toYAML(*SymToProtobufAndBack));
+  }
+}
+
+TEST_F(RemoteMarshallingTest, ReferenceSerialization) {
+  for (const auto &SymbolWithRefs : Refs) {
+    for (const auto &Ref : SymbolWithRefs.second) {
+      const auto RefToProtobufAndBack = fromProtobuf(toProtobuf(Ref), &Strings);
+      EXPECT_TRUE(RefToProtobufAndBack.hasValue());
+      EXPECT_EQ(toYAML(Ref), toYAML(*RefToProtobufAndBack));
+    }
+  }
+}
+
+} // namespace
+} // namespace remote
+} // namespace clangd
+} // namespace clang