[clang-tools-extra] fb5588b - [clangd] Propagate remote index errors via Expected
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 31 02:48:54 PDT 2020
Author: Kirill Bobyrev
Date: 2020-07-31T11:48:32+02:00
New Revision: fb5588b0ad59522031d037b0d1a3fdcf8ada8a79
URL: https://github.com/llvm/llvm-project/commit/fb5588b0ad59522031d037b0d1a3fdcf8ada8a79
DIFF: https://github.com/llvm/llvm-project/commit/fb5588b0ad59522031d037b0d1a3fdcf8ada8a79.diff
LOG: [clangd] Propagate remote index errors via Expected
This is a refactoring: errors should be logged only on the highest level.
Switch from Optional to Expected in the serialization code.
Reviewed By: kadircet
Differential Revision: https://reviews.llvm.org/D84939
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/index/remote/Client.cpp b/clang-tools-extra/clangd/index/remote/Client.cpp
index af58645d1795..4c1741e715a5 100644
--- a/clang-tools-extra/clangd/index/remote/Client.cpp
+++ b/clang-tools-extra/clangd/index/remote/Client.cpp
@@ -16,6 +16,7 @@
#include "support/Logger.h"
#include "support/Trace.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
#include <chrono>
@@ -53,7 +54,9 @@ class IndexClient : public clangd::SymbolIndex {
}
auto Response = ProtobufMarshaller->fromProtobuf(Reply.stream_result());
if (!Response) {
- elog("Received invalid {0}", ReplyT::descriptor()->name());
+ elog("Received invalid {0}: {1}. Reason: {2}",
+ ReplyT::descriptor()->name(), Reply.stream_result().DebugString(),
+ Response.takeError());
++FailedToParse;
continue;
}
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
index 270e15347ee0..bac5b7e6e958 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.cpp
@@ -19,8 +19,6 @@
#include "support/Logger.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/DenseSet.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"
@@ -46,6 +44,11 @@ llvm::Expected<llvm::DenseSet<SymbolID>> getIDs(IDRange IDs) {
return Result;
}
+llvm::Error makeStringError(llvm::StringRef Message) {
+ return llvm::make_error<llvm::StringError>(Message,
+ llvm::inconvertibleErrorCode());
+}
+
} // namespace
Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
@@ -124,20 +127,13 @@ Marshaller::fromProtobuf(const RelationsRequest *Message) {
return Req;
}
-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;
- }
+llvm::Expected<clangd::Symbol> Marshaller::fromProtobuf(const Symbol &Message) {
+ if (!Message.has_info() || !Message.has_canonical_declaration())
+ return makeStringError("Missing info or declaration.");
clangd::Symbol Result;
auto ID = SymbolID::fromStr(Message.id());
- if (!ID) {
- elog("Cannot parse SymbolID {0} given protobuf: {1}", ID.takeError(),
- Message.DebugString());
- return llvm::None;
- }
+ if (!ID)
+ return ID.takeError();
Result.ID = *ID;
Result.SymInfo = fromProtobuf(Message.info());
Result.Name = Message.name();
@@ -148,11 +144,8 @@ llvm::Optional<clangd::Symbol> Marshaller::fromProtobuf(const Symbol &Message) {
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;
- }
+ if (!Declaration)
+ return Declaration.takeError();
Result.CanonicalDeclaration = *Declaration;
Result.References = Message.references();
Result.Origin = static_cast<clangd::SymbolOrigin>(Message.origin());
@@ -164,49 +157,36 @@ llvm::Optional<clangd::Symbol> Marshaller::fromProtobuf(const Symbol &Message) {
Result.Type = Message.type();
for (const auto &Header : Message.headers()) {
auto SerializedHeader = fromProtobuf(Header);
- if (SerializedHeader)
- Result.IncludeHeaders.push_back(*SerializedHeader);
- else
- elog("Cannot convert HeaderWithIncludes from protobuf: {0}",
- Header.DebugString());
+ if (!SerializedHeader)
+ return SerializedHeader.takeError();
+ Result.IncludeHeaders.push_back(*SerializedHeader);
}
Result.Flags = static_cast<clangd::Symbol::SymbolFlag>(Message.flags());
return Result;
}
-llvm::Optional<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
- if (!Message.has_location()) {
- elog("Cannot convert Ref from protobuf (missing location): {0}",
- Message.DebugString());
- return llvm::None;
- }
+llvm::Expected<clangd::Ref> Marshaller::fromProtobuf(const Ref &Message) {
+ if (!Message.has_location())
+ return makeStringError("Missing location.");
clangd::Ref Result;
auto Location = fromProtobuf(Message.location());
- if (!Location) {
- elog("Cannot convert Ref from protobuf (invalid location): {0}",
- Message.DebugString());
- return llvm::None;
- }
+ if (!Location)
+ return Location.takeError();
Result.Location = *Location;
Result.Kind = static_cast<clangd::RefKind>(Message.kind());
return Result;
}
-llvm::Optional<std::pair<clangd::SymbolID, clangd::Symbol>>
+llvm::Expected<std::pair<clangd::SymbolID, clangd::Symbol>>
Marshaller::fromProtobuf(const Relation &Message) {
auto SubjectID = SymbolID::fromStr(Message.subject_id());
- if (!SubjectID) {
- elog("Cannot convert Relation from protobuf (invalid Subject ID): {0}",
- SubjectID.takeError());
- return llvm::None;
- }
- if (!Message.has_object()) {
- elog("Cannot convert Relation from protobuf (missing Object): {0}");
- return llvm::None;
- }
+ if (!SubjectID)
+ return SubjectID.takeError();
+ if (!Message.has_object())
+ return makeStringError("Missing Object.");
auto Object = fromProtobuf(Message.object());
if (!Object)
- return llvm::None;
+ return Object.takeError();
return std::make_pair(*SubjectID, *Object);
}
@@ -258,27 +238,21 @@ RelationsRequest Marshaller::toProtobuf(const clangd::RelationsRequest &From) {
return RPCRequest;
}
-llvm::Optional<Symbol> Marshaller::toProtobuf(const clangd::Symbol &From) {
+llvm::Expected<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());
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;
- }
+ if (!Definition)
+ return Definition.takeError();
*Result.mutable_definition() = *Definition;
}
Result.set_scope(From.Scope.str());
auto Declaration = toProtobuf(From.CanonicalDeclaration);
- if (!Declaration) {
- elog("Can not convert Symbol to protobuf (invalid declaration) {0}: {1}",
- From, From.CanonicalDeclaration);
- return llvm::None;
- }
+ if (!Declaration)
+ return Declaration.takeError();
*Result.mutable_canonical_declaration() = *Declaration;
Result.set_references(From.References);
Result.set_origin(static_cast<uint32_t>(From.Origin));
@@ -291,11 +265,8 @@ llvm::Optional<Symbol> Marshaller::toProtobuf(const clangd::Symbol &From) {
Result.set_type(From.Type.str());
for (const auto &Header : From.IncludeHeaders) {
auto Serialized = toProtobuf(Header);
- if (!Serialized) {
- elog("Can not convert IncludeHeaderWithReferences to protobuf: {0}",
- Header.IncludeHeader);
- continue;
- }
+ if (!Serialized)
+ return Serialized.takeError();
auto *NextHeader = Result.add_headers();
*NextHeader = *Serialized;
}
@@ -303,64 +274,52 @@ llvm::Optional<Symbol> Marshaller::toProtobuf(const clangd::Symbol &From) {
return Result;
}
-llvm::Optional<Ref> Marshaller::toProtobuf(const clangd::Ref &From) {
+llvm::Expected<Ref> Marshaller::toProtobuf(const clangd::Ref &From) {
Ref Result;
Result.set_kind(static_cast<uint32_t>(From.Kind));
auto Location = toProtobuf(From.Location);
- if (!Location) {
- elog("Can not convert Reference to protobuf (invalid location) {0}: {1}",
- From, From.Location);
- return llvm::None;
- }
+ if (!Location)
+ return Location.takeError();
*Result.mutable_location() = *Location;
return Result;
}
-llvm::Optional<Relation> Marshaller::toProtobuf(const clangd::SymbolID &Subject,
+llvm::Expected<Relation> Marshaller::toProtobuf(const clangd::SymbolID &Subject,
const clangd::Symbol &Object) {
Relation Result;
*Result.mutable_subject_id() = Subject.str();
auto SerializedObject = toProtobuf(Object);
- if (!SerializedObject) {
- elog("Can not convert Relation to protobuf (invalid symbol): {0}", Object);
- return llvm::None;
- }
+ if (!SerializedObject)
+ return SerializedObject.takeError();
*Result.mutable_object() = *SerializedObject;
return Result;
}
-llvm::Optional<std::string>
+llvm::Expected<std::string>
Marshaller::relativePathToURI(llvm::StringRef RelativePath) {
assert(LocalIndexRoot);
assert(RelativePath == llvm::sys::path::convert_to_slash(
RelativePath, llvm::sys::path::Style::posix));
- if (RelativePath.empty()) {
- return llvm::None;
- }
- if (llvm::sys::path::is_absolute(RelativePath)) {
- return llvm::None;
- }
+ if (RelativePath.empty())
+ return makeStringError("Empty relative path.");
+ if (llvm::sys::path::is_absolute(RelativePath))
+ return makeStringError("RelativePath is absolute.");
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> Marshaller::uriToRelativePath(llvm::StringRef URI) {
+llvm::Expected<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,
- ParsedURI.takeError());
- return llvm::None;
- }
- if (ParsedURI->scheme() != "file") {
- return llvm::None;
- }
+ if (!ParsedURI)
+ return ParsedURI.takeError();
+ if (ParsedURI->scheme() != "file")
+ return makeStringError("Can not use URI schemes other than file.");
llvm::SmallString<256> Result = ParsedURI->body();
- if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, "")) {
- return llvm::None;
- }
+ if (!llvm::sys::path::replace_path_prefix(Result, *RemoteIndexRoot, ""))
+ return makeStringError("File path doesn't start with RemoteIndexRoot.");
// Make sure the result has UNIX slashes.
return llvm::sys::path::convert_to_slash(Result,
llvm::sys::path::Style::posix);
@@ -401,31 +360,31 @@ SymbolInfo Marshaller::toProtobuf(const clang::index::SymbolInfo &Info) {
return Result;
}
-llvm::Optional<clangd::SymbolLocation>
+llvm::Expected<clangd::SymbolLocation>
Marshaller::fromProtobuf(const SymbolLocation &Message) {
clangd::SymbolLocation Location;
auto URIString = relativePathToURI(Message.file_path());
if (!URIString)
- return llvm::None;
+ return URIString.takeError();
Location.FileURI = Strings.save(*URIString).begin();
Location.Start = fromProtobuf(Message.start());
Location.End = fromProtobuf(Message.end());
return Location;
}
-llvm::Optional<SymbolLocation>
+llvm::Expected<SymbolLocation>
Marshaller::toProtobuf(const clangd::SymbolLocation &Location) {
remote::SymbolLocation Result;
auto RelativePath = uriToRelativePath(Location.FileURI);
if (!RelativePath)
- return llvm::None;
+ return RelativePath.takeError();
*Result.mutable_file_path() = *RelativePath;
*Result.mutable_start() = toProtobuf(Location.Start);
*Result.mutable_end() = toProtobuf(Location.End);
return Result;
}
-llvm::Optional<HeaderWithReferences> Marshaller::toProtobuf(
+llvm::Expected<HeaderWithReferences> Marshaller::toProtobuf(
const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader) {
HeaderWithReferences Result;
Result.set_references(IncludeHeader.References);
@@ -436,18 +395,18 @@ llvm::Optional<HeaderWithReferences> Marshaller::toProtobuf(
}
auto RelativePath = uriToRelativePath(Header);
if (!RelativePath)
- return llvm::None;
+ return RelativePath.takeError();
Result.set_header(*RelativePath);
return Result;
}
-llvm::Optional<clangd::Symbol::IncludeHeaderWithReferences>
+llvm::Expected<clangd::Symbol::IncludeHeaderWithReferences>
Marshaller::fromProtobuf(const HeaderWithReferences &Message) {
std::string Header = Message.header();
- if (Header.front() != '<' && Header.front() != '"') {
+ if (!isLiteralInclude(Header)) {
auto URIString = relativePathToURI(Header);
if (!URIString)
- return llvm::None;
+ return URIString.takeError();
Header = *URIString;
}
return clangd::Symbol::IncludeHeaderWithReferences{Strings.save(Header),
diff --git a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
index 9588f1ef6411..18ce6074264c 100644
--- a/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
+++ b/clang-tools-extra/clangd/index/remote/marshalling/Marshalling.h
@@ -38,10 +38,9 @@ class Marshaller {
Marshaller() = delete;
Marshaller(llvm::StringRef RemoteIndexRoot, llvm::StringRef LocalIndexRoot);
- // FIXME(kirillbobyrev): Switch from Optional to Expected.
- llvm::Optional<clangd::Symbol> fromProtobuf(const Symbol &Message);
- llvm::Optional<clangd::Ref> fromProtobuf(const Ref &Message);
- llvm::Optional<std::pair<clangd::SymbolID, clangd::Symbol>>
+ llvm::Expected<clangd::Symbol> fromProtobuf(const Symbol &Message);
+ llvm::Expected<clangd::Ref> fromProtobuf(const Ref &Message);
+ llvm::Expected<std::pair<clangd::SymbolID, clangd::Symbol>>
fromProtobuf(const Relation &Message);
llvm::Expected<clangd::LookupRequest>
@@ -61,9 +60,9 @@ class Marshaller {
RefsRequest toProtobuf(const clangd::RefsRequest &From);
RelationsRequest toProtobuf(const clangd::RelationsRequest &From);
- llvm::Optional<Symbol> toProtobuf(const clangd::Symbol &From);
- llvm::Optional<Ref> toProtobuf(const clangd::Ref &From);
- llvm::Optional<Relation> toProtobuf(const clangd::SymbolID &Subject,
+ llvm::Expected<Symbol> toProtobuf(const clangd::Symbol &From);
+ llvm::Expected<Ref> toProtobuf(const clangd::Ref &From);
+ llvm::Expected<Relation> toProtobuf(const clangd::SymbolID &Subject,
const clangd::Symbol &Object);
/// Translates \p RelativePath into the absolute path and builds URI for the
@@ -72,23 +71,23 @@ class Marshaller {
/// provided by the client.
///
/// The relative path passed over the wire has unix slashes.
- llvm::Optional<std::string> relativePathToURI(llvm::StringRef RelativePath);
+ llvm::Expected<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);
+ llvm::Expected<std::string> uriToRelativePath(llvm::StringRef URI);
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>
+ llvm::Expected<clangd::SymbolLocation>
fromProtobuf(const SymbolLocation &Message);
- llvm::Optional<SymbolLocation>
+ llvm::Expected<SymbolLocation>
toProtobuf(const clangd::SymbolLocation &Location);
- llvm::Optional<HeaderWithReferences>
+ llvm::Expected<HeaderWithReferences>
toProtobuf(const clangd::Symbol::IncludeHeaderWithReferences &IncludeHeader);
- llvm::Optional<clangd::Symbol::IncludeHeaderWithReferences>
+ llvm::Expected<clangd::Symbol::IncludeHeaderWithReferences>
fromProtobuf(const HeaderWithReferences &Message);
/// RemoteIndexRoot and LocalIndexRoot are absolute paths to the project (on
diff --git a/clang-tools-extra/clangd/index/remote/server/Server.cpp b/clang-tools-extra/clangd/index/remote/server/Server.cpp
index 27e89cbbb48e..e9838cce85e3 100644
--- a/clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ b/clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -15,6 +15,7 @@
#include "support/Trace.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Signals.h"
@@ -82,7 +83,7 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status Lookup(grpc::ServerContext *Context,
const LookupRequest *Request,
grpc::ServerWriter<LookupReply> *Reply) override {
- trace::Span Tracer(LookupRequest::descriptor()->name());
+ trace::Span Tracer("LookupRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
elog("Can not parse LookupRequest from protobuf: {0}", Req.takeError());
@@ -93,6 +94,8 @@ class RemoteIndexServer final : public SymbolIndex::Service {
Index->lookup(*Req, [&](const clangd::Symbol &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Symbol to protobuf: {0}",
+ SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -112,7 +115,7 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status FuzzyFind(grpc::ServerContext *Context,
const FuzzyFindRequest *Request,
grpc::ServerWriter<FuzzyFindReply> *Reply) override {
- trace::Span Tracer(FuzzyFindRequest::descriptor()->name());
+ trace::Span Tracer("FuzzyFindRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
elog("Can not parse FuzzyFindRequest from protobuf: {0}",
@@ -124,6 +127,8 @@ class RemoteIndexServer final : public SymbolIndex::Service {
bool HasMore = Index->fuzzyFind(*Req, [&](const clangd::Symbol &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Symbol to protobuf: {0}",
+ SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -142,7 +147,7 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status Refs(grpc::ServerContext *Context, const RefsRequest *Request,
grpc::ServerWriter<RefsReply> *Reply) override {
- trace::Span Tracer(RefsRequest::descriptor()->name());
+ trace::Span Tracer("RefsRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
elog("Can not parse RefsRequest from protobuf: {0}", Req.takeError());
@@ -153,6 +158,8 @@ class RemoteIndexServer final : public SymbolIndex::Service {
bool HasMore = Index->refs(*Req, [&](const clangd::Ref &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Ref to protobuf: {0}",
+ SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -172,7 +179,7 @@ class RemoteIndexServer final : public SymbolIndex::Service {
grpc::Status Relations(grpc::ServerContext *Context,
const RelationsRequest *Request,
grpc::ServerWriter<RelationsReply> *Reply) override {
- trace::Span Tracer(RelationsRequest::descriptor()->name());
+ trace::Span Tracer("RelationsRequest");
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
elog("Can not parse RelationsRequest from protobuf: {0}",
@@ -185,6 +192,8 @@ class RemoteIndexServer final : public SymbolIndex::Service {
*Req, [&](const SymbolID &Subject, const clangd::Symbol &Object) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Subject, Object);
if (!SerializedItem) {
+ elog("Unable to convert Relation to protobuf: {0}",
+ SerializedItem.takeError());
++FailedToSend;
return;
}
diff --git a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
index 6abf16a8dd49..ff819146621a 100644
--- a/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "../TestTU.h"
+#include "Index.pb.h"
#include "TestFS.h"
#include "index/Index.h"
#include "index/Ref.h"
@@ -97,11 +98,11 @@ TEST(RemoteMarshallingTest, URITranslation) {
"clangd/unittests/remote/MarshallingTests.cpp",
Strings);
auto Serialized = ProtobufMarshaller.toProtobuf(Original);
- ASSERT_TRUE(Serialized);
+ ASSERT_TRUE(bool(Serialized));
EXPECT_EQ(Serialized->location().file_path(),
"clang-tools-extra/clangd/unittests/remote/MarshallingTests.cpp");
auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
EXPECT_STREQ(Deserialized->Location.FileURI,
testPathURI("home/my-projects/llvm-project/clang-tools-extra/"
"clangd/unittests/remote/MarshallingTests.cpp",
@@ -109,12 +110,16 @@ TEST(RemoteMarshallingTest, URITranslation) {
// Can't have empty paths.
*Serialized->mutable_location()->mutable_file_path() = std::string();
- EXPECT_FALSE(ProtobufMarshaller.fromProtobuf(*Serialized));
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_FALSE(bool(Deserialized));
+ llvm::consumeError(Deserialized.takeError());
clangd::Ref WithInvalidURI;
// Invalid URI results in serialization failure.
WithInvalidURI.Location.FileURI = "This is not a URI";
- EXPECT_FALSE(ProtobufMarshaller.toProtobuf(WithInvalidURI));
+ auto DeserializedRef = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+ EXPECT_FALSE(bool(DeserializedRef));
+ llvm::consumeError(DeserializedRef.takeError());
// Can not use URIs with scheme
diff erent from "file".
auto UnittestURI =
@@ -122,14 +127,18 @@ TEST(RemoteMarshallingTest, URITranslation) {
ASSERT_TRUE(bool(UnittestURI));
WithInvalidURI.Location.FileURI =
Strings.save(UnittestURI->toString()).begin();
- EXPECT_FALSE(ProtobufMarshaller.toProtobuf(WithInvalidURI));
+ auto DeserializedSymbol = ProtobufMarshaller.toProtobuf(WithInvalidURI);
+ EXPECT_FALSE(bool(DeserializedSymbol));
+ llvm::consumeError(DeserializedSymbol.takeError());
// 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";
- EXPECT_FALSE(ProtobufMarshaller.fromProtobuf(WithAbsolutePath));
+ Deserialized = ProtobufMarshaller.fromProtobuf(WithAbsolutePath);
+ EXPECT_FALSE(bool(Deserialized));
+ llvm::consumeError(Deserialized.takeError());
}
TEST(RemoteMarshallingTest, SymbolSerialization) {
@@ -142,9 +151,9 @@ TEST(RemoteMarshallingTest, SymbolSerialization) {
// Check that symbols are exactly the same if the path to indexed project is
// the same on indexing machine and the client.
auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
+ ASSERT_TRUE(bool(Serialized));
auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
// Serialized paths are relative and have UNIX slashes.
EXPECT_EQ(convert_to_slash(Serialized->definition().file_path(),
@@ -156,36 +165,44 @@ TEST(RemoteMarshallingTest, SymbolSerialization) {
// Missing definition is OK.
Sym.Definition = clangd::SymbolLocation();
Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
- EXPECT_TRUE(ProtobufMarshaller.fromProtobuf(*Serialized));
+ ASSERT_TRUE(bool(Serialized));
+ ASSERT_TRUE(bool(ProtobufMarshaller.fromProtobuf(*Serialized)));
// Relative path is absolute.
*Serialized->mutable_canonical_declaration()->mutable_file_path() =
convert_to_slash("/path/to/Declaration.h");
- EXPECT_FALSE(ProtobufMarshaller.fromProtobuf(*Serialized));
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_FALSE(bool(Deserialized));
+ llvm::consumeError(Deserialized.takeError());
// Fail with an invalid URI.
Sym.Definition.FileURI = "Not A URI";
- EXPECT_FALSE(ProtobufMarshaller.toProtobuf(Sym));
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(bool(Serialized));
+ llvm::consumeError(Serialized.takeError());
// Schemes other than "file" can not be used.
auto UnittestURI = URI::create(testPath("home/SomePath.h"), "unittest");
ASSERT_TRUE(bool(UnittestURI));
Sym.Definition.FileURI = Strings.save(UnittestURI->toString()).begin();
- EXPECT_FALSE(ProtobufMarshaller.toProtobuf(Sym));
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(bool(Serialized));
+ llvm::consumeError(Serialized.takeError());
// Passing root that is not prefix of the original file path.
Sym.Definition.FileURI = testPathURI("home/File.h", Strings);
// Check that the symbol is valid and passing the correct path works.
Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
+ ASSERT_TRUE(bool(Serialized));
Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
EXPECT_STREQ(Deserialized->Definition.FileURI,
testPathURI("home/File.h", Strings));
// Fail with a wrong root.
Marshaller WrongMarshaller(testPath("nothome/"), testPath("home/"));
- EXPECT_FALSE(WrongMarshaller.toProtobuf(Sym));
+ Serialized = WrongMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(Serialized);
+ llvm::consumeError(Serialized.takeError());
}
TEST(RemoteMarshallingTest, RefSerialization) {
@@ -208,9 +225,9 @@ TEST(RemoteMarshallingTest, RefSerialization) {
testPath("llvm-project/"));
auto Serialized = ProtobufMarshaller.toProtobuf(Ref);
- ASSERT_TRUE(Serialized);
+ ASSERT_TRUE(bool(Serialized));
auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
EXPECT_EQ(toYAML(Ref), toYAML(*Deserialized));
}
@@ -218,60 +235,60 @@ TEST(RemoteMarshallingTest, IncludeHeaderURIs) {
llvm::BumpPtrAllocator Arena;
llvm::UniqueStringSaver Strings(Arena);
- llvm::SmallVector<clangd::Symbol::IncludeHeaderWithReferences, 1>
- ValidHeaders;
+ clangd::Symbol Sym = createSymbol("remote/", Strings);
+
clangd::Symbol::IncludeHeaderWithReferences Header;
+ // Add only valid headers.
Header.IncludeHeader = Strings.save(
URI::createFile("/usr/local/user/home/project/Header.h").toString());
Header.References = 21;
- ValidHeaders.push_back(Header);
+ Sym.IncludeHeaders.push_back(Header);
Header.IncludeHeader = Strings.save("<iostream>");
Header.References = 100;
- ValidHeaders.push_back(Header);
+ Sym.IncludeHeaders.push_back(Header);
Header.IncludeHeader = Strings.save("\"cstdio\"");
Header.References = 200;
- ValidHeaders.push_back(Header);
+ Sym.IncludeHeaders.push_back(Header);
+
+ Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/"));
+
+ auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ ASSERT_TRUE(bool(Serialized));
+ EXPECT_EQ(static_cast<size_t>(Serialized->headers_size()),
+ Sym.IncludeHeaders.size());
+ auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ ASSERT_TRUE(bool(Deserialized));
+ EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
- 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);
+ Sym.IncludeHeaders.push_back(Header);
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(bool(Serialized));
+ llvm::consumeError(Serialized.takeError());
+
+ // Remove last invalid header.
+ Sym.IncludeHeaders.pop_back();
// 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;
- // Fill in definition and declaration, Symbool will be invalid otherwise.
- clangd::SymbolLocation Location;
- Location.Start.setLine(1);
- Location.Start.setColumn(2);
- Location.End.setLine(3);
- Location.End.setColumn(4);
- Location.FileURI = testPathURI("File.h", Strings);
- Sym.Definition = Location;
- Sym.CanonicalDeclaration = Location;
-
- // 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;
-
- Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/"));
-
- auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
- EXPECT_EQ(static_cast<size_t>(Serialized->headers_size()),
- ValidHeaders.size());
- auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ Sym.IncludeHeaders.push_back(Header);
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ EXPECT_FALSE(bool(Serialized));
+ llvm::consumeError(Serialized.takeError());
- Sym.IncludeHeaders = ValidHeaders;
- EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
+ // Try putting an invalid header into already serialized symbol.
+ Sym.IncludeHeaders.pop_back();
+ Serialized = ProtobufMarshaller.toProtobuf(Sym);
+ ASSERT_TRUE(bool(Serialized));
+ HeaderWithReferences InvalidHeader;
+ InvalidHeader.set_header(convert_to_slash("/absolute/path/Header.h"));
+ InvalidHeader.set_references(9000);
+ *Serialized->add_headers() = InvalidHeader;
+ Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
+ EXPECT_FALSE(bool(Deserialized));
+ llvm::consumeError(Deserialized.takeError());
}
TEST(RemoteMarshallingTest, LookupRequestSerialization) {
@@ -294,7 +311,7 @@ TEST(RemoteMarshallingTest, LookupRequestFailingSerialization) {
auto Serialized = ProtobufMarshaller.toProtobuf(Request);
Serialized.add_ids("Invalid Symbol ID");
auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
- EXPECT_FALSE(Deserialized);
+ EXPECT_FALSE(bool(Deserialized));
llvm::consumeError(Deserialized.takeError());
}
@@ -340,7 +357,7 @@ TEST(RemoteMarshallingTest, RefsRequestFailingSerialization) {
auto Serialized = ProtobufMarshaller.toProtobuf(Request);
Serialized.add_ids("Invalid Symbol ID");
auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
- EXPECT_FALSE(Deserialized);
+ EXPECT_FALSE(bool(Deserialized));
llvm::consumeError(Deserialized.takeError());
}
@@ -375,7 +392,7 @@ TEST(RemoteMarshallingTest, RelationsRequestFailingSerialization) {
Serialized.add_subjects("ZZZZZZZZZZZZZZZZ");
Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
auto Deserialized = ProtobufMarshaller.fromProtobuf(&Serialized);
- EXPECT_FALSE(Deserialized);
+ EXPECT_FALSE(bool(Deserialized));
llvm::consumeError(Deserialized.takeError());
}
@@ -387,8 +404,9 @@ TEST(RemoteMarshallingTest, RelationsSerializion) {
SymbolID ID = llvm::cantFail(SymbolID::fromStr("0000000000000002"));
Marshaller ProtobufMarshaller(testPath("remote/"), testPath("local/"));
auto Serialized = ProtobufMarshaller.toProtobuf(ID, Sym);
+ ASSERT_TRUE(bool(Serialized));
auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
EXPECT_THAT(Deserialized->first, ID);
EXPECT_THAT(Deserialized->second.ID, Sym.ID);
}
@@ -396,11 +414,16 @@ TEST(RemoteMarshallingTest, RelationsSerializion) {
TEST(RemoteMarshallingTest, RelativePathToURITranslation) {
Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"",
/*LocalIndexRoot=*/testPath("home/project/"));
- EXPECT_TRUE(ProtobufMarshaller.relativePathToURI("lib/File.cpp"));
+ auto URIString = ProtobufMarshaller.relativePathToURI("lib/File.cpp");
+ ASSERT_TRUE(bool(URIString));
// RelativePath can not be absolute.
- EXPECT_FALSE(ProtobufMarshaller.relativePathToURI("/lib/File.cpp"));
+ URIString = ProtobufMarshaller.relativePathToURI("/lib/File.cpp");
+ EXPECT_FALSE(bool(URIString));
+ llvm::consumeError(URIString.takeError());
// RelativePath can not be empty.
- EXPECT_FALSE(ProtobufMarshaller.relativePathToURI(std::string()));
+ URIString = ProtobufMarshaller.relativePathToURI(std::string());
+ EXPECT_FALSE(bool(URIString));
+ llvm::consumeError(URIString.takeError());
}
TEST(RemoteMarshallingTest, URIToRelativePathTranslation) {
@@ -408,14 +431,17 @@ TEST(RemoteMarshallingTest, URIToRelativePathTranslation) {
llvm::UniqueStringSaver Strings(Arena);
Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/testPath("remote/project/"),
/*LocalIndexRoot=*/"");
- EXPECT_TRUE(ProtobufMarshaller.uriToRelativePath(
- testPathURI("remote/project/lib/File.cpp", Strings)));
+ auto RelativePath = ProtobufMarshaller.uriToRelativePath(
+ testPathURI("remote/project/lib/File.cpp", Strings));
+ ASSERT_TRUE(bool(RelativePath));
// 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)));
+ RelativePath = WrongMarshaller.uriToRelativePath(
+ testPathURI("remote/project/lib/File.cpp", Strings));
+ EXPECT_FALSE(bool(RelativePath));
+ llvm::consumeError(RelativePath.takeError());
}
} // namespace
More information about the cfe-commits
mailing list