[PATCH] D114183: [ThreadPool] Add template argument for future result type.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D114183#3143705 <https://reviews.llvm.org/D114183#3143705>, @fhahn wrote:

> Variants of
>
>   llvm/include/llvm/Support/ThreadPool.h:184:12: error: no viable conversion from returned value of type 'shared_future<void>' to function return type 'shared_future<llvm::SmallString<0>>'
>       return Future.share();
>              ^~~~~~~~~~~~~~

This seems to be a mismatch of `asyncImpl`'s future return type and the return type of the function passed to `asyncImpl`. That is, both should be the same template parameter, like the `template<typename RetTy> std::shared_future<RetTy> async(std::function<RetTy()> F);` suggested before. But this is requirements is only with the `asyncImpl` defintion.

More crucially, the line

  Tasks.push(std::move(PackagedTask));

wraps the PackagedTask into <https://en.cppreference.com/w/cpp/thread/packaged_task/operator()> a `std::function<void()>` which does not depend on the return type anymore. That is, we don't need `ThreadPool::Tasks` to be a template of the return type, i.e. it can store tasks of any return type (I think the actually result value is stored as a closure parameter of the `std::function<void()>` object, the `std::future` can be used to retrieve it), the ThreadPool itself does not need to store the future which was only returned by `asyncImpl` for the caller to remember.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114183/new/

https://reviews.llvm.org/D114183