From 1383754f90945eeb8409bcce024be01557f0ebfb Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Tue, 6 Oct 2020 10:27:22 -0400 Subject: [PATCH 1/2] ARROW-10196: [C++] Add Future::DeferNotOk --- cpp/src/arrow/util/future.h | 27 +++++++++++---------------- cpp/src/arrow/util/thread_pool.h | 4 +--- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h index 19fd632c065..44f66549a41 100644 --- a/cpp/src/arrow/util/future.h +++ b/cpp/src/arrow/util/future.h @@ -154,9 +154,8 @@ class FutureStorage : public FutureStorageBase { Status status() const { return result_.status(); } - template - void MarkFinished(U&& value) { - result_ = std::forward(value); + void MarkFinished(Result result) { + result_ = std::move(result); if (ARROW_PREDICT_TRUE(result_.ok())) { impl_->MarkFinished(); } else { @@ -250,7 +249,7 @@ class Future { // The default constructor creates an invalid Future. Use Future::Make() // for a valid Future. This constructor is mostly for the convenience // of being able to presize a vector of Futures. - Future() : impl_(NULLPTR) {} + Future() = default; // Consumer API @@ -358,6 +357,13 @@ class Future { return fut; } + static Future DeferNotOk(Result maybe_future) { + if (ARROW_PREDICT_FALSE(!maybe_future.ok())) { + return MakeFinished(std::move(maybe_future).status()); + } + return std::move(maybe_future).MoveValueUnsafe(); + } + protected: void CheckValid() const { #ifndef NDEBUG @@ -368,7 +374,7 @@ class Future { } std::shared_ptr> storage_; - FutureImpl* impl_; + FutureImpl* impl_ = NULLPTR; friend class FutureWaiter; }; @@ -419,15 +425,4 @@ inline std::vector WaitForAny(const std::vector*>& futures, return waiter->MoveFinishedFutures(); } -#define ARROW_ASSIGN_OR_RETURN_FUTURE_IMPL(result_name, lhs, T, rexpr) \ - auto result_name = (rexpr); \ - if (ARROW_PREDICT_FALSE(!(result_name).ok())) { \ - return Future::MakeFinished(std::move(result_name).status()); \ - } \ - lhs = std::move(result_name).MoveValueUnsafe(); - -#define ARROW_ASSIGN_OR_RETURN_FUTURE(lhs, T, rexpr) \ - ARROW_ASSIGN_OR_RETURN_FUTURE_IMPL( \ - ARROW_ASSIGN_OR_RAISE_NAME(_error_or_value, __COUNTER__), lhs, T, rexpr); - } // namespace arrow diff --git a/cpp/src/arrow/util/thread_pool.h b/cpp/src/arrow/util/thread_pool.h index fa6583e7371..45dc1ca7145 100644 --- a/cpp/src/arrow/util/thread_pool.h +++ b/cpp/src/arrow/util/thread_pool.h @@ -143,10 +143,8 @@ class ARROW_EXPORT Executor { typename RT = typename detail::ExecutorResultTraits, typename ValueType = typename RT::ValueType> Future SubmitAsFuture(Function&& func, Args&&... args) { - ARROW_ASSIGN_OR_RETURN_FUTURE( - auto future, ValueType, + return Future::DeferNotOk( Submit(std::forward(func), std::forward(args)...)); - return future; } // Return the level of parallelism (the number of tasks that may be executed From 1f54d6152c5880836856017c9033debd6ae6eb43 Mon Sep 17 00:00:00 2001 From: Benjamin Kietzman Date: Wed, 7 Oct 2020 10:29:44 -0400 Subject: [PATCH 2/2] add doccomment --- cpp/src/arrow/util/future.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cpp/src/arrow/util/future.h b/cpp/src/arrow/util/future.h index 44f66549a41..575f5cb3c41 100644 --- a/cpp/src/arrow/util/future.h +++ b/cpp/src/arrow/util/future.h @@ -310,6 +310,15 @@ class Future { return impl_->Wait(seconds); } + /// If a Result holds an error instead of a Future, construct a finished Future + /// holding that error. + static Future DeferNotOk(Result maybe_future) { + if (ARROW_PREDICT_FALSE(!maybe_future.ok())) { + return MakeFinished(std::move(maybe_future).status()); + } + return std::move(maybe_future).MoveValueUnsafe(); + } + // Producer API /// \brief Producer API: execute function and mark Future finished @@ -357,13 +366,6 @@ class Future { return fut; } - static Future DeferNotOk(Result maybe_future) { - if (ARROW_PREDICT_FALSE(!maybe_future.ok())) { - return MakeFinished(std::move(maybe_future).status()); - } - return std::move(maybe_future).MoveValueUnsafe(); - } - protected: void CheckValid() const { #ifndef NDEBUG