Fix coroutine continuation cleanup

[jb-gcx]

C++ coroutines clean up their local variables after invoking return_value, return_void or unhandled_exception. This means that executing a continuation in one of these methods basically inverts the cleanup order. Example:

#include <Future.hpp>
#include <iostream>
#include <thread>

struct LogOnDestruct {
    std::string message;
    ~LogOnDestruct() { std::cout << message << "\n"; }
};

djinni::Future<void> foo() {
    LogOnDestruct log{"~foo"};

    // The issue only occurs when we suspend
    // So make sure to suspend on another future
    djinni::Promise<void> promise;
    std::thread{[&promise] {
        std::this_thread::sleep_for(std::chrono::milliseconds(10));
        promise.setValue();
    }}.detach();
    co_return co_await promise.getFuture();
}

djinni::Future<void> bar() {
    LogOnDestruct log{"~bar"};
    co_await foo();
}

int main() {
    bar().get();
    return 0;
}

Before this PR, the above code produced:

~bar
~foo

Which is inverted (caller gets cleaned up before callee). This can even lead to crashes etc, if a reference to a local variable from bar is used during cleanup in foo - because that local variable in bar was already cleaned up!

After this PR the code produces:

~foo
~bar

As it should.

Merge considerations

• I use a requires, which is a C++20 feature. I decided that this is acceptable, since coroutines themselves are also a C++20 feature. If this is not okay, let me know and I'll change it to a more compatible SFINAE approach.
• I'd like to add a test for this, but I'm not sure how. There is no dedicated C++ test suite and it would take me a while to create one as I have zero bazel experience. Should I just add an XCTest, or what's the preferred way?

[li-feng-sc]

Thank you for this contribution. Regarding your questions:

• Although Snap has been using C++20 for a while, we haven't get to refresh Djinni to use C++20. There are lots of other places that needs to be cleaned up for C++20 and I would prefer to do them together. So let's try to avoid requires for now if this is not too much trouble.
• Yeah the tests are focused on testing interoperability so no pure C++ tests. A XCTest is fine.

[jb-gcx]

Thanks for the feedback. I've replaced requires with enable_if and added an XCTest. Please take another look.

[li-feng-sc]

Let's use the alias detail::CoroutineHandle for now

[jb-gcx]

Done 👍

[li-feng-sc]

I find I have to change the feature detection to the following to make it work in c++17 + -fcoroutine-ts

#if defined(__cpp_coroutines) && __has_include(<experimental/coroutine>)
    #include <experimental/coroutine>
    namespace djinni::detail {
        template <typename Promise = void> using CoroutineHandle = std::experimental::coroutine_handle<Promise>;
        using SuspendNever = std::experimental::suspend_never;
    }
    #define DJINNI_FUTURE_HAS_COROUTINE_SUPPORT 1
#elif defined(__cpp_impl_coroutine) && __has_include(<coroutine>)
    #include <coroutine>
    namespace djinni::detail {
        template <typename Promise = void> using CoroutineHandle = std::coroutine_handle<Promise>;
        using SuspendNever = std::suspend_never;
    }
    #define DJINNI_FUTURE_HAS_COROUTINE_SUPPORT 1
#endif

[jb-gcx]

Looks like with these settings the <coroutine> header is available but not functional. Your suggestion fixes that nicely, but I found another issue: with clang 16 and -std=c++20 -stdlib=libc++, it chooses the experimental implementation even though the proper one is available.

I've done some experiments:

• __cpp_coroutines is available for both C++20 and C++17 + -fcoroutines-ts until clang 17 when the macro and -fcoroutines-ts were completely removed
• __cpp_impl_coroutine and __cpp_lib_coroutine are only available in proper C++20 mode when we have <coroutine>

I've just pushed another suggestion based on these findings. Here's what I used to test it with a few compilers/settings: https://godbolt.org/z/rj737E5eq

Caveat: I've tested only with clang. For iOS and Android that seems to be the relevant compiler.

[li-feng-sc]

thanks! this works really well.