jbaldwin / libcoro

C++20 coroutine library
Apache License 2.0
566 stars 57 forks source link

task does not support rvalue return type #180

Closed a858438680 closed 11 months ago

a858438680 commented 11 months ago

I don't know if this is by design or a bug. The type coro::task does not support rvalue reference as the return type.

This minimal example is the following file:

#include "task.hpp"

auto rvalue() -> coro::task<int&&> {
    static int i = 42;
    co_return std::move(i);
}

This file cannot be compiled, because the coro::detail::promise<int&&>::held_type is std::optional<int&&> which is not constructible.

a858438680 commented 11 months ago

Fix #181 has been added to solve this problem

jbaldwin commented 11 months ago

Hey thanks for opening the issue and the PR! Seems recent changes problem have possibly broken this, it's definitely not intended. On your PR would you be willing to add a test with your use case so we can prove we don't break this again moving forward?

a858438680 commented 11 months ago

Of course, I'd like to. But I'm new to such contribution. Could you please tell me how and where shoud I add the test?

jbaldwin commented 11 months ago

I think heretest_task.cpp would be a great place for it, you could add a new TEST_CASE and you can put your r-value example in there and assert it returns the correct value.

jbaldwin commented 11 months ago

Tests can be run locally after building via ctest, looks like I should add a task to include test instructions in the readme!

a858438680 commented 11 months ago

I think heretest_task.cpp would be a great place for it, you could add a new TEST_CASE and you can put your r-value example in there and assert it returns the correct value.

Hello, I have added a test case to test my pull request and all the tests have been passed on my local computer.

Also, I add some more description in the pull request to explain in detail what is changed by the pull request.

a858438680 commented 11 months ago

Sorry, I don't quite understand the pull request machenism. In the PR page, I saw that there is a button "Update Branch", and I clicked it. Now it seems that the pr needs approve again. Do I need to withdraw the third commit? Or I just leave it there?