Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The modifications remove unnecessary uses of std::move() for improve… #421

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ArielSilver
Copy link

@ArielSilver ArielSilver commented Jul 2, 2023

This pull request addresses two modifications in the promise_impl class within the experimental/impl/promise.hpp file:

that there is indeed an unnecessary use of std::move() in the apply_impl function. Specifically, the line f(std::get(std::move(result_type))...); could be modified to remove the unnecessary std::move().

Here's the corrected version of the apply_impl function:
template<typename Func, std::size_t... Idx>
void apply_impl(Func f, boost::asio::detail::index_sequence<Idx...>)
{
auto& result_type = reinterpret_cast<promise_impl::result_type>(&result);
f(std::get(result_type)...); // Remove std::move()
}

and In the operator() function of the promise_handler class, the modification involves removing the unnecessary use of std::move() when storing the result values in the promise object. The line new (&impl_->result) result_type(ts...); initializes the result object by directly forwarding the ts arguments without using std::move().

This modification ensures that the result values are efficiently stored in the promise object without unnecessary moves.
Certainly! Here's an explanation of the modifications made to the code:

**void operator()(std::remove_reference_t... ts)
{
assert(impl_);

using result_type = typename promise_impl<
void(Ts...), allocator_type, executor_type>::result_type ;

new (&impl_->result) result_type(ts...); // Remove std::move()

impl_->done = true;

if (impl_->completion)
impl_->complete_with_result();
}**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant