This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeef162c330b0: [clangd] Don't send invalid messages from
remote index (authored by kbobyrev).
Changed prior to commit:
https://reviews.llvm.org/D83826?vs=278683&id=279041#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83826/new/
https://reviews.llvm.org/D83826
Files:
clang-tools-extra/clangd/index/remote/Client.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
clang-tools-extra/clangd/index/remote/server/Server.cpp
clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
Index: clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -13,6 +13,7 @@
#include "index/Serialization.h"
#include "index/Symbol.h"
#include "index/SymbolID.h"
+#include "index/SymbolLocation.h"
#include "index/remote/marshalling/Marshalling.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/SmallString.h"
@@ -39,29 +40,35 @@
TEST(RemoteMarshallingTest, URITranslation) {
llvm::BumpPtrAllocator Arena;
llvm::UniqueStringSaver Strings(Arena);
+ Marshaller ProtobufMarshaller(
+ testPath("remote/machine/projects/llvm-project/"),
+ testPath("home/my-projects/llvm-project/"));
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(),
+ auto Serialized = ProtobufMarshaller.toProtobuf(Original);
+ EXPECT_TRUE(Serialized);
+ 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/"));
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
EXPECT_TRUE(Deserialized);
- EXPECT_EQ(Deserialized->Location.FileURI,
- testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
- "clangd/unittests/remote/MarshallingTests.cpp",
- Strings));
+ EXPECT_STREQ(Deserialized->Location.FileURI,
+ testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
+ "clangd/unittests/remote/MarshallingTests.cpp",
+ Strings));
+
+ // Can't have empty paths.
+ *Serialized->mutable_location()->mutable_file_path() = std::string();
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_FALSE(Deserialized);
clangd::Ref WithInvalidURI;
- // Invalid URI results in empty path.
+ // Invalid URI results in serialization failure.
WithInvalidURI.Location.FileURI = "This is not a URI";
- Serialized = toProtobuf(WithInvalidURI, testPath("home/"));
- EXPECT_EQ(Serialized.location().file_path(), "");
+ Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+ EXPECT_FALSE(Serialized);
// Can not use URIs with scheme different from "file".
auto UnittestURI =
@@ -69,15 +76,15 @@
EXPECT_TRUE(bool(UnittestURI));
WithInvalidURI.Location.FileURI =
Strings.save(UnittestURI->toString()).begin();
- Serialized = toProtobuf(WithInvalidURI, testPath("project/lib/"));
- EXPECT_EQ(Serialized.location().file_path(), "");
+ Serialized = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+ EXPECT_FALSE(Serialized);
+ // Paths transmitted over the wire can not be absolute, they have to be
+ // relative.
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.
+ Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
EXPECT_FALSE(Deserialized);
}
@@ -128,48 +135,63 @@
Sym.Flags = clangd::Symbol::SymbolFlag::IndexedForCodeCompletion;
+ Marshaller ProtobufMarshaller(testPath("home/"), testPath("home/"));
+
// 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/"));
+ auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_TRUE(Serialized);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
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(),
+ EXPECT_EQ(convert_to_slash(Serialized->definition().file_path(),
llvm::sys::path::Style::posix),
- Serialized.definition().file_path());
+ Serialized->definition().file_path());
EXPECT_TRUE(
- llvm::sys::path::is_relative(Serialized.definition().file_path()));
+ llvm::sys::path::is_relative(Serialized->definition().file_path()));
+
+ // Missing definition is OK.
+ Sym.Definition = clangd::SymbolLocation();
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_TRUE(Serialized);
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_TRUE(Deserialized);
+
+ // Relative path is absolute.
+ *Serialized->mutable_canonical_declaration()->mutable_file_path() =
+ convert_to_slash("/path/to/Declaration.h");
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_FALSE(Deserialized);
// Fail with an invalid URI.
Location.FileURI = "Not A URI";
Sym.Definition = Location;
- Serialized = toProtobuf(Sym, testPath("home/"));
- Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
- EXPECT_FALSE(Deserialized);
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(Serialized);
// 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_FALSE(Deserialized);
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(Serialized);
// 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/"));
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_TRUE(Serialized);
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
EXPECT_TRUE(Deserialized);
- EXPECT_EQ(Deserialized->Definition.FileURI,
- testPathURI("home/File.h", Strings));
+ EXPECT_STREQ(Deserialized->Definition.FileURI,
+ testPathURI("home/File.h", Strings));
// Fail with a wrong root.
- Serialized = toProtobuf(Sym, testPath("nothome/"));
- Deserialized = fromProtobuf(Serialized, &Strings, testPath("home/"));
- EXPECT_FALSE(Deserialized);
+ Marshaller WrongMarshaller(testPath("nothome/"), testPath("home/"));
+ Serialized = WrongMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(Serialized);
}
TEST(RemoteMarshallingTest, RefSerialization) {
@@ -188,9 +210,12 @@
"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/"));
+ Marshaller ProtobufMarshaller(testPath("llvm-project/"),
+ testPath("llvm-project/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Ref);
+ EXPECT_TRUE(Serialized);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
EXPECT_TRUE(Deserialized);
EXPECT_EQ(toYAML(Ref), toYAML(*Deserialized));
}
@@ -242,10 +267,13 @@
InvalidHeaders.end());
Sym.IncludeHeaders = AllHeaders;
- auto Serialized = toProtobuf(Sym, convert_to_slash("/"));
- EXPECT_EQ(static_cast<size_t>(Serialized.headers_size()),
+ Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_EQ(static_cast<size_t>(Serialized->headers_size()),
ValidHeaders.size());
- auto Deserialized = fromProtobuf(Serialized, &Strings, convert_to_slash("/"));
+ EXPECT_TRUE(Serialized);
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
EXPECT_TRUE(Deserialized);
Sym.IncludeHeaders = ValidHeaders;
@@ -257,35 +285,38 @@
Request.ProximityPaths = {testPath("remote/Header.h"),
testPath("remote/subdir/OtherHeader.h"),
testPath("notremote/File.h"), "Not a Path."};
- auto Serialized = toProtobuf(Request, testPath("remote/"));
+ Marshaller ProtobufMarshaller(testPath("remote/"), testPath("home/"));
+ auto Serialized = ProtobufMarshaller.toProtobuf(Request);
EXPECT_EQ(Serialized.proximity_paths_size(), 2);
- auto Deserialized = fromProtobuf(&Serialized, testPath("home/"));
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
EXPECT_THAT(Deserialized.ProximityPaths,
testing::ElementsAre(testPath("home/Header.h"),
testPath("home/subdir/OtherHeader.h")));
}
TEST(RemoteMarshallingTest, RelativePathToURITranslation) {
- EXPECT_TRUE(relativePathToURI("lib/File.cpp", testPath("home/project/")));
+ Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"",
+ /*LocalIndexRoot=*/testPath("home/project/"));
+ EXPECT_TRUE(ProtobufMarshaller.relativePathToURI("lib/File.cpp"));
// RelativePath can not be absolute.
- EXPECT_FALSE(relativePathToURI("/lib/File.cpp", testPath("home/project/")));
- // IndexRoot has to be absolute path.
- EXPECT_FALSE(relativePathToURI("lib/File.cpp", "home/project/"));
+ EXPECT_FALSE(ProtobufMarshaller.relativePathToURI("/lib/File.cpp"));
+ // RelativePath can not be empty.
+ EXPECT_FALSE(ProtobufMarshaller.relativePathToURI(std::string()));
}
TEST(RemoteMarshallingTest, URIToRelativePathTranslation) {
llvm::BumpPtrAllocator Arena;
llvm::UniqueStringSaver Strings(Arena);
- EXPECT_TRUE(
- uriToRelativePath(testPathURI("home/project/lib/File.cpp", Strings),
- testPath("home/project/")));
- // IndexRoot has to be absolute path.
- EXPECT_FALSE(uriToRelativePath(
- testPathURI("home/project/lib/File.cpp", Strings), "home/project/"));
- // IndexRoot has to be be a prefix of the file path.
- EXPECT_FALSE(
- uriToRelativePath(testPathURI("home/project/lib/File.cpp", Strings),
- testPath("home/other/project/")));
+ Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/testPath("remote/project/"),
+ /*LocalIndexRoot=*/"");
+ EXPECT_TRUE(ProtobufMarshaller.uriToRelativePath(
+ testPathURI("remote/project/lib/File.cpp", Strings)));
+ // RemoteIndexRoot has to be be a prefix of the file path.
+ Marshaller WrongMarshaller(
+ /*RemoteIndexRoot=*/testPath("remote/other/project/"),
+ /*LocalIndexRoot=*/"");
+ EXPECT_FALSE(WrongMarshaller.uriToRelativePath(
+ testPathURI("remote/project/lib/File.cpp", Strings)));
}
} // namespace
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
@@ -50,7 +50,9 @@
: Index(std::move(Index)) {
llvm::SmallString<256> NativePath = IndexRoot;
llvm::sys::path::native(NativePath);
- IndexedProjectRoot = std::string(NativePath);
+ ProtobufMarshaller = std::unique_ptr<Marshaller>(new Marshaller(
+ /*RemoteIndexRoot=*/llvm::StringRef(NativePath),
+ /*LocalIndexRoot=*/""));
}
private:
@@ -65,9 +67,11 @@
Req.IDs.insert(*SID);
}
Index->lookup(Req, [&](const clangd::Symbol &Sym) {
+ auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
+ if (!SerializedSymbol)
+ return;
LookupReply NextMessage;
- *NextMessage.mutable_stream_result() =
- toProtobuf(Sym, IndexedProjectRoot);
+ *NextMessage.mutable_stream_result() = *SerializedSymbol;
Reply->Write(NextMessage);
});
LookupReply LastMessage;
@@ -79,11 +83,13 @@
grpc::Status FuzzyFind(grpc::ServerContext *Context,
const FuzzyFindRequest *Request,
grpc::ServerWriter<FuzzyFindReply> *Reply) override {
- const auto Req = fromProtobuf(Request, IndexedProjectRoot);
+ const auto Req = ProtobufMarshaller->fromProtobuf(Request);
bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol &Sym) {
+ auto SerializedSymbol = ProtobufMarshaller->toProtobuf(Sym);
+ if (!SerializedSymbol)
+ return;
FuzzyFindReply NextMessage;
- *NextMessage.mutable_stream_result() =
- toProtobuf(Sym, IndexedProjectRoot);
+ *NextMessage.mutable_stream_result() = *SerializedSymbol;
Reply->Write(NextMessage);
});
FuzzyFindReply LastMessage;
@@ -102,8 +108,11 @@
Req.IDs.insert(*SID);
}
bool HasMore = Index->refs(Req, [&](const clangd::Ref &Reference) {
+ auto SerializedRef = ProtobufMarshaller->toProtobuf(Reference);
+ if (!SerializedRef)
+ return;
RefsReply NextMessage;
- *NextMessage.mutable_stream_result() = toProtobuf(Reference, IndexRoot);
+ *NextMessage.mutable_stream_result() = *SerializedRef;
Reply->Write(NextMessage);
});
RefsReply LastMessage;
@@ -113,7 +122,7 @@
}
std::unique_ptr<clangd::SymbolIndex> Index;
- std::string IndexedProjectRoot;
+ std::unique_ptr<Marshaller> ProtobufMarshaller;
};
void runServer(std::unique_ptr<clangd::SymbolIndex> 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
@@ -5,31 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-//
-// 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 the
-// 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
-// to the remote server. Client receives this relative path and constructs a URI
-// that points to the relevant file in the filesystem. The relative path is
-// appended to IndexRoot to construct the full path and build the final URI.
-//
-// Index root is the absolute path to the project and includes a trailing slash.
-// The relative path passed over the wire has unix slashes.
-//
-// 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.
-//
-//===----------------------------------------------------------------------===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REMOTE_MARSHALLING_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_REMOTE_MARSHALLING_H
@@ -43,33 +18,76 @@
namespace clangd {
namespace remote {
-clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request,
- llvm::StringRef IndexRoot);
-llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message,
- llvm::UniqueStringSaver *Strings,
- llvm::StringRef IndexRoot);
-llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message,
- llvm::UniqueStringSaver *Strings,
- llvm::StringRef IndexRoot);
+// Marshalling provides an interface for translattion 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
+/// (RemoteIndexRoot) from the 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 to the remote server. Client receives this
+/// relative path and constructs a URI that points to the relevant file in the
+/// filesystem. The relative path is appended to LocalIndexRoot to construct the
+/// full path and build the final URI.
+class Marshaller {
+public:
+ Marshaller() = delete;
+ Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
+
+ clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request);
+ llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message);
+ llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message);
+
+ /// 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.
+ LookupRequest toProtobuf(const clangd::LookupRequest &From);
+ FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From);
+ RefsRequest toProtobuf(const clangd::RefsRequest &From);
+
+ llvm::Optional<Ref> toProtobuf(const clangd::Ref &From);
+ llvm::Optional<Symbol> toProtobuf(const clangd::Symbol &From);
-LookupRequest toProtobuf(const clangd::LookupRequest &From);
-FuzzyFindRequest toProtobuf(const clangd::FuzzyFindRequest &From,
- llvm::StringRef IndexRoot);
-RefsRequest toProtobuf(const clangd::RefsRequest &From);
+ /// 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.
+ ///
+ /// The relative path passed over the wire has unix slashes.
+ llvm::Optional<std::string> relativePathToURI(llvm::StringRef RelativePath);
+ /// 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);
-Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot);
-Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot);
+private:
+ clangd::SymbolLocation::Position fromProtobuf(const Position &Message);
+ Position toProtobuf(const clangd::SymbolLocation::Position &Position);
+ clang::index::SymbolInfo fromProtobuf(const SymbolInfo &Message);
+ SymbolInfo toProtobuf(const clang::index::SymbolInfo &Info);
+ llvm::Optional<clangd::SymbolLocation>
+ fromProtobuf(const SymbolLocation &Message);
+ llvm::Optional<SymbolLocation>
+ toProtobuf(const clangd::SymbolLocation &Location);
+ llvm::Optional<HeaderWithReferences>
+ toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader);
+ llvm::Optional<clangd::Symbol::IncludeHeaderWithReferences>
+ fromProtobuf(const HeaderWithReferences &Message);
-/// 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);
-/// 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);
+ /// RemoteIndexRoot and LocalIndexRoot are absolute paths to the project (on
+ /// remote and local machine respectively) and include a trailing slash. One
+ /// of them can be missing (if the machines are different they don't know each
+ /// other's specifics and will only do one-way translation), but both can not
+ /// be missing at the same time.
+ llvm::Optional<std::string> RemoteIndexRoot;
+ llvm::Optional<std::string> LocalIndexRoot;
+ llvm::BumpPtrAllocator Arena;
+ llvm::UniqueStringSaver Strings;
+};
} // 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
@@ -30,101 +30,28 @@
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;
-}
-
-llvm::Optional<clangd::SymbolLocation>
-fromProtobuf(const SymbolLocation &Message, llvm::UniqueStringSaver *Strings,
- llvm::StringRef IndexRoot) {
- clangd::SymbolLocation Location;
- 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());
- return Location;
-}
-
-llvm::Optional<SymbolLocation>
-toProtobuf(const clangd::SymbolLocation &Location, llvm::StringRef IndexRoot) {
- remote::SymbolLocation Result;
- auto RelativePath = uriToRelativePath(Location.FileURI, IndexRoot);
- if (!RelativePath)
- return llvm::None;
- *Result.mutable_file_path() = *RelativePath;
- *Result.mutable_start() = toProtobuf(Location.Start);
- *Result.mutable_end() = toProtobuf(Location.End);
- return Result;
-}
-
-llvm::Optional<HeaderWithReferences>
-toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader,
- llvm::StringRef IndexRoot) {
- HeaderWithReferences Result;
- Result.set_references(IncludeHeader.References);
- const std::string Header = IncludeHeader.IncludeHeader.str();
- if (isLiteralInclude(Header)) {
- Result.set_header(Header);
- return Result;
+Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
+ llvm::StringRef LocalIndexRoot)
+ : Strings(Arena) {
+ if (!RemoteIndexRoot.empty()) {
+ assert(llvm::sys::path::is_absolute(RemoteIndexRoot));
+ assert(RemoteIndexRoot ==
+ llvm::sys::path::convert_to_slash(RemoteIndexRoot));
+ assert(RemoteIndexRoot.endswith(llvm::sys::path::get_separator()));
+ this->RemoteIndexRoot = RemoteIndexRoot.str();
}
- auto RelativePath = uriToRelativePath(Header, IndexRoot);
- if (!RelativePath)
- return llvm::None;
- Result.set_header(*RelativePath);
- return Result;
-}
-
-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() != '"') {
- auto URIString = relativePathToURI(Header, IndexRoot);
- if (!URIString)
- return llvm::None;
- Header = *URIString;
+ if (!LocalIndexRoot.empty()) {
+ assert(llvm::sys::path::is_absolute(LocalIndexRoot));
+ assert(LocalIndexRoot == llvm::sys::path::convert_to_slash(LocalIndexRoot));
+ assert(LocalIndexRoot.endswith(llvm::sys::path::get_separator()));
+ this->LocalIndexRoot = LocalIndexRoot.str();
}
- return clangd::Symbol::IncludeHeaderWithReferences{Strings->save(Header),
- Message.references()};
+ assert(!RemoteIndexRoot.empty() || !LocalIndexRoot.empty());
}
-} // namespace
-
-clangd::FuzzyFindRequest fromProtobuf(const FuzzyFindRequest *Request,
- llvm::StringRef IndexRoot) {
+clangd::FuzzyFindRequest
+Marshaller::fromProtobuf(const FuzzyFindRequest *Request) {
+ assert(LocalIndexRoot);
clangd::FuzzyFindRequest Result;
Result.Query = Request->query();
for (const auto &Scope : Request->scopes())
@@ -134,7 +61,7 @@
Result.Limit = Request->limit();
Result.RestrictForCodeCompletion = Request->restricted_for_code_completion();
for (const auto &Path : Request->proximity_paths()) {
- llvm::SmallString<256> LocalPath = llvm::StringRef(IndexRoot);
+ llvm::SmallString<256> LocalPath = llvm::StringRef(*LocalIndexRoot);
llvm::sys::path::append(LocalPath, Path);
Result.ProximityPaths.push_back(std::string(LocalPath));
}
@@ -143,34 +70,35 @@
return Result;
}
-llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message,
- llvm::UniqueStringSaver *Strings,
- llvm::StringRef IndexRoot) {
- if (!Message.has_info() || !Message.has_definition() ||
- !Message.has_canonical_declaration()) {
- elog("Cannot convert Symbol from Protobuf: {0}",
- Message.ShortDebugString());
+llvm::Optional<clangd::Symbol> Marshaller::fromProtobuf(const Symbol &Message) {
+ if (!Message.has_info() || !Message.has_canonical_declaration()) {
+ elog("Cannot convert Symbol from protobuf (missing info, definition or "
+ "declaration): {0}",
+ Message.DebugString());
return llvm::None;
}
clangd::Symbol Result;
auto ID = SymbolID::fromStr(Message.id());
if (!ID) {
- elog("Cannot parse SymbolID {0} given Protobuf: {1}", ID.takeError(),
- Message.ShortDebugString());
+ elog("Cannot parse SymbolID {0} given protobuf: {1}", ID.takeError(),
+ Message.DebugString());
return llvm::None;
}
Result.ID = *ID;
Result.SymInfo = fromProtobuf(Message.info());
Result.Name = Message.name();
Result.Scope = Message.scope();
- auto Definition = fromProtobuf(Message.definition(), Strings, IndexRoot);
- if (!Definition)
- return llvm::None;
- Result.Definition = *Definition;
- auto Declaration =
- fromProtobuf(Message.canonical_declaration(), Strings, IndexRoot);
- if (!Declaration)
+ if (Message.has_definition()) {
+ auto Definition = fromProtobuf(Message.definition());
+ if (Definition)
+ Result.Definition = *Definition;
+ }
+ auto Declaration = fromProtobuf(Message.canonical_declaration());
+ if (!Declaration) {
+ elog("Cannot convert Symbol from protobuf (invalid declaration): {0}",
+ Message.DebugString());
return llvm::None;
+ }
Result.CanonicalDeclaration = *Declaration;
Result.References = Message.references();
Result.Origin = static_cast<clangd::SymbolOrigin>(Message.origin());
@@ -181,39 +109,44 @@
Result.ReturnType = Message.return_type();
Result.Type = Message.type();
for (const auto &Header : Message.headers()) {
- auto SerializedHeader = fromProtobuf(Header, Strings, IndexRoot);
+ auto SerializedHeader = fromProtobuf(Header);
if (SerializedHeader)
Result.IncludeHeaders.push_back(*SerializedHeader);
+ else
+ elog("Cannot convert HeaderWithIncludes from protobuf: {0}",
+ Header.DebugString());
}
Result.Flags = static_cast<clangd::Symbol::SymbolFlag>(Message.flags());
return Result;
}
-llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message,
- llvm::UniqueStringSaver *Strings,
- llvm::StringRef IndexRoot) {
+llvm::Optional<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
if (!Message.has_location()) {
- elog("Cannot convert Ref from Protobuf: {}", Message.ShortDebugString());
+ elog("Cannot convert Ref from protobuf (missing location): {0}",
+ Message.DebugString());
return llvm::None;
}
clangd::Ref Result;
- auto Location = fromProtobuf(Message.location(), Strings, IndexRoot);
- if (!Location)
+ auto Location = fromProtobuf(Message.location());
+ if (!Location) {
+ elog("Cannot convert Ref from protobuf (invalid location): {0}",
+ Message.DebugString());
return llvm::None;
+ }
Result.Location = *Location;
Result.Kind = static_cast<clangd::RefKind>(Message.kind());
return Result;
}
-LookupRequest toProtobuf(const clangd::LookupRequest &From) {
+LookupRequest Marshaller::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,
- llvm::StringRef IndexRoot) {
+FuzzyFindRequest Marshaller::toProtobuf(const clangd::FuzzyFindRequest &From) {
+ assert(RemoteIndexRoot);
FuzzyFindRequest RPCRequest;
RPCRequest.set_query(From.Query);
for (const auto &Scope : From.Scopes)
@@ -224,7 +157,8 @@
RPCRequest.set_restricted_for_code_completion(From.RestrictForCodeCompletion);
for (const auto &Path : From.ProximityPaths) {
llvm::SmallString<256> RelativePath = llvm::StringRef(Path);
- if (llvm::sys::path::replace_path_prefix(RelativePath, IndexRoot, ""))
+ if (llvm::sys::path::replace_path_prefix(RelativePath, *RemoteIndexRoot,
+ ""))
RPCRequest.add_proximity_paths(llvm::sys::path::convert_to_slash(
RelativePath, llvm::sys::path::Style::posix));
}
@@ -233,7 +167,7 @@
return RPCRequest;
}
-RefsRequest toProtobuf(const clangd::RefsRequest &From) {
+RefsRequest Marshaller::toProtobuf(const clangd::RefsRequest &From) {
RefsRequest RPCRequest;
for (const auto &ID : From.IDs)
RPCRequest.add_ids(ID.str());
@@ -243,18 +177,28 @@
return RPCRequest;
}
-Symbol toProtobuf(const clangd::Symbol &From, llvm::StringRef IndexRoot) {
+llvm::Optional<Symbol> Marshaller::toProtobuf(const clangd::Symbol &From) {
Symbol Result;
Result.set_id(From.ID.str());
*Result.mutable_info() = toProtobuf(From.SymInfo);
Result.set_name(From.Name.str());
- auto Definition = toProtobuf(From.Definition, IndexRoot);
- if (Definition)
+ if (*From.Definition.FileURI) {
+ auto Definition = toProtobuf(From.Definition);
+ if (!Definition) {
+ elog("Can not convert Symbol to protobuf (invalid definition) {0}: {1}",
+ From, From.Definition);
+ return llvm::None;
+ }
*Result.mutable_definition() = *Definition;
+ }
Result.set_scope(From.Scope.str());
- auto Declaration = toProtobuf(From.CanonicalDeclaration, IndexRoot);
- if (Declaration)
- *Result.mutable_canonical_declaration() = *Declaration;
+ auto Declaration = toProtobuf(From.CanonicalDeclaration);
+ if (!Declaration) {
+ elog("Can not convert Symbol to protobuf (invalid declaration) {0}: {1}",
+ From, From.CanonicalDeclaration);
+ return llvm::None;
+ }
+ *Result.mutable_canonical_declaration() = *Declaration;
Result.set_references(From.References);
Result.set_origin(static_cast<uint32_t>(From.Origin));
Result.set_signature(From.Signature.str());
@@ -265,9 +209,12 @@
Result.set_return_type(From.ReturnType.str());
Result.set_type(From.Type.str());
for (const auto &Header : From.IncludeHeaders) {
- auto Serialized = toProtobuf(Header, IndexRoot);
- if (!Serialized)
+ auto Serialized = toProtobuf(Header);
+ if (!Serialized) {
+ elog("Can not convert IncludeHeaderWithReferences to protobuf: {0}",
+ Header.IncludeHeader);
continue;
+ }
auto *NextHeader = Result.add_headers();
*NextHeader = *Serialized;
}
@@ -275,50 +222,38 @@
return Result;
}
-// FIXME(kirillbobyrev): A reference without location is invalid.
-// llvm::Optional<Ref> here and on the server side?
-Ref toProtobuf(const clangd::Ref &From, llvm::StringRef IndexRoot) {
+llvm::Optional<Ref> Marshaller::toProtobuf(const clangd::Ref &From) {
Ref Result;
Result.set_kind(static_cast<uint32_t>(From.Kind));
- auto Location = toProtobuf(From.Location, IndexRoot);
- if (Location)
- *Result.mutable_location() = *Location;
+ auto Location = toProtobuf(From.Location);
+ if (!Location) {
+ elog("Can not convert Reference to protobuf (invalid location) {0}: {1}",
+ From, From.Location);
+ return llvm::None;
+ }
+ *Result.mutable_location() = *Location;
return Result;
}
-llvm::Optional<std::string> relativePathToURI(llvm::StringRef RelativePath,
- llvm::StringRef IndexRoot) {
+llvm::Optional<std::string>
+Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
+ assert(LocalIndexRoot);
assert(RelativePath == llvm::sys::path::convert_to_slash(
RelativePath, llvm::sys::path::Style::posix));
- assert(IndexRoot == llvm::sys::path::convert_to_slash(IndexRoot));
- assert(IndexRoot.endswith(llvm::sys::path::get_separator()));
- if (RelativePath.empty())
- return std::string();
- if (llvm::sys::path::is_absolute(RelativePath)) {
- elog("Remote index client got absolute path from server: {0}",
- RelativePath);
+ if (RelativePath.empty()) {
return llvm::None;
}
- if (llvm::sys::path::is_relative(IndexRoot)) {
- elog("Remote index client got a relative path as index root: {0}",
- IndexRoot);
+ if (llvm::sys::path::is_absolute(RelativePath)) {
return llvm::None;
}
- llvm::SmallString<256> FullPath = IndexRoot;
+ llvm::SmallString<256> FullPath = llvm::StringRef(*LocalIndexRoot);
llvm::sys::path::append(FullPath, RelativePath);
auto Result = URI::createFile(FullPath);
return Result.toString();
}
-llvm::Optional<std::string> uriToRelativePath(llvm::StringRef URI,
- llvm::StringRef IndexRoot) {
- assert(IndexRoot.endswith(llvm::sys::path::get_separator()));
- assert(IndexRoot == llvm::sys::path::convert_to_slash(IndexRoot));
- assert(!IndexRoot.empty());
- if (llvm::sys::path::is_relative(IndexRoot)) {
- elog("Index root {0} is not absolute path", IndexRoot);
- return llvm::None;
- }
+llvm::Optional<std::string> Marshaller::uriToRelativePath(llvm::StringRef URI) {
+ assert(RemoteIndexRoot);
auto ParsedURI = URI::parse(URI);
if (!ParsedURI) {
elog("Remote index got bad URI from client {0}: {1}", URI,
@@ -326,14 +261,10 @@
return llvm::None;
}
if (ParsedURI->scheme() != "file") {
- elog("Remote index got URI with scheme other than \"file\" {0}: {1}", URI,
- ParsedURI->scheme());
return llvm::None;
}
llvm::SmallString<256> Result = ParsedURI->body();
- 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);
+ if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, "")) {
return llvm::None;
}
// Make sure the result has UNIX slashes.
@@ -341,6 +272,94 @@
llvm::sys::path::Style::posix);
}
+clangd::SymbolLocation::Position
+Marshaller::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
+Marshaller::toProtobuf(const clangd::SymbolLocation::Position &Position) {
+ remote::Position Result;
+ Result.set_column(Position.column());
+ Result.set_line(Position.line());
+ return Result;
+}
+
+clang::index::SymbolInfo Marshaller::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 Marshaller::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;
+}
+
+llvm::Optional<clangd::SymbolLocation>
+Marshaller::fromProtobuf(const SymbolLocation &Message) {
+ clangd::SymbolLocation Location;
+ auto URIString = relativePathToURI(Message.file_path());
+ if (!URIString)
+ return llvm::None;
+ Location.FileURI = Strings.save(*URIString).begin();
+ Location.Start = fromProtobuf(Message.start());
+ Location.End = fromProtobuf(Message.end());
+ return Location;
+}
+
+llvm::Optional<SymbolLocation>
+Marshaller::toProtobuf(const clangd::SymbolLocation &Location) {
+ remote::SymbolLocation Result;
+ auto RelativePath = uriToRelativePath(Location.FileURI);
+ if (!RelativePath)
+ return llvm::None;
+ *Result.mutable_file_path() = *RelativePath;
+ *Result.mutable_start() = toProtobuf(Location.Start);
+ *Result.mutable_end() = toProtobuf(Location.End);
+ return Result;
+}
+
+llvm::Optional<HeaderWithReferences> Marshaller::toProtobuf(
+ const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) {
+ HeaderWithReferences Result;
+ Result.set_references(IncludeHeader.References);
+ const std::string Header = IncludeHeader.IncludeHeader.str();
+ if (isLiteralInclude(Header)) {
+ Result.set_header(Header);
+ return Result;
+ }
+ auto RelativePath = uriToRelativePath(Header);
+ if (!RelativePath)
+ return llvm::None;
+ Result.set_header(*RelativePath);
+ return Result;
+}
+
+llvm::Optional<clangd::Symbol::IncludeHeaderWithReferences>
+Marshaller::fromProtobuf(const HeaderWithReferences &Message) {
+ std::string Header = Message.header();
+ if (Header.front() != '<' && Header.front() != '"') {
+ auto URIString = relativePathToURI(Header);
+ if (!URIString)
+ return llvm::None;
+ Header = *URIString;
+ }
+ return clangd::Symbol::IncludeHeaderWithReferences{Strings.save(Header),
+ Message.references()};
+}
+
} // namespace remote
} // namespace clangd
} // namespace clang
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
@@ -29,16 +29,6 @@
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,
@@ -46,24 +36,23 @@
CallbackT Callback) const {
bool FinalResult = false;
trace::Span Tracer(RequestT::descriptor()->name());
- const auto RPCRequest = serializeRequest<ClangdRequestT, RequestT>(Request);
+ const auto RPCRequest = ProtobufMarshaller->toProtobuf(Request);
grpc::ClientContext Context;
std::chrono::system_clock::time_point Deadline =
std::chrono::system_clock::now() + DeadlineWaitingTime;
Context.set_deadline(Deadline);
auto Reader = (Stub.get()->*RPCCall)(&Context, RPCRequest);
- llvm::BumpPtrAllocator Arena;
- llvm::UniqueStringSaver Strings(Arena);
ReplyT Reply;
while (Reader->Read(&Reply)) {
if (!Reply.has_stream_result()) {
FinalResult = Reply.final_result();
continue;
}
- auto Response =
- fromProtobuf(Reply.stream_result(), &Strings, ProjectRoot);
- if (!Response)
+ auto Response = ProtobufMarshaller->fromProtobuf(Reply.stream_result());
+ if (!Response) {
elog("Received invalid {0}", ReplyT::descriptor()->name());
+ continue;
+ }
Callback(*Response);
}
SPAN_ATTACH(Tracer, "status", Reader->Finish().ok());
@@ -74,7 +63,9 @@
IndexClient(
std::shared_ptr<grpc::Channel> Channel, llvm::StringRef ProjectRoot,
std::chrono::milliseconds DeadlineTime = std::chrono::milliseconds(1000))
- : Stub(remote::SymbolIndex::NewStub(Channel)), ProjectRoot(ProjectRoot),
+ : Stub(remote::SymbolIndex::NewStub(Channel)),
+ ProtobufMarshaller(new Marshaller(/*RemoteIndexRoot=*/"",
+ /*LocalIndexRoot=*/ProjectRoot)),
DeadlineWaitingTime(DeadlineTime) {}
void lookup(const clangd::LookupRequest &Request,
@@ -105,7 +96,7 @@
private:
std::unique_ptr<remote::SymbolIndex::Stub> Stub;
- std::string ProjectRoot;
+ std::unique_ptr<Marshaller> ProtobufMarshaller;
// Each request will be terminated if it takes too long.
std::chrono::milliseconds DeadlineWaitingTime;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
