westonpace commented on pull request #10205: URL: https://github.com/apache/arrow/pull/10205#issuecomment-839428127
> Overall I'm a bit surprised by the amount of complication that seems necessary. Is there a way to straighten this up. Most of your comments should be addressable. However, I'm not certain if this comment is referring to the overall idea of splitting `Future` into three classes (well two with one specialization) `FutureBase`, `Future<>`, and `Future<T>`. The need there stems from the fact that I cannot (to my knowledge) apply SFINAE to a method based on a class template parameter (`T`). I think I could take the "dummy parameter" approach (e.g. something like `void f(T&& x, typename std::enable_if<!std::is_reference<T_>::value, std::nullptr_t>::type = nullptr)`) but I personally find that to be less readable and, since the inheritance is non-virtual, I don't think it would be more performant. Another approach could be to allow `Result<Status>` (perhaps with a special factory method to allow the static assert to remain) but I seem to recall that being frowned upon. If that is on the table then I'd be happy to investigate that option as well. -- 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: us...@infra.apache.org
