All Errors must be checked

[beanz]

Summary

If an error is ever returned from any of the functions called here, the error must be joined with the Result Error before being returned otherwise the Result Error will assert on destruction.

[lhames]

I think you need to consume Result here instead of joining it:

if (auto Err = SerializationTraits<ChannelT, Error, Error>::deserialize(C, Result)) {
  consumeError(std::move(Result));
  return std::move(Err);
}

In the first check deserialization has failed, so Result doesn't contain a meaningful Error value (it's probably still success, but it would depend on how the deserializer was written). In the second check you could return Result (since it has been fully deserialized) but for symmetry with other values I think you just want to consume it and return the RPC failure: that's what we do everywhere else.

[beanz]

Updates based on feedback from Lang.

[lhames]

LGTM. Thanks Chris!

llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h

public:
   ResponseHandlerImpl(HandlerT Handler) : Handler(std::move(Handler)) {}
 
   // Handle the result by deserializing it from the channel then passing it
   // to the user defined handler.
   Error handleResponse(ChannelT &C) override {
     Error Result = Error::success();
     if (auto Err =
             SerializationTraits<ChannelT, Error, Error>::deserialize(C, Result))
-      return Err;
+      return joinErrors(std::move(Result), std::move(Err));
     if (auto Err = C.endReceiveMessage())
-      return Err;
+      return joinErrors(std::move(Result), std::move(Err));
     return Handler(std::move(Result));
   }
 
   // Abandon this response by calling the handler with an 'abandoned response'
   // error.
   void abandon() override {
     if (auto Err = Handler(this->createAbandonedResponseError())) {
       // Handlers should not fail when passed an abandoned response error.