kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous.
Herald added a project: clang.
kbobyrev requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
This is a refactoring: errors should be logged only on the highest level.
Switch from Optional to Expected in the serialization code.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84939
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
@@ -97,11 +97,11 @@
"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 +109,16 @@
// 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 different from "file".
auto UnittestURI =
@@ -122,14 +126,18 @@
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 +150,9 @@
// 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 +164,44 @@
// Missing definition is OK.
Sym.Definition = clangd::SymbolLocation();
Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
- EXPECT_TRUE(ProtobufMarshaller.fromProtobuf(*Serialized));
+ ASSERT_TRUE(bool(Serialized));
+ EXPECT_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 +224,9 @@
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));
}
@@ -243,17 +259,7 @@
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;
-
+ clangd::Symbol Sym = createSymbol("remote/", Strings);
// Try to serialize all headers but only valid ones will end up in Protobuf
// message.
auto AllHeaders = ValidHeaders;
@@ -264,11 +270,11 @@
Marshaller ProtobufMarshaller(convert_to_slash("/"), convert_to_slash("/"));
auto Serialized = ProtobufMarshaller.toProtobuf(Sym);
- ASSERT_TRUE(Serialized);
+ ASSERT_TRUE(bool(Serialized));
EXPECT_EQ(static_cast<size_t>(Serialized->headers_size()),
ValidHeaders.size());
auto Deserialized = ProtobufMarshaller.fromProtobuf(*Serialized);
- ASSERT_TRUE(Deserialized);
+ ASSERT_TRUE(bool(Deserialized));
Sym.IncludeHeaders = ValidHeaders;
EXPECT_EQ(toYAML(Sym), toYAML(*Deserialized));
@@ -294,7 +300,7 @@
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 +346,7 @@
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 +381,7 @@
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 +393,9 @@
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 +403,16 @@
TEST(RemoteMarshallingTest, RelativePathToURITranslation) {
Marshaller ProtobufMarshaller(/*RemoteIndexRoot=*/"",
/*LocalIndexRoot=*/testPath("home/project/"));
- EXPECT_TRUE(ProtobufMarshaller.relativePathToURI("lib/File.cpp"));
+ auto URIString = ProtobufMarshaller.relativePathToURI("lib/File.cpp");
+ EXPECT_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 +420,17 @@
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));
+ EXPECT_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
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
@@ -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"
@@ -85,7 +86,8 @@
trace::Span Tracer(LookupRequest::descriptor()->name());
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
- elog("Can not parse LookupRequest from protobuf: {0}", Req.takeError());
+ elog("Can not parse {0} from protobuf: {1}",
+ LookupRequest::descriptor()->name(), Req.takeError());
return grpc::Status::CANCELLED;
}
unsigned Sent = 0;
@@ -93,6 +95,9 @@
Index->lookup(*Req, [&](const auto &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Symbol to protobuf: {0}",
+ SerializedItem.takeError());
+ llvm::consumeError(SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -115,8 +120,8 @@
trace::Span Tracer(FuzzyFindRequest::descriptor()->name());
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
- elog("Can not parse FuzzyFindRequest from protobuf: {0}",
- Req.takeError());
+ elog("Can not parse {0} from protobuf: {1}",
+ LookupRequest::descriptor()->name(), Req.takeError());
return grpc::Status::CANCELLED;
}
unsigned Sent = 0;
@@ -124,6 +129,9 @@
bool HasMore = Index->fuzzyFind(*Req, [&](const auto &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Symbol to protobuf: {0}",
+ SerializedItem.takeError());
+ llvm::consumeError(SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -145,7 +153,8 @@
trace::Span Tracer(RefsRequest::descriptor()->name());
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
- elog("Can not parse RefsRequest from protobuf: {0}", Req.takeError());
+ elog("Can not parse {0} from protobuf: {1}",
+ RefsRequest::descriptor()->name(), Req.takeError());
return grpc::Status::CANCELLED;
}
unsigned Sent = 0;
@@ -153,6 +162,9 @@
bool HasMore = Index->refs(*Req, [&](const auto &Item) {
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
if (!SerializedItem) {
+ elog("Unable to convert Ref to protobuf: {0}",
+ SerializedItem.takeError());
+ llvm::consumeError(SerializedItem.takeError());
++FailedToSend;
return;
}
@@ -175,8 +187,8 @@
trace::Span Tracer(RelationsRequest::descriptor()->name());
auto Req = ProtobufMarshaller->fromProtobuf(Request);
if (!Req) {
- elog("Can not parse RelationsRequest from protobuf: {0}",
- Req.takeError());
+ elog("Can not parse {0} from protobuf: {1}",
+ RelationsRequest::descriptor()->name(), Req.takeError());
return grpc::Status::CANCELLED;
}
unsigned Sent = 0;
@@ -185,6 +197,9 @@
*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());
+ llvm::consumeError(SerializedItem.takeError());
++FailedToSend;
return;
}
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
@@ -38,10 +38,9 @@
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 @@
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 @@
/// 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
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
@@ -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"
@@ -48,6 +46,10 @@
} // namespace
+using llvm::inconvertibleErrorCode;
+using llvm::make_error;
+using llvm::StringError;
+
Marshaller::Marshaller(llvm::StringRef RemoteIndexRoot,
llvm::StringRef LocalIndexRoot)
: Strings(Arena) {
@@ -124,20 +126,14 @@
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 make_error<StringError>("Missing info, definition or declaration.",
+ inconvertibleErrorCode());
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 @@
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());
@@ -167,46 +160,38 @@
if (SerializedHeader)
Result.IncludeHeaders.push_back(*SerializedHeader);
else
- elog("Cannot convert HeaderWithIncludes from protobuf: {0}",
- Header.DebugString());
+ elog("Cannot convert HeaderWithIncludes from protobuf; skipping it: {0}. "
+ "Reason: {1}",
+ Header.DebugString(), SerializedHeader.takeError());
}
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 make_error<StringError>("Missing location.",
+ inconvertibleErrorCode());
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 (!SubjectID)
+ return SubjectID.takeError();
if (!Message.has_object()) {
- elog("Cannot convert Relation from protobuf (missing Object): {0}");
- return llvm::None;
+ return make_error<StringError>("Missing Object.", inconvertibleErrorCode());
}
auto Object = fromProtobuf(Message.object());
if (!Object)
- return llvm::None;
+ return Object.takeError();
return std::make_pair(*SubjectID, *Object);
}
@@ -258,27 +243,21 @@
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));
@@ -292,8 +271,10 @@
for (const auto &Header : From.IncludeHeaders) {
auto Serialized = toProtobuf(Header);
if (!Serialized) {
- elog("Can not convert IncludeHeaderWithReferences to protobuf: {0}",
- Header.IncludeHeader);
+ elog("Can not convert IncludeHeaderWithReferences {0} to protobuf; "
+ "skipping it. Reason: {1}",
+ Header.IncludeHeader, Serialized.takeError());
+ llvm::consumeError(Serialized.takeError());
continue;
}
auto *NextHeader = Result.add_headers();
@@ -303,64 +284,57 @@
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 make_error<StringError>("Empty relative path.",
+ inconvertibleErrorCode());
+ if (llvm::sys::path::is_absolute(RelativePath))
+ return make_error<StringError>("RelativePath is absolute.",
+ inconvertibleErrorCode());
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 make_error<StringError>("Can not use URI schemes other than file.",
+ inconvertibleErrorCode());
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 make_error<StringError>(
+ "File path doesn't start with RemoteIndexRoot.",
+ inconvertibleErrorCode());
// Make sure the result has UNIX slashes.
return llvm::sys::path::convert_to_slash(Result,
llvm::sys::path::Style::posix);
@@ -401,31 +375,31 @@
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 +410,18 @@
}
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() != '"') {
auto URIString = relativePathToURI(Header);
if (!URIString)
- return llvm::None;
+ return URIString.takeError();
Header = *URIString;
}
return clangd::Symbol::IncludeHeaderWithReferences{Strings.save(Header),
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
@@ -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,10 @@
}
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.DebugString(),
+ Response.takeError());
+ llvm::consumeError(Response.takeError());
++FailedToParse;
continue;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
