Skip to content

Commit

Permalink
Some minor fixes (#537)
Browse files Browse the repository at this point in the history
* Minor bug fixes around operations that require mutability.
* Replacing an incorrect identity function with std::identity{}
* Removed unnecessary moves.
  • Loading branch information
sean-parent authored Feb 23, 2024
1 parent c59cfac commit bc092d5
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 44 deletions.
15 changes: 4 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ Release changelogs are listed in [CHANGES.md](CHANGES.md).

## Requirements

- A standards-compliant C++14, C++17, or C++20 compiler
- **Use with a compiler in C++14-compliant mode** requires Boost.Optional and Boost.Variant >= 1.74.0
- A standards-compliant C++17, C++20, or C++23 compiler
- **Building** the library requires CMake 3.23 or later
- **Testing or developing** the library requires Boost.Test >= 1.74.0

Expand All @@ -41,16 +40,10 @@ for an introduction to this tool.
1. Create a build directory outside this library's source tree. In this guide, we'll use a sibling
directory called `BUILD`.

2. If you are using the library in C++14-compliant mode or need to run the test suite, be sure you
have the necessary parts of Boost >= 1.74.0 installed. Linux distributions usually make a
suitable version available through standard package managers. On macOS or Linux, you can easilly
install Boost using [Homebrew](https://brew.sh/). To install Boost on Windows, you can use
Boost's [binary installers](https://sourceforge.net/projects/boost/files/boost-binaries/).

3. Install a version of CMake >= 3.23. If you are on Debian or Ubuntu Linux you may need to use
1. Install a version of CMake >= 3.23. If you are on Debian or Ubuntu Linux you may need to use
`snap` to find one that's new enough.

4. If you are using MSVC, you may need to set environment variables appropriately for your target
1. If you are using MSVC, you may need to set environment variables appropriately for your target
architecture by invoking `VCVARSALL.BAT` with an appropriate option.

### Configure
Expand All @@ -65,7 +58,7 @@ cmake -S . -B ../BUILD -DCMAKE_BUILD_TYPE=# SEE BELOW
but there are other options you may need to append in order to be successful. Among them:

* `-DCMAKE_BUILD_TYPE=`[`Release`|`Debug`] to build the given configuration (required unless you're using visual studio or another multi-config generator).
* `-DCMAKE_CXX_STANDARD=`[`14`|`17`|`20`|`23`] to build with compliance to the given C++ standard.
* `-DCMAKE_CXX_STANDARD=`[`17`|`20`|`23`] to build with compliance to the given C++ standard.
* `-DBUILD_TESTING=OFF` if you only intend to build, but not test, this library.
* `-DBoost_USE_STATIC_LIBS=TRUE` if you will be testing on Windows.

Expand Down
6 changes: 4 additions & 2 deletions stlab/concurrency/await.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,10 @@ T await(future<T> x) {
std::condition_variable condition;
bool flag{false};

future<T> result;

auto hold = std::move(x).recover(immediate_executor, [&](future<T>&& r) {
x = std::move(r);
result = std::move(r);
{
std::unique_lock<std::mutex> lock{m};
flag = true;
Expand Down Expand Up @@ -136,7 +138,7 @@ T await(future<T> x) {

#endif

return detail::_get_ready_future<T>{}(std::move(x));
return detail::_get_ready_future<T>{}(std::move(result));
}

namespace detail {
Expand Down
2 changes: 1 addition & 1 deletion stlab/concurrency/default_executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ constexpr auto platform_priority(executor_priority p) {
case executor_priority::low:
return DISPATCH_QUEUE_PRIORITY_LOW;
default:
assert(!"Unknown value!");
assert(false && "Unknown value!");
}
return DISPATCH_QUEUE_PRIORITY_DEFAULT;
}
Expand Down
52 changes: 23 additions & 29 deletions stlab/concurrency/future.hpp
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <atomic>
#include <cassert>
#include <exception>
#include <functional>
#include <initializer_list>
#include <memory>
#include <mutex>
Expand Down Expand Up @@ -567,34 +568,28 @@ class packaged_task {
packaged_task() = default;

~packaged_task() {
auto p = _p.lock();
if (p) p->remove_promise();
if (auto p = _p.lock()) p->remove_promise();
}

packaged_task(const packaged_task& x) : _p(x._p) {
auto p = _p.lock();
if (p) p->add_promise();
if (auto p = _p.lock()) p->add_promise();
}

packaged_task(packaged_task&&) noexcept = default;
packaged_task& operator=(const packaged_task& x) {
auto tmp = x;
*this = std::move(tmp);
return *this;
}

packaged_task& operator=(const packaged_task& x) { return *this = packaged_task{x}; }

packaged_task& operator=(packaged_task&& x) noexcept = default;

template <typename... A>
void operator()(A&&... args) const noexcept {
auto p = _p.lock();
if (p) (*p)(std::forward<A>(args)...);
if (auto p = _p.lock()) (*p)(std::forward<A>(args)...);
}

bool canceled() const { return _p.expired(); }

void set_exception(std::exception_ptr error) const {
auto p = _p.lock();
if (p) p->set_error(std::move(error));
if (auto p = _p.lock()) p->set_error(std::move(error));
}
};

Expand Down Expand Up @@ -636,7 +631,7 @@ class STLAB_NODISCARD() future<T, enable_if_copyable<T>> {

template <typename F>
auto then(F&& f) const& {
return recover([_f = std::forward<F>(f)](future<result_type>&& p) {
return recover([_f = std::forward<F>(f)](future<result_type>&& p) mutable {
return std::move(_f)(*std::move(p).get_try());
});
}
Expand All @@ -649,7 +644,7 @@ class STLAB_NODISCARD() future<T, enable_if_copyable<T>> {
template <typename E, typename F>
auto then(E&& executor, F&& f) const& {
return recover(std::forward<E>(executor),
[_f = std::forward<F>(f)](future<result_type>&& p) {
[_f = std::forward<F>(f)](future<result_type>&& p) mutable {
return std::move(_f)(*std::move(p).get_try());
});
}
Expand Down Expand Up @@ -781,7 +776,7 @@ class STLAB_NODISCARD() future<void, void> {

template <typename F>
auto then(F&& f) const& {
return recover([_f = std::forward<F>(f)](future<result_type>&& p) {
return recover([_f = std::forward<F>(f)](future<result_type>&& p) mutable {
std::move(p).get_try();
return std::move(_f)();
});
Expand All @@ -795,7 +790,7 @@ class STLAB_NODISCARD() future<void, void> {
template <typename E, typename F>
auto then(E&& executor, F&& f) const& {
return recover(std::forward<E>(executor),
[_f = std::forward<F>(f)](future<result_type>&& p) {
[_f = std::forward<F>(f)](future<result_type>&& p) mutable {
(void)std::move(p).get_try();
return std::move(_f)();
});
Expand Down Expand Up @@ -1244,7 +1239,7 @@ struct make_when_any {
using result_t = detail::result_t<F, T, size_t>;

auto shared = std::make_shared<detail::when_any_shared<sizeof...(Ts) + 1, T>>();
auto p = package<result_t()>(executor, [_f = std::move(f), _p = shared] {
auto p = package<result_t()>(executor, [_f = std::move(f), _p = shared]() mutable {
return detail::apply_when_any_arg(_f, _p);
});
shared->_f = std::move(p.first);
Expand All @@ -1264,7 +1259,7 @@ struct make_when_any<void> {
using result_t = detail::result_t<F, size_t>;

auto shared = std::make_shared<detail::when_any_shared<sizeof...(Ts), void>>();
auto p = package<result_t()>(executor, [_f = std::forward<F>(f), _p = shared] {
auto p = package<result_t()>(executor, [_f = std::forward<F>(f), _p = shared]() mutable {
return detail::apply_when_any_arg(_f, _p);
});
shared->_f = std::move(p.first);
Expand Down Expand Up @@ -1566,7 +1561,7 @@ struct _reduce_coroutine : std::enable_shared_from_this<_reduce_coroutine<P, R>>
_this->_promise.set_exception(e);
return;
}
_this->stage_0(std::move(a));
_this->stage_0(std::forward<decltype(a)>(a));
});
}
void stage_0(future<future<R>>&& r) {
Expand Down Expand Up @@ -1672,12 +1667,13 @@ auto async(E executor, F&& f, Args&&... args)
using result_type = detail::result_t<std::decay_t<F>, std::decay_t<Args>...>;

auto p = package<result_type()>(
executor, std::bind<result_type>(
[_f = std::forward<F>(f)](
unwrap_reference_t<std::decay_t<Args>>&... args) mutable -> result_type {
return _f(move_if<!is_reference_wrapper_v<std::decay_t<Args>>>(args)...);
},
std::forward<Args>(args)...));
executor,
std::bind<result_type>(
[_f = std::forward<F>(f)](
unwrap_reference_t<std::decay_t<Args>>&... args) mutable -> result_type {
return std::move(_f)(move_if<!is_reference_wrapper_v<std::decay_t<Args>>>(args)...);
},
std::forward<Args>(args)...));

executor(std::move(p.first));

Expand Down Expand Up @@ -1813,9 +1809,7 @@ struct std::coroutine_traits<stlab::future<T>, Args...> {
std::pair<stlab::packaged_task<T>, stlab::future<T>> _promise;

promise_type() {
_promise = stlab::package<T(T)>(stlab::immediate_executor, [](auto&& x) -> decltype(x) {
return std::forward<decltype(x)>(x);
});
_promise = stlab::package<T(T)>(stlab::immediate_executor, std::identity{});
}

stlab::future<T> get_return_object() { return std::move(_promise.second); }
Expand Down
2 changes: 1 addition & 1 deletion stlab/concurrency/system_timer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class system_timer {
[[deprecated("Use chrono::duration as parameter instead")]] void operator()(
std::chrono::steady_clock::time_point when, F&& f) {
using namespace std::chrono;
operator()(when - steady_clock::now(), std::move(f));
operator()(when - steady_clock::now(), std::forward<decltype(f)>(f));
}

template <typename F, typename Rep, typename Per = std::ratio<1>>
Expand Down

0 comments on commit bc092d5

Please sign in to comment.