[GitHub] [arrow] bkietz commented on a change in pull request #8591: ARROW-10484: [C++] Make Future<> more generic

[GitBox <gi...@apache.org>]

bkietz commented on a change in pull request #8591:
URL: https://github.com/apache/arrow/pull/8591#discussion_r519864378



##########
File path: cpp/src/arrow/util/future.h
##########
@@ -265,28 +188,24 @@ class Future {
   }
 
   /// \brief Wait for the Future to complete and return its Result
-  ///
-  /// This function is not available on Future<void> and Future<Status>.
-  /// For these specializations, please call status() instead.
-  template <typename U = T>
-  const Result<T>& result(EnableResult<U>* = NULLPTR) const& {
-    CheckValid();
+  const Result<ValueType>& result() const& {
     Wait();
-    return storage_->result_;
+    return *GetResult();
   }
-
-  template <typename U = T>
-  Result<T> result(EnableResult<U>* = NULLPTR) && {
-    CheckValid();
+  Result<ValueType>&& result() && {
     Wait();
-    return std::move(storage_->result_);
+    return std::move(*GetResult());
   }
 
   /// \brief Wait for the Future to complete and return its Status
-  Status status() const {
-    CheckValid();
-    Wait();
-    return storage_->status();
+  const Status& status() const { return result().status(); }
+
+  /// \brief Future<T> is convertible to Future<>, which views only the
+  /// Status of the original. Marking this Future Finished is not supported.

Review comment:
       That's a good point; I'll clarify the ambiguity




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org