From 9581f2dda8fac638b456f826d0f2a0e9a5fa2fea Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Tue, 30 Aug 2016 15:12:58 +0000 Subject: [PATCH] Revert "[ORC][RPC] Make the future type of an Orc RPC call Error/Expected rather than" This reverts commit r280016, and the followups of r280017, r280027, r280051, r280058, and r280059. MSVC's implementation of std::promise does not get along with llvm::Error. It uses its promised value too much like a normal value type. llvm-svn: 280100 --- llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h | 158 +++++++++------------ llvm/include/llvm/Support/Error.h | 21 --- .../unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp | 14 +- 3 files changed, 76 insertions(+), 117 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h b/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h index f9aabb9..966a496 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/RPCUtils.h @@ -17,6 +17,7 @@ #include #include +#include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ExecutionEngine/Orc/OrcError.h" @@ -60,7 +61,6 @@ public: // partially specialized. class RPCBase { protected: - // RPC Function description type. // // This class provides the information and operations needed to support the @@ -69,9 +69,12 @@ protected: // betwen the two. Both specializations have the same interface: // // Id - The function's unique identifier. - // ErrorReturn - The return type for blocking calls. + // OptionalReturn - The return type for asyncronous calls. + // ErrorReturn - The return type for synchronous calls. + // optionalToErrorReturn - Conversion from a valid OptionalReturn to an + // ErrorReturn. // readResult - Deserialize a result from a channel. - // abandon - Abandon a promised result. + // abandon - Abandon a promised (asynchronous) result. // respond - Retun a result on the channel. template class FunctionHelper {}; @@ -88,29 +91,32 @@ protected: static const FunctionIdT Id = FuncId; + typedef Optional OptionalReturn; + typedef Expected ErrorReturn; + static ErrorReturn optionalToErrorReturn(OptionalReturn &&V) { + assert(V && "Return value not available"); + return std::move(*V); + } + template - static Error readResult(ChannelT &C, std::promise &P) { + static Error readResult(ChannelT &C, std::promise &P) { RetT Val; auto Err = deserialize(C, Val); auto Err2 = endReceiveMessage(C); Err = joinErrors(std::move(Err), std::move(Err2)); - if (Err) - return Err; + if (Err) { + P.set_value(OptionalReturn()); + return Err; + } P.set_value(std::move(Val)); return Error::success(); } - static void abandon(std::promise &P) { - P.set_value( - make_error("RPC function call failed to return", - inconvertibleErrorCode())); - } - - static void consumeAbandoned(std::future &P) { - consumeError(P.get().takeError()); + static void abandon(std::promise &P) { + P.set_value(OptionalReturn()); } template @@ -142,24 +148,22 @@ protected: static const FunctionIdT Id = FuncId; + typedef bool OptionalReturn; typedef Error ErrorReturn; + static ErrorReturn optionalToErrorReturn(OptionalReturn &&V) { + assert(V && "Return value not available"); + return Error::success(); + } + template - static Error readResult(ChannelT &C, std::promise &P) { + static Error readResult(ChannelT &C, std::promise &P) { // Void functions don't have anything to deserialize, so we're good. - P.set_value(Error::success()); + P.set_value(true); return endReceiveMessage(C); } - static void abandon(std::promise &P) { - P.set_value( - make_error("RPC function call failed to return", - inconvertibleErrorCode())); - } - - static void consumeAbandoned(std::future &P) { - consumeError(P.get()); - } + static void abandon(std::promise &P) { P.set_value(false); } template static Error respond(ChannelT &C, SequenceNumberT SeqNo, @@ -374,27 +378,30 @@ public: template using Function = FunctionHelper; - /// Return type for non-blocking call primitives. + /// Return type for asynchronous call primitives. template - using NonBlockingCallResult = std::future; + using AsyncCallResult = std::future; - /// Return type for non-blocking call-with-seq primitives. + /// Return type for asynchronous call-with-seq primitives. template - using NonBlockingCallWithSeqResult = - std::pair, SequenceNumberT>; + using AsyncCallWithSeqResult = + std::pair, SequenceNumberT>; - /// Call Func on Channel C. Does not block, does not call send. Returns a pair - /// of a future result and the sequence number assigned to the result. + /// Serialize Args... to channel C, but do not call C.send(). + /// + /// Returns an error (on serialization failure) or a pair of: + /// (1) A future Optional (or future for void functions), and + /// (2) A sequence number. /// /// This utility function is primarily used for single-threaded mode support, /// where the sequence number can be used to wait for the corresponding - /// result. In multi-threaded mode the appendCallNB method, which does not + /// result. In multi-threaded mode the appendCallAsync method, which does not /// return the sequence numeber, should be preferred. template - Expected> - appendCallNBWithSeq(ChannelT &C, const ArgTs &... Args) { + Expected> + appendCallAsyncWithSeq(ChannelT &C, const ArgTs &... Args) { auto SeqNo = SequenceNumberMgr.getSequenceNumber(); - std::promise Promise; + std::promise Promise; auto Result = Promise.get_future(); OutstandingResults[SeqNo] = createOutstandingResult(std::move(Promise)); @@ -402,23 +409,21 @@ public: if (auto Err = CallHelper::call(C, SeqNo, Args...)) { abandonOutstandingResults(); - Func::consumeAbandoned(Result); return std::move(Err); } else - return NonBlockingCallWithSeqResult(std::move(Result), SeqNo); + return AsyncCallWithSeqResult(std::move(Result), SeqNo); } - /// The same as appendCallNBWithSeq, except that it calls C.send() to + /// The same as appendCallAsyncWithSeq, except that it calls C.send() to /// flush the channel after serializing the call. template - Expected> - callNBWithSeq(ChannelT &C, const ArgTs &... Args) { - auto Result = appendCallNBWithSeq(C, Args...); + Expected> + callAsyncWithSeq(ChannelT &C, const ArgTs &... Args) { + auto Result = appendCallAsyncWithSeq(C, Args...); if (!Result) return Result; if (auto Err = C.send()) { abandonOutstandingResults(); - Func::consumeAbandoned(Result->first); return std::move(Err); } return Result; @@ -426,66 +431,41 @@ public: /// Serialize Args... to channel C, but do not call send. /// Returns an error if serialization fails, otherwise returns a - /// std::future> (or a future for void functions). + /// std::future> (or a future for void functions). template - Expected> appendCallNB(ChannelT &C, - const ArgTs &... Args) { - auto FutureResAndSeqOrErr = appendCallNBWithSeq(C, Args...); - if (FutureResAndSeqOrErr) - return std::move(FutureResAndSeqOrErr->first); - return FutureResAndSeqOrErr.getError(); + Expected> appendCallAsync(ChannelT &C, + const ArgTs &... Args) { + auto ResAndSeqOrErr = appendCallAsyncWithSeq(C, Args...); + if (ResAndSeqOrErr) + return std::move(ResAndSeqOrErr->first); + return ResAndSeqOrErr.getError(); } - /// The same as appendCallNB, except that it calls C.send to flush the + /// The same as appendCallAsync, except that it calls C.send to flush the /// channel after serializing the call. template - Expected> callNB(ChannelT &C, - const ArgTs &... Args) { - auto FutureResAndSeqOrErr = callNBWithSeq(C, Args...); - if (FutureResAndSeqOrErr) - return std::move(FutureResAndSeqOrErr->first); - return FutureResAndSeqOrErr.getError(); - } - - /// Call Func on Channel C. Blocks waiting for a result. Returns an Error - /// for void functions or an Expected for functions returning a T. - /// - /// This function is for use in threaded code where another thread is - /// handling responses and incoming calls. - template - typename Func::ErrorReturn callB(ChannelT &C, const ArgTs &... Args) { - if (auto FutureResOrErr = callNBWithSeq(C, Args...)) { - if (auto Err = C.send()) { - abandonOutstandingResults(); - Func::consumeAbandoned(*FutureResOrErr); - return std::move(Err); - } - return FutureResOrErr->get(); - } else - return FutureResOrErr.takeError(); + Expected> callAsync(ChannelT &C, + const ArgTs &... Args) { + auto ResAndSeqOrErr = callAsyncWithSeq(C, Args...); + if (ResAndSeqOrErr) + return std::move(ResAndSeqOrErr->first); + return ResAndSeqOrErr.getError(); } - /// Call Func on Channel C. Block waiting for a result. While blocked, run - /// HandleOther to handle incoming calls (Response calls will be handled - /// implicitly before calling HandleOther). Returns an Error for void - /// functions or an Expected for functions returning a T. - /// - /// This function is for use in single threaded mode when the calling thread - /// must act as both sender and receiver. + /// This can be used in single-threaded mode. template typename Func::ErrorReturn callSTHandling(ChannelT &C, HandleFtor &HandleOther, const ArgTs &... Args) { - if (auto ResultAndSeqNoOrErr = callNBWithSeq(C, Args...)) { + if (auto ResultAndSeqNoOrErr = callAsyncWithSeq(C, Args...)) { auto &ResultAndSeqNo = *ResultAndSeqNoOrErr; if (auto Err = waitForResult(C, ResultAndSeqNo.second, HandleOther)) return std::move(Err); - return ResultAndSeqNo.first.get(); + return Func::optionalToErrorReturn(ResultAndSeqNo.first.get()); } else return ResultAndSeqNoOrErr.takeError(); } - /// Call Func on Channel C. Block waiting for a result. Returns an Error for - /// void functions or an Expected for functions returning a T. + // This can be used in single-threaded mode. template typename Func::ErrorReturn callST(ChannelT &C, const ArgTs &... Args) { return callSTHandling(C, handleNone, Args...); @@ -676,7 +656,7 @@ private: class OutstandingResultImpl : public OutstandingResult { private: public: - OutstandingResultImpl(std::promise &&P) + OutstandingResultImpl(std::promise &&P) : P(std::move(P)) {} Error readResult(ChannelT &C) override { return Func::readResult(C, P); } @@ -684,13 +664,13 @@ private: void abandon() override { Func::abandon(P); } private: - std::promise P; + std::promise P; }; // Create an outstanding result for the given function. template std::unique_ptr - createOutstandingResult(std::promise &&P) { + createOutstandingResult(std::promise &&P) { return llvm::make_unique>(std::move(P)); } diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h index d4d4a00..afd2660 100644 --- a/llvm/include/llvm/Support/Error.h +++ b/llvm/include/llvm/Support/Error.h @@ -629,27 +629,6 @@ private: typedef const typename std::remove_reference::type *const_pointer; public: - -#ifdef _MSC_VER - // WARNING: This constructor should *never* be called in user code. - // It is provided under MSVC only so that Expected can be used - // with MSVC's header, which requires types to be default - // constructible. - // - // FIXME; Kill this as soon as MSVC's implementation no longer - // requires types to be default constructible. - Expected() - : HasError(true) -#ifndef NDEBUG - , - Checked(true) -#endif // NDEBUG - { - new (getErrorStorage()) Error(); - (void)!!*getErrorStorage(); - } -#endif // _MSC_VER - /// Create an Expected error value from the given Error. Expected(Error Err) : HasError(true) diff --git a/llvm/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp index babea5c..7d55641 100644 --- a/llvm/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp +++ b/llvm/unittests/ExecutionEngine/Orc/RPCUtilsTest.cpp @@ -83,7 +83,7 @@ TEST_F(DummyRPC, TestAsyncVoidBool) { QueueChannel C2(Q2, Q1); // Make an async call. - auto ResOrErr = callNBWithSeq(C1, true); + auto ResOrErr = callAsyncWithSeq(C1, true); EXPECT_TRUE(!!ResOrErr) << "Simple call over queue failed"; { @@ -102,8 +102,8 @@ TEST_F(DummyRPC, TestAsyncVoidBool) { } // Verify that the function returned ok. - auto Err = ResOrErr->first.get(); - EXPECT_FALSE(!!Err) << "Remote void function failed to execute."; + auto Val = ResOrErr->first.get(); + EXPECT_TRUE(Val) << "Remote void function failed to execute."; } TEST_F(DummyRPC, TestAsyncIntInt) { @@ -112,7 +112,7 @@ TEST_F(DummyRPC, TestAsyncIntInt) { QueueChannel C2(Q2, Q1); // Make an async call. - auto ResOrErr = callNBWithSeq(C1, 21); + auto ResOrErr = callAsyncWithSeq(C1, 21); EXPECT_TRUE(!!ResOrErr) << "Simple call over queue failed"; { @@ -143,7 +143,7 @@ TEST_F(DummyRPC, TestSerialization) { // Make a call to Proc1. std::vector v({42, 7}); - auto ResOrErr = callNBWithSeq( + auto ResOrErr = callAsyncWithSeq( C1, -101, 250, -10000, 10000, -1000000000, 1000000000, -10000000000, 10000000000, true, "foo", v); EXPECT_TRUE(!!ResOrErr) << "Big (serialization test) call over queue failed"; @@ -179,8 +179,8 @@ TEST_F(DummyRPC, TestSerialization) { } // Verify that the function returned ok. - auto Err = ResOrErr->first.get(); - EXPECT_FALSE(!!Err) << "Remote void function failed to execute."; + auto Val = ResOrErr->first.get(); + EXPECT_TRUE(Val) << "Remote void function failed to execute."; } // Test the synchronous call API. -- 2.7.4