Continuing the discussion in #1242

@sylveon It seems your change in #1228 did break cancel propagation, which shows up in the async_propagate_cancel test inside Check([] { return ActionAction(10); }); sporadically, because it only fails if async.Cancel() is called after the first call to co_await ActionAction(wait, depth - 1); has already gone through await_transform (base_coroutine_foundation.h):

Line 611-618 is responsible for propagating the cancellation to the child coroutine. The condition and cast here is:

if constexpr (std::is_convertible_v<std::remove_reference_t<decltype(expression)>&, enable_await_cancellation&>) {
    //[...]
    static_cast<enable_await_cancellation&>(expression).set_cancellable_promise(&m_cancellable);

But before #1228, the check inside notify_awaiter was:

if constexpr (std::is_convertible_v<std::remove_reference_t<decltype(awaitable)>&, enable_await_cancellation&>) {
    // [...]
    static_cast<enable_await_cancellation&>(awaitable).set_cancellable_promise(promise /* same as &m_cancellable */);

awaitable was declared as:

decltype(get_awaiter(std::declval<T&&>())) awaitable
    /* effectively: */ = get_awaiter(static_cast<T&&>(expression));

get_awaiter makes use of operator co_await to convert the awaitables. The following applies:

Because this conversion does not happen anymore after #1228, the cancellation is not propagated properly, therefore the child coroutines don't get cancelled, and depending on how late async.Canecl() is called, the completion handler may not be invoked. My change in alvinhochun@c34c31a triggers this reliably.

I can confirm that the failure is gone after reverting #1228 and these two changes.

CC @kennykerr @oldnewthing