-
Notifications
You must be signed in to change notification settings - Fork 2
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
XX025: && parameters in coroutines - very likely result in UB #13
Comments
You can kind of work around it with something like this, which move-constructs copies of r-value reference parameters and passes l-value references through. template<typename... Args>
generator<int> f(Args&&... args) {
return [](Args... args) -> generator<int> {
g(std::forward<Args>(args)...);
co_return;
}(std::forward<Args>(args)...);
} |
Cool workaround! Though, I think this is something that evolution group might have want to have some input on. |
I do worry that if we come up with a rule like "rvalue references are always move-copied" then we'll penalise use cases like std::expected or std::optional for which there would not be undefined behavior for rvalue references. What if we let the promise object or coroutine_traits decide how parameters are captured? Eg by defining some nested typedef in Something like this: template<typename T, typename... ARGS>
class coroutine_traits<generator<T>, ARGS...>
{
using arg_capture = void(std::remove_rvalue_reference_t<ARGS>...);
}; This could possibly be a solution to allowing capture of 'this' by value too? If you can determine that call is to a member function from coroutine_traits. |
Possibly some constexpr member in the promise_type? That way we don't have to do the traits if we control the return type |
So were you thinking maybe something like this: struct generator_promise<T>
{
generator<T> get_return_object();
suspend_always initial_suspend();
suspend_always final_suspend();
void unhandled_exception();
// Should this be a non-static member function?
template<typename ARG>
static constexpr ARG parameter_transform(ARG&& arg) { return std::forward<ARG>(arg); }
}; Then within the coroutine a reference to the parameter is replaced with a reference to How would such a facility affect the ability for the compiler to elide copies if the parameter is not referenced after the first suspend-point? If provided, would the compiler still be required to call parameter_transform() even if not referenced after first suspend-point? Would the parameter_transform() function be allowed to return a value of a different type than the parameter? This could lead to particularly surprising (and difficult to understand) code. eg. generator<int> f(int a, int b)
{
static_assert(std::is_same_v<decltype(a), std::string>);
auto ab = a + b; // <--- string concatenation here!
co_return;
} I can, however, see potential in the ability to, say, translate For non-static member-function coroutines, would |
Good thinking. I had parameter_transform in early design for coroutine customization points that I later dropped to trying to make the design leaner and simpler. This is something that can be added in the future in a non-breaking fashion. I'd like to explore simpler alternatives for C++20. |
So something ugly like Or do we just recommend that generator coroutine functions (and other coroutines that are suspended before executing the body) take all parameters by value rather than trying to use perfect-forwarding? eg. template<typename... ARGS>
generator<int> f(ARGS... args)
{
g(std::move(args)...); // or possibly some other std::forward-like thing that unwraps std::reference_wrapper if required.
co_return;
} |
Another idea that came up during a discussion on #coroutines slack channel was to just prevent r-value references. template<typename T, typename... ARGS>
struct coroutine_traits<generator<T>, ARGS...>
{
static_assert(
!std::disjunction<std::is_rvalue_reference_t<ARGS>...>::value,
"Passing parameters by rvalue reference is not allowed for generator<T> coroutines");
using promise_type = generator_promise<T>;
}; Possibly a bit of a sledgehammer, though. |
This coroutine follows a perfect forwarding pattern, but, results in UB
Should we change the rules for how && parameters are captured in coroutines?
The text was updated successfully, but these errors were encountered: