kbobyrev updated this revision to Diff 276647.
kbobyrev added a comment.

Replace Sym in Client response with a more generic name (it can be a reference).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82938/new/

https://reviews.llvm.org/D82938

Files:
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/index/remote/Client.h
  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/index/remote/unimplemented/UnimplementedClient.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
@@ -7,85 +7,257 @@
 //===----------------------------------------------------------------------===//
 
 #include "../TestTU.h"
+#include "TestFS.h"
+#include "index/Index.h"
+#include "index/Ref.h"
 #include "index/Serialization.h"
+#include "index/Symbol.h"
+#include "index/SymbolID.h"
 #include "index/remote/marshalling/Marshalling.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include <cstring>
 
 namespace clang {
 namespace clangd {
 namespace remote {
 namespace {
 
+using llvm::sys::path::convert_to_slash;
+
+const char *testPathURI(llvm::StringRef Path,
+                        llvm::UniqueStringSaver &Strings) {
+  const auto URI = URI::createFile(testPath(Path));
+  return Strings.save(URI.toString()).begin();
+}
+
+TEST(RemoteMarshallingTest, URITranslation) {
+  llvm::BumpPtrAllocator Arena;
+  llvm::UniqueStringSaver Strings(Arena);
+  clangd::Ref Original;
+  Original.Location.FileURI =
+      testPathURI("remote/machine/projects/llvm-project/clang-tools-extra/"
+                  "clangd/unittests/remote/MarshallingTests.cpp",
+                  Strings);
+  auto Serialized =
+      toProtobuf(Original, testPath("remote/machine/projects/llvm-project/"));
+  EXPECT_EQ(Serialized.location().file_path(),
+            "clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
+  const std::string LocalIndexPrefix = testPath("local/machine/project/");
+  auto Deserialized = fromProtobuf(Serialized, &Strings,
+                                   testPath("home/my-projects/llvm-project"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(Deserialized->Location.FileURI,
+            testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
+                        "clangd/unittests/remote/MarshallingTests.cpp",
+                        Strings));
+
+  clangd::Ref WithInvalidURI;
+  // Invalid URI results in empty path.
+  WithInvalidURI.Location.FileURI = "This is not a URI";
+  Serialized = toProtobuf(WithInvalidURI, testPath("home/"));
+  EXPECT_EQ(Serialized.location().file_path(), "");
+
+  // Can not use URIs with scheme different from "file".
+  auto UnittestURI =
+      URI::create(testPath("project/lib/HelloWorld.cpp"), "unittest");
+  EXPECT_TRUE(bool(UnittestURI));
+  WithInvalidURI.Location.FileURI =
+      Strings.save(UnittestURI->toString()).begin();
+  Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/"));
+  EXPECT_EQ(Serialized.location().file_path(), "");
+
+  Ref WithAbsolutePath;
+  *WithAbsolutePath.mutable_location()->mutable_file_path() =
+      "/usr/local/user/home/HelloWorld.cpp";
+  Deserialized = fromProtobuf(WithAbsolutePath, &Strings, LocalIndexPrefix);
+  // Paths transmitted over the wire can not be absolute, they have to be
+  // relative.
+  EXPECT_FALSE(Deserialized);
+}
+
 TEST(RemoteMarshallingTest, SymbolSerialization) {
-  const auto *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 ();
-  )";
-  const auto TU = TestTU::withHeaderCode(Header);
-  const auto Symbols = TU.headerSymbols();
-  // Sanity check: there are more than 5 symbols available.
-  EXPECT_GE(Symbols.size(), 5UL);
+  clangd::Symbol Sym;
+
+  auto ID = SymbolID::fromStr("057557CEBF6E6B2D");
+  EXPECT_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);
-  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));
-  }
+
+  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;
+
+  // Check that symbols are exactly the same if the path to indexed project is
+  // the same on indexing machine and the client.
+  auto Serialized = toProtobuf(Sym, testPath("home/"));
+  auto Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
+  // Serialized paths are relative and have UNIX slashes.
+  EXPECT_EQ(convert_to_slash(Serialized.definition().file_path(),
+                             llvm::sys::path::Style::posix),
+            Serialized.definition().file_path());
+  EXPECT_TRUE(
+      llvm::sys::path::is_relative(Serialized.definition().file_path()));
+
+  // Invalid URI: Definition will be blank in deserialized object.
+  Location.FileURI = "Not A URI";
+  Sym.Definition = Location;
+  Serialized = toProtobuf(Sym, testPath("home/"));
+  Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(Deserialized->Definition, clangd::SymbolLocation());
+
+  // Schemes other than "file" can not be used.
+  auto UnittestURI = URI::create(testPath("home/SomePath.h"), "unittest");
+  EXPECT_TRUE(bool(UnittestURI));
+  Location.FileURI = Strings.save(UnittestURI->toString()).begin();
+  Sym.Definition = Location;
+  Serialized = toProtobuf(Sym, testPath("home/"));
+  Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(Deserialized->Definition, clangd::SymbolLocation());
+
+  // Passing root that is not prefix of the original file path.
+  Location.FileURI = testPathURI("home/File.h", Strings);
+  Sym.Definition = Location;
+  // Check that the symbol is valid and passing the correct path works.
+  Serialized = toProtobuf(Sym, testPath("home/"));
+  Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(Deserialized->Definition.FileURI,
+            testPathURI("home/File.h", Strings));
+  // Passing wrong root will leave definition blank.
+  Serialized = toProtobuf(Sym, testPath("nothome/"));
+  Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(Deserialized->Definition, clangd::SymbolLocation());
 }
 
-TEST(RemoteMarshallingTest, ReferenceSerialization) {
-  TestTU TU;
-  TU.HeaderCode = R"(
-  int foo();
-  int GlobalVariable = 42;
-  class Foo {
-  public:
-    Foo();
-
-    char Symbol = 'S';
-  };
-  template <typename T>
-  T getT() { return T(); }
-  )";
-  TU.Code = R"(
-  int foo() {
-    ++GlobalVariable;
-
-    Foo foo = Foo();
-    if (foo.Symbol - 'a' == 42) {
-      foo.Symbol = 'b';
-    }
-
-    const auto bar = getT<Foo>();
-  }
-  )";
-  const auto References = TU.headerRefs();
+TEST(RemoteMarshallingTest, RefSerialization) {
+  clangd::Ref Ref;
+  Ref.Kind = clangd::RefKind::Spelled | clangd::RefKind::Declaration;
+
   llvm::BumpPtrAllocator Arena;
   llvm::UniqueStringSaver Strings(Arena);
-  // Sanity check: there are more than 5 references available.
-  EXPECT_GE(References.numRefs(), 5UL);
-  for (const auto &SymbolWithRefs : References) {
-    for (const auto &Ref : SymbolWithRefs.second) {
-      const auto RefToProtobufAndBack = fromProtobuf(toProtobuf(Ref), &Strings);
-      EXPECT_TRUE(RefToProtobufAndBack.hasValue());
-      EXPECT_EQ(toYAML(Ref), toYAML(*RefToProtobufAndBack));
-    }
-  }
-} // namespace
+
+  clangd::SymbolLocation Location;
+  Location.Start.setLine(124);
+  Location.Start.setColumn(21);
+  Location.End.setLine(3213);
+  Location.End.setColumn(541);
+  Location.FileURI = testPathURI(
+      "llvm-project/llvm/clang-tools-extra/clangd/Protocol.h", Strings);
+  Ref.Location = Location;
+
+  auto Serialized = toProtobuf(Ref, testPath("llvm-project/"));
+  auto Deserialized =
+      fromProtobuf(Serialized, &Strings, testPath("llvm-project/"));
+  EXPECT_TRUE(Deserialized);
+  EXPECT_EQ(toYAML(Ref), toYAML(*Deserialized));
+}
+
+TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
+  llvm::BumpPtrAllocator Arena;
+  llvm::UniqueStringSaver Strings(Arena);
+
+  llvm::SmallVector<clangd::Symbol::IncludeHeaderWithReferences, 1>
+      ValidHeaders;
+  clangd::Symbol::IncludeHeaderWithReferences Header;
+  Header.IncludeHeader = Strings.save(
+      URI::createFile("/usr/local/user/home/project/Header.h").toString());
+  Header.References = 21;
+  ValidHeaders.push_back(Header);
+  Header.IncludeHeader = Strings.save("<iostream>");
+  Header.References = 100;
+  ValidHeaders.push_back(Header);
+  Header.IncludeHeader = Strings.save("\"cstdio\"");
+  Header.References = 200;
+  ValidHeaders.push_back(Header);
+
+  llvm::SmallVector<clangd::Symbol::IncludeHeaderWithReferences, 1>
+      InvalidHeaders;
+  // This is an absolute path to a header: can not be transmitted over the wire.
+  Header.IncludeHeader = Strings.save(testPath("project/include/Common.h"));
+  Header.References = 42;
+  InvalidHeaders.push_back(Header);
+  // This is not a valid header: can not be transmitted over the wire;
+  Header.IncludeHeader = Strings.save("NotAHeader");
+  Header.References = 5;
+  InvalidHeaders.push_back(Header);
+
+  clangd::Symbol Sym;
+
+  // Try to serialize all headers but only valid ones will end up in Protobuf
+  // message.
+  auto AllHeaders = ValidHeaders;
+  AllHeaders.insert(AllHeaders.end(), InvalidHeaders.begin(),
+                    InvalidHeaders.end());
+  Sym.IncludeHeaders = AllHeaders;
+
+  auto Serialized = toProtobuf(Sym, convert_to_slash("/"));
+  EXPECT_EQ(static_cast<size_t>(Serialized.headers_size()),
+            ValidHeaders.size());
+  auto Deserialized = fromProtobuf(Serialized, &Strings, "/");
+  EXPECT_TRUE(Deserialized);
+
+  Sym.IncludeHeaders = ValidHeaders;
+  EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
+}
+
+TEST(RemoteMarshallingTest, FuzzyFindRequestSerialization) {
+  clangd::FuzzyFindRequest Request;
+  Request.ProximityPaths = {testPath("remote/Header.h"),
+                            testPath("remote/subdir/OtherHeader.h"),
+                            testPath("notremote/File.h"), "Not a Path."};
+  const auto Serialized = toProtobuf(Request, testPath("remote/"));
+  EXPECT_EQ(Serialized.proximity_paths_size(), 2);
+  const auto Deserialized = fromProtobuf(&Serialized, testPath("home/"));
+  EXPECT_THAT(Deserialized.ProximityPaths,
+              testing::ElementsAre(testPath("home/Header.h"),
+                                   testPath("home/subdir/OtherHeader.h")));
+}
 
 } // namespace
 } // namespace remote
Index: clang-tools-extra/clangd/index/remote/unimplemented/UnimplementedClient.cpp
===================================================================
--- clang-tools-extra/clangd/index/remote/unimplemented/UnimplementedClient.cpp
+++ clang-tools-extra/clangd/index/remote/unimplemented/UnimplementedClient.cpp
@@ -8,12 +8,14 @@
 
 #include "index/remote/Client.h"
 #include "support/Logger.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace clangd {
 namespace remote {
 
-std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address) {
+std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address,
+                                               llvm::StringRef IndexRoot) {
   elog("Can't create SymbolIndex client without Remote Index support.");
   return nullptr;
 }
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
@@ -11,6 +11,7 @@
 #include "index/remote/marshalling/Marshalling.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Signals.h"
 
 #include <grpc++/grpc++.h>
@@ -31,6 +32,9 @@
 llvm::cl::opt<std::string> IndexPath(llvm::cl::desc("<INDEX FILE>"),
                                      llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt<std::string> IndexRoot(llvm::cl::desc("<PROJECT ROOT>"),
+                                     llvm::cl::Positional, llvm::cl::Required);
+
 llvm::cl::opt<std::string> ServerAddress(
     "server-address", llvm::cl::init("0.0.0.0:50051"),
     llvm::cl::desc("Address of the invoked server. Defaults to 0.0.0.0:50051"));
@@ -41,8 +45,13 @@
 
 class RemoteIndexServer final : public SymbolIndex::Service {
 public:
-  RemoteIndexServer(std::unique_ptr<clangd::SymbolIndex> Index)
-      : Index(std::move(Index)) {}
+  RemoteIndexServer(std::unique_ptr<clangd::SymbolIndex> Index,
+                    llvm::StringRef IndexRoot)
+      : Index(std::move(Index)) {
+    llvm::SmallString<256> NativePath = IndexRoot;
+    llvm::sys::path::native(NativePath);
+    IndexedProjectRoot = std::string(NativePath);
+  }
 
 private:
   grpc::Status Lookup(grpc::ServerContext *Context,
@@ -57,7 +66,8 @@
     }
     Index->lookup(Req, [&](const clangd::Symbol &Sym) {
       LookupReply NextMessage;
-      *NextMessage.mutable_stream_result() = toProtobuf(Sym);
+      *NextMessage.mutable_stream_result() =
+          toProtobuf(Sym, IndexedProjectRoot);
       Reply->Write(NextMessage);
     });
     LookupReply LastMessage;
@@ -69,10 +79,11 @@
   grpc::Status FuzzyFind(grpc::ServerContext *Context,
                          const FuzzyFindRequest *Request,
                          grpc::ServerWriter<FuzzyFindReply> *Reply) override {
-    const auto Req = fromProtobuf(Request);
+    const auto Req = fromProtobuf(Request, IndexedProjectRoot);
     bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
       FuzzyFindReply NextMessage;
-      *NextMessage.mutable_stream_result() = toProtobuf(Sym);
+      *NextMessage.mutable_stream_result() =
+          toProtobuf(Sym, IndexedProjectRoot);
       Reply->Write(NextMessage);
     });
     FuzzyFindReply LastMessage;
@@ -92,7 +103,7 @@
     }
     bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
       RefsReply NextMessage;
-      *NextMessage.mutable_stream_result() = toProtobuf(Reference);
+      *NextMessage.mutable_stream_result() = toProtobuf(Reference, IndexRoot);
       Reply->Write(NextMessage);
     });
     RefsReply LastMessage;
@@ -102,11 +113,12 @@
   }
 
   std::unique_ptr<clangd::SymbolIndex> Index;
+  std::string IndexedProjectRoot;
 };
 
 void runServer(std::unique_ptr<clangd::SymbolIndex> Index,
                const std::string &ServerAddress) {
-  RemoteIndexServer Service(std::move(Index));
+  RemoteIndexServer Service(std::move(Index), IndexRoot);
 
   grpc::EnableDefaultHealthCheckService(true);
   grpc::ServerBuilder Builder;
@@ -128,6 +140,11 @@
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  if (!llvm::sys::path::is_absolute(IndexRoot)) {
+    llvm::outs() << "Index root should be an absolute path.\n";
+    return -1;
+  }
+
   std::unique_ptr<clang::clangd::SymbolIndex> Index = openIndex(IndexPath);
 
   if (!Index) {
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
@@ -6,7 +6,27 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// Transformations between native Clangd types and Protobuf-generated classes.
+// Marshalling provides translation between native clangd types into the
+// Protobuf-generated classes. Most translations are 1-to-1 and wrap variables
+// into appropriate Protobuf types.
+//
+// A notable exception is URI translation. Because paths to files are different
+// on indexing machine and client machine
+// ("/remote/machine/projects/llvm-project/llvm/include/HelloWorld.h" versus
+// "/usr/local/username/llvm-project/llvm/include/HelloWorld.h"), they need to
+// be converted appropriately. Remote machine strips the prefix from an absolute
+// path and passes paths relative to the project root over the wire
+// ("include/HelloWorld.h" in this example). The indexed project root is passed
+// passed to the remote server. It contains absolute path to the project root
+// and includes a trailing slash. Relative path passed over the wire has unix
+// slashes. Client receives this relative path and constructs a URI that points
+// to the relevant file in the filesystem. Relative path is appended to
+// IndexRoot to construct the full path and build the final URI.
+//
+// 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
+// URIs.
 //
 //===----------------------------------------------------------------------===//
 
@@ -15,24 +35,29 @@
 
 #include "Index.pb.h"
 #include "index/Index.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/StringSaver.h"
 
 namespace clang {
 namespace clangd {
 namespace remote {
 
-clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request);
+clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request,
+                                      llvm::StringRef IndexRoot);
 llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message,
-                                            llvm::UniqueStringSaver *Strings);
+                                            llvm::UniqueStringSaver *Strings,
+                                            llvm::StringRef IndexRoot);
 llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message,
-                                         llvm::UniqueStringSaver *Strings);
+                                         llvm::UniqueStringSaver *Strings,
+                                         llvm::StringRef IndexRoot);
 
 LookupRequest toProtobuf(const clangd::LookupRequest &From);
-FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From);
+FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From,
+                            llvm::StringRef IndexRoot);
 RefsRequest toProtobuf(const clangd::RefsRequest &From);
 
-Ref toProtobuf(const clangd::Ref &From);
-Symbol toProtobuf(const clangd::Symbol &From);
+Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot);
+Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot);
 
 } // namespace remote
 } // namespace clangd
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
@@ -16,7 +16,13 @@
 #include "index/SymbolOrigin.h"
 #include "support/Logger.h"
 #include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/StringSaver.h"
 
 namespace clang {
@@ -25,6 +31,52 @@
 
 namespace {
 
+/// Translates \p RelativePath into the absolute path and builds URI for the
+/// user machine. This translation happens on the client side with the
+/// \p RelativePath received from remote index server and \p IndexRoot is
+/// provided by the client.
+llvm::Optional<std::string> relativePathToURI(llvm::StringRef RelativePath,
+                                              llvm::StringRef IndexRoot) {
+  assert(RelativePath == llvm::sys::path::convert_to_slash(
+                             RelativePath, llvm::sys::path::Style::posix));
+  if (RelativePath.empty())
+    return std::string();
+  if (llvm::sys::path::is_absolute(RelativePath)) {
+    elog("{0} is not relative path", RelativePath);
+    return llvm::None;
+  }
+  llvm::SmallString<256> FullPath = IndexRoot;
+  llvm::sys::path::append(FullPath, RelativePath);
+  const auto Result = URI::createFile(FullPath);
+  return Result.toString();
+}
+
+/// Translates a URI from the server's backing index to a relative path suitable
+/// to send over the wire to the client.
+llvm::Optional<std::string> uriToRelativePath(llvm::StringRef URI,
+                                              llvm::StringRef IndexRoot) {
+  auto ParsedURI = URI::parse(URI);
+  if (!ParsedURI) {
+    elog("Can not parse URI {0}: {1}", URI, ParsedURI.takeError());
+    return llvm::None;
+  }
+  if (ParsedURI->scheme() != "file") {
+    elog("Can not parse URI with scheme other than \"file\" {0}", URI);
+    return llvm::None;
+  }
+  llvm::SmallString<256> Result = ParsedURI->body();
+  if (IndexRoot.empty())
+    return std::string(Result);
+  if (!llvm::sys::path::replace_path_prefix(Result, IndexRoot, "")) {
+    elog("Can not get relative path from the URI {0} given the index root {1}",
+         URI, IndexRoot);
+    return llvm::None;
+  }
+  // Make sure the result has UNIX slashes.
+  return llvm::sys::path::convert_to_slash(Result,
+                                           llvm::sys::path::Style::posix);
+}
+
 clangd::SymbolLocation::Position fromProtobuf(const Position &Message) {
   clangd::SymbolLocation::Position Result;
   Result.setColumn(static_cast<uint32_t>(Message.column()));
@@ -58,40 +110,74 @@
   return Result;
 }
 
-clangd::SymbolLocation fromProtobuf(const SymbolLocation &Message,
-                                    llvm::UniqueStringSaver *Strings) {
+llvm::Optional<clangd::SymbolLocation>
+fromProtobuf(const SymbolLocation &Message, llvm::UniqueStringSaver *Strings,
+             llvm::StringRef IndexRoot) {
   clangd::SymbolLocation Location;
+  const auto URIString = relativePathToURI(Message.file_path(), IndexRoot);
+  if (!URIString)
+    return llvm::None;
+  Location.FileURI = Strings->save(*URIString).begin();
   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) {
+SymbolLocation toProtobuf(const clangd::SymbolLocation &Location,
+                          llvm::StringRef IndexRoot) {
   remote::SymbolLocation Result;
+  const auto RelativePath = uriToRelativePath(Location.FileURI, IndexRoot);
+  if (!RelativePath)
+    return Result;
+  *Result.mutable_file_path() = *RelativePath;
   *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) {
+llvm::Optional<HeaderWithReferences>
+toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader,
+           llvm::StringRef IndexRoot) {
   HeaderWithReferences Result;
-  Result.set_header(IncludeHeader.IncludeHeader.str());
   Result.set_references(IncludeHeader.References);
+  const std::string Header = IncludeHeader.IncludeHeader.str();
+  // If header starts with < or " it is not a URI: do not attempt to parse it.
+  if (Header.front() == '<' || Header.front() == '"') {
+    Result.set_header(Header);
+    return Result;
+  }
+  auto URI = URI::parse(Header);
+  if (!URI) {
+    elog("Can not parse header URI {0} from HeaderWithReferences: {1}", Header,
+         URI.takeError());
+    return llvm::None;
+  }
+  const auto RelativePath =
+      uriToRelativePath(IncludeHeader.IncludeHeader.str(), IndexRoot);
+  if (!RelativePath)
+    return llvm::None;
+  Result.set_header(*RelativePath);
   return Result;
 }
 
-clangd::Symbol::IncludeHeaderWithReferences
-fromProtobuf(const HeaderWithReferences &Message) {
-  return clangd::Symbol::IncludeHeaderWithReferences{Message.header(),
+llvm::Optional<clangd::Symbol::IncludeHeaderWithReferences>
+fromProtobuf(const HeaderWithReferences &Message,
+             llvm::UniqueStringSaver *Strings, llvm::StringRef IndexRoot) {
+  std::string Header = Message.header();
+  if (Header.front() != '<' && Header.front() != '"') {
+    const auto URIString = relativePathToURI(Header, IndexRoot);
+    if (!URIString)
+      return llvm::None;
+    Header = *URIString;
+  }
+  return clangd::Symbol::IncludeHeaderWithReferences{Strings->save(Header),
                                                      Message.references()};
 }
 
 } // namespace
 
-clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request) {
+clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request,
+                                      llvm::StringRef IndexRoot) {
   clangd::FuzzyFindRequest Result;
   Result.Query = Request->query();
   for (const auto &Scope : Request->scopes())
@@ -100,24 +186,28 @@
   if (Request->limit())
     Result.Limit = Request->limit();
   Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
-  for (const auto &Path : Request->proximity_paths())
-    Result.ProximityPaths.push_back(Path);
+  for (const auto &Path : Request->proximity_paths()) {
+    llvm::SmallString<256> LocalPath = llvm::StringRef(IndexRoot);
+    llvm::sys::path::append(LocalPath, Path);
+    Result.ProximityPaths.push_back(std::string(LocalPath));
+  }
   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) {
+                                            llvm::UniqueStringSaver *Strings,
+                                            llvm::StringRef IndexRoot) {
   if (!Message.has_info() || !Message.has_definition() ||
-      !Message.has_canonical_declarattion()) {
+      !Message.has_canonical_declaration()) {
     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(),
+    elog("Cannot parse SymbolID {} given Protobuf: {}", ID.takeError(),
          Message.ShortDebugString());
     return llvm::None;
   }
@@ -125,9 +215,16 @@
   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);
+  const auto Definition =
+      fromProtobuf(Message.definition(), Strings, IndexRoot);
+  if (!Definition)
+    return llvm::None;
+  Result.Definition = *Definition;
+  const auto Declaration =
+      fromProtobuf(Message.canonical_declaration(), Strings, IndexRoot);
+  if (!Declaration)
+    return llvm::None;
+  Result.CanonicalDeclaration = *Declaration;
   Result.References = Message.references();
   Result.Origin = static_cast<clangd::SymbolOrigin>(Message.origin());
   Result.Signature = Message.signature();
@@ -137,20 +234,26 @@
   Result.ReturnType = Message.return_type();
   Result.Type = Message.type();
   for (const auto &Header : Message.headers()) {
-    Result.IncludeHeaders.push_back(fromProtobuf(Header));
+    const auto SerializedHeader = fromProtobuf(Header, Strings, IndexRoot);
+    if (SerializedHeader)
+      Result.IncludeHeaders.push_back(*SerializedHeader);
   }
   Result.Flags = static_cast<clangd::Symbol::SymbolFlag>(Message.flags());
   return Result;
 }
 
 llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message,
-                                         llvm::UniqueStringSaver *Strings) {
+                                         llvm::UniqueStringSaver *Strings,
+                                         llvm::StringRef IndexRoot) {
   if (!Message.has_location()) {
     elog("Cannot convert Ref from Protobuf: {}", Message.ShortDebugString());
     return llvm::None;
   }
   clangd::Ref Result;
-  Result.Location = fromProtobuf(Message.location(), Strings);
+  const auto Location = fromProtobuf(Message.location(), Strings, IndexRoot);
+  if (!Location)
+    return llvm::None;
+  Result.Location = *Location;
   Result.Kind = static_cast<clangd::RefKind>(Message.kind());
   return Result;
 }
@@ -162,7 +265,8 @@
   return RPCRequest;
 }
 
-FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From) {
+FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From,
+                            llvm::StringRef IndexRoot) {
   FuzzyFindRequest RPCRequest;
   RPCRequest.set_query(From.Query);
   for (const auto &Scope : From.Scopes)
@@ -171,8 +275,12 @@
   if (From.Limit)
     RPCRequest.set_limit(*From.Limit);
   RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
-  for (const auto &Path : From.ProximityPaths)
-    RPCRequest.add_proximity_paths(Path);
+  for (const auto &Path : From.ProximityPaths) {
+    llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
+    if (llvm::sys::path::replace_path_prefix(RelativePath, IndexRoot, ""))
+      RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
+          RelativePath, llvm::sys::path::Style::posix));
+  }
   for (const auto &Type : From.PreferredTypes)
     RPCRequest.add_preferred_types(Type);
   return RPCRequest;
@@ -188,15 +296,15 @@
   return RPCRequest;
 }
 
-Symbol toProtobuf(const clangd::Symbol &From) {
+Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot) {
   Symbol Result;
   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.mutable_definition() = toProtobuf(From.Definition, IndexRoot);
   Result.set_scope(From.Scope.str());
-  *Result.mutable_canonical_declarattion() =
-      toProtobuf(From.CanonicalDeclaration);
+  *Result.mutable_canonical_declaration() =
+      toProtobuf(From.CanonicalDeclaration, IndexRoot);
   Result.set_references(From.References);
   Result.set_origin(static_cast<uint32_t>(From.Origin));
   Result.set_signature(From.Signature.str());
@@ -207,17 +315,20 @@
   Result.set_return_type(From.ReturnType.str());
   Result.set_type(From.Type.str());
   for (const auto &Header : From.IncludeHeaders) {
+    const auto Serialized = toProtobuf(Header, IndexRoot);
+    if (!Serialized)
+      continue;
     auto *NextHeader = Result.add_headers();
-    *NextHeader = toProtobuf(Header);
+    *NextHeader = *Serialized;
   }
   Result.set_flags(static_cast<uint32_t>(From.Flags));
   return Result;
 }
 
-Ref toProtobuf(const clangd::Ref &From) {
+Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot) {
   Ref Result;
   Result.set_kind(static_cast<uint32_t>(From.Kind));
-  *Result.mutable_location() = toProtobuf(From.Location);
+  *Result.mutable_location() = toProtobuf(From.Location, IndexRoot);
   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
@@ -71,7 +71,7 @@
   string name = 3;
   SymbolLocation definition = 4;
   string scope = 5;
-  SymbolLocation canonical_declarattion = 6;
+  SymbolLocation canonical_declaration = 6;
   int32 references = 7;
   uint32 origin = 8;
   string signature = 9;
@@ -99,7 +99,10 @@
 message SymbolLocation {
   Position start = 1;
   Position end = 2;
-  string file_uri = 3;
+  // clangd::SymbolLocation stores FileURI, but the protocol transmits a the
+  // relative path. Because paths are different on the remote and local machines
+  // they will be translated in the marshalling layer.
+  string file_path = 3;
 }
 
 message Position {
Index: clang-tools-extra/clangd/index/remote/Client.h
===================================================================
--- clang-tools-extra/clangd/index/remote/Client.h
+++ clang-tools-extra/clangd/index/remote/Client.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REMOTE_INDEX_H
 
 #include "index/Index.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace clangd {
@@ -17,12 +18,16 @@
 
 /// Returns an SymbolIndex client that passes requests to remote index located
 /// at \p Address. The client allows synchronous RPC calls.
+/// \p IndexRoot is an absolute path on the local machine to the source tree
+/// described by the remote index. Paths returned by the index will be treated
+/// as relative to this directory.
 ///
 /// This method attempts to resolve the address and establish the connection.
 ///
 /// \returns nullptr if the address is not resolved during the function call or
 /// if the project was compiled without Remote Index support.
-std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address);
+std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address,
+                                               llvm::StringRef IndexRoot);
 
 } // namespace remote
 } // namespace clangd
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
@@ -10,10 +10,12 @@
 
 #include "Client.h"
 #include "Index.grpc.pb.h"
+#include "index/Index.h"
 #include "index/Serialization.h"
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "llvm/ADT/StringRef.h"
 
 #include <chrono>
 
@@ -27,6 +29,16 @@
   using StreamingCall = std::unique_ptr<grpc::ClientReader<ReplyT>> (
       remote::SymbolIndex::Stub::*)(grpc::ClientContext *, const RequestT &);
 
+  template <typename ClangdRequestT, typename RequestT>
+  RequestT serializeRequest(ClangdRequestT Request) const {
+    return toProtobuf(Request);
+  }
+
+  template <>
+  FuzzyFindRequest serializeRequest(clangd::FuzzyFindRequest Request) const {
+    return toProtobuf(Request, ProjectRoot);
+  }
+
   template <typename RequestT, typename ReplyT, typename ClangdRequestT,
             typename CallbackT>
   bool streamRPC(ClangdRequestT Request,
@@ -34,7 +46,7 @@
                  CallbackT Callback) const {
     bool FinalResult = false;
     trace::Span Tracer(RequestT::descriptor()->name());
-    const auto RPCRequest = toProtobuf(Request);
+    const auto RPCRequest = serializeRequest<ClangdRequestT, RequestT>(Request);
     grpc::ClientContext Context;
     std::chrono::system_clock::time_point Deadline =
         std::chrono::system_clock::now() + DeadlineWaitingTime;
@@ -48,10 +60,11 @@
         FinalResult = Reply.final_result();
         continue;
       }
-      auto Sym = fromProtobuf(Reply.stream_result(), &Strings);
-      if (!Sym)
+      auto Response =
+          fromProtobuf(Reply.stream_result(), &Strings, ProjectRoot);
+      if (!Response)
         elog("Received invalid {0}", ReplyT::descriptor()->name());
-      Callback(*Sym);
+      Callback(*Response);
     }
     SPAN_ATTACH(Tracer, "status", Reader->Finish().ok());
     return FinalResult;
@@ -59,9 +72,9 @@
 
 public:
   IndexClient(
-      std::shared_ptr<grpc::Channel> Channel,
+      std::shared_ptr<grpc::Channel> Channel, llvm::StringRef ProjectRoot,
       std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
-      : Stub(remote::SymbolIndex::NewStub(Channel)),
+      : Stub(remote::SymbolIndex::NewStub(Channel)), ProjectRoot(ProjectRoot),
         DeadlineWaitingTime(DeadlineTime) {}
 
   void lookup(const clangd::LookupRequest &Request,
@@ -86,22 +99,26 @@
             llvm::function_ref<void(const SymbolID &, const clangd::Symbol &)>)
       const {}
 
-  // IndexClient does not take any space since the data is stored on the server.
+  // IndexClient does not take any space since the data is stored on the
+  // server.
   size_t estimateMemoryUsage() const { return 0; }
 
 private:
   std::unique_ptr<remote::SymbolIndex::Stub> Stub;
+  std::string ProjectRoot;
   // Each request will be terminated if it takes too long.
   std::chrono::milliseconds DeadlineWaitingTime;
 };
 
 } // namespace
 
-std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address) {
+std::unique_ptr<clangd::SymbolIndex> getClient(llvm::StringRef Address,
+                                               llvm::StringRef ProjectRoot) {
   const auto Channel =
       grpc::CreateChannel(Address.str(), grpc::InsecureChannelCredentials());
   Channel->GetState(true);
-  return std::unique_ptr<clangd::SymbolIndex>(new IndexClient(Channel));
+  return std::unique_ptr<clangd::SymbolIndex>(
+      new IndexClient(Channel, ProjectRoot));
 }
 
 } // namespace remote
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===================================================================
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -32,6 +32,9 @@
 llvm::cl::opt<std::string>
     ExecCommand("c", llvm::cl::desc("Command to execute and then exit"));
 
+llvm::cl::opt<std::string> ProjectRoot("project-root",
+                                       llvm::cl::desc("Path to the project"));
+
 static constexpr char Overview[] = R"(
 This is an **experimental** interactive tool to process user-provided search
 queries over given symbol collection obtained via clangd-indexer. The
@@ -326,7 +329,8 @@
 
 std::unique_ptr<SymbolIndex> openIndex(llvm::StringRef Index) {
   return Index.startswith("remote:")
-             ? remote::getClient(Index.drop_front(strlen("remote:")))
+             ? remote::getClient(Index.drop_front(strlen("remote:")),
+                                 ProjectRoot)
              : loadIndex(Index, /*UseDex=*/true);
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to