On Thu, 14 May 2020 at 14:59, Avi Kivity <avi@scylladb.com> wrote:

On 5/14/20 2:46 PM, Ville Voutilainen wrote:

On Thu, 14 May 2020 at 14:34, Avi Kivity <avi@scylladb.com> wrote:

With a lambda coroutine, we have a problem. This is because lambdas are
captured by reference:

Says what?

Says the quote below, from cppreference.

Your wording was confusing; I thought you meant a lambda as a
coroutine parameter.
For cases where we are in a non-static member function, the *this is
indeed not copied.

Ah, the lambda _is_ a coroutine parameter in a sense, if you follow the
regular translation of a lambda to a struct with operator().


And note this is not specific to lambdas, it happens for any member
coroutine called on a temporary. A lambda is just the most common way of
generating such calls.

Right. Calling a member function coroutine of a temporary seems like
madness to me.
It's quite like calling a member function that queues async work that
is later completed in
some other member of the object.

It is super common in async frameworks.

For example the predecessor of coroutines is future::then, which accepts a callable that is called when the future becomes ready. So if I write

   future<int> get_int();

   int foo;

   extern int bar;

   get_int().then([foo] (int val) -> future<> {

        int another = co_await get_int();

        bar = foo + val + another;

   });

This breaks with the current standard, because the lambda object can still be on the stack when operator()(int) is invoked.

I'm not sure I understand where the temporary really is in the
not-so-lame testcase in the bug report. To me,
it seems like everything is stored and lifetimes don't end
prematurely, but I don't understand how initial_suspend
really works. :)

Copying it here for reference:

#include <coroutine>
#include <functional>
#include <optional>
#include <cassert>

template <typename T>
class lazy {
    std::function<T ()> _compute;
    lazy(std::function<T ()> compute) : _compute(std::move(compute)) {}
public:
    T get() { return _compute(); }
    static lazy make(std::function<T ()> compute) {
        return lazy(std::move(compute));
    }
};

namespace std {

template <typename T, typename... Args>
struct coroutine_traits<lazy<T>, Args...> {
    struct promise_type {
        std::optional<T> value;
        suspend_always initial_suspend() const { return {}; }
        suspend_always final_suspend() const { return {}; }
        void return_value(T val) {
            value = std::move(val);
        }
        lazy<T> get_return_object() {
            return lazy<T>::make([this] () -> T {
                auto handle = coroutine_handle<promise_type>::from_promise(*this);
                handle.resume();
                auto ret = std::move(*value);
                handle.destroy();
                return ret;
            });
        }
        void unhandled_exception() {
            std::terminate();
        }
    };
};

}

struct fake_lambda_state {
    int i = 5;
};

lazy<int> fake_lambda(fake_lambda_state s) {
    co_return s.i;
}

lazy<int> get_fake_lambda() {
    // fake_lambda_state() is captured in the promise
    return fake_lambda(fake_lambda_state());
}

lazy<int> get_real_lambda() {
    // the state (i) is not captured
    return [i = 6] () -> lazy<int> {
        co_return i;
    }();
}

int main(int ac, char** av) {
    auto l1 = get_fake_lambda();
    assert(l1.get() == 5);
    auto l2 = get_real_lambda();
    assert(l2.get() == 6);
    return 0;
}

The following happens in the execution of "auto l2 = get_real_lambda()". Please forgive any excessive detail, I figure it is better to provide too much than too little:

1. A lambda object is constructed on the stack, with i initialized to 6.

2. The lambda's operator()() is called.

3. A coroutine frame is allocated with space for a pointer to the lambda.

4. The pointer to the lambda is copied to the coroutine frame.

5. initial_suspend() is called, and because it returns suspend_always, get_return_object() is called.

6. get_return_object returns a lazy<int> initialized with some std::function, that captures the coroutine handle (really the frame)

7. the coroutine returns the its caller, get_real_lambda()

8. get_real_lambda destroys the lambda (creating a dangling reference)

9. the lazy<int> is assigned to l2.

The next line, l2.get():

10. lazy<int>::get() is called, calling the function we created in step 6

11. The coroutine handle is used to resume the coroutine just after the point where we called initial_suspend (at the "{")

12. We evaluate "i" for "co_return i". This is really lambda_ptr->i, where lambda_ptr was captured in step 4.

13. the access to i is to a destroyed stack frame, which is undefined behavior, so the compiler is at liberty to destroy all life on earth.

Note that fake_lambda conceptually does exactly the same things, but because it is captured by value, everything works.