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