Skip to content

Remove low-level coroutine suspension notifications #1228

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.

Sign up for GitHub

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

Merged
kennykerr merged 3 commits into from
Nov 11, 2022
Merged

Remove low-level coroutine suspension notifications #1228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
29f4c3a
Remove suspension point hooks
sylveon Nov 11, 2022
ec06d8b
Remove clang workarounds
sylveon Nov 11, 2022
5c0dde0
Fix cancellation propagation
sylveon Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions strings/base_coroutine_foundation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,11 +577,6 @@ namespace winrt::impl

auto final_suspend() noexcept
{
if (winrt_suspend_handler)
{
winrt_suspend_handler(this);
}

return final_suspend_awaiter{ this };
}

Expand All @@ -606,14 +601,23 @@ namespace winrt::impl
}

template <typename Expression>
auto await_transform(Expression&& expression)
Expression&& await_transform(Expression&& expression)
{
if (Status() == AsyncStatus::Canceled)
{
throw winrt::hresult_canceled();
}

return notify_awaiter<Expression>{ static_cast<Expression&&>(expression), m_propagate_cancellation ? &m_cancellable : nullptr };
if constexpr (std::is_convertible_v<std::remove_reference_t<decltype(expression)>&, enable_await_cancellation&>)
{
if (m_propagate_cancellation)
{
static_cast<enable_await_cancellation&>(expression).set_cancellable_promise(&m_cancellable);
expression.enable_cancellation(&m_cancellable);
}
}

return std::forward<Expression>(expression);
}

cancellation_token<Derived> await_transform(get_cancellation_token_t) noexcept
Expand Down
101 changes: 0 additions & 101 deletions strings/base_coroutine_threadpool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,26 +133,6 @@ namespace winrt::impl
resume_apartment_sync(context.m_context, handle, failure);
}
}

template <typename T>
class awaiter_finder
{
template <typename> static constexpr bool find_awaitable_member(...) { return false; }
template <typename> static constexpr bool find_co_await_member(...) { return false; }
template <typename> static constexpr bool find_co_await_free(...) { return false; }

#ifdef WINRT_IMPL_COROUTINES
template <typename U, typename = decltype(std::declval<U>().await_ready())> static constexpr bool find_awaitable_member(int) { return true; }
template <typename U, typename = decltype(std::declval<U>().operator co_await())> static constexpr bool find_co_await_member(int) { return true; }
template <typename U, typename = decltype(operator co_await(std::declval<U>()))> static constexpr bool find_co_await_free(int) { return true; }
#endif

public:

static constexpr bool has_awaitable_member = find_awaitable_member<T>(0);
static constexpr bool has_co_await_member = find_co_await_member<T&&>(0);
static constexpr bool has_co_await_free = find_co_await_free<T&&>(0);
};
}

WINRT_EXPORT namespace winrt
Expand Down Expand Up @@ -226,76 +206,6 @@ WINRT_EXPORT namespace winrt
};
}

namespace winrt::impl
{
template <typename T>
decltype(auto) get_awaiter(T&& value) noexcept
{
#ifdef WINRT_IMPL_COROUTINES
if constexpr (awaiter_finder<T>::has_co_await_member)
{
static_assert(!awaiter_finder<T>::has_co_await_free, "Ambiguous operator co_await (as both member and free function).");
return static_cast<T&&>(value).operator co_await();
}
else if constexpr (awaiter_finder<T>::has_co_await_free)
{
return operator co_await(static_cast<T&&>(value));
}
else
{
static_assert(awaiter_finder<T>::has_awaitable_member, "Not an awaitable type");
return static_cast<T&&>(value);
}
#else
return static_cast<T&&>(value);
#endif
}

template <typename T>
struct notify_awaiter
{
decltype(get_awaiter(std::declval<T&&>())) awaitable;

notify_awaiter(T&& awaitable_arg, [[maybe_unused]] cancellable_promise* promise = nullptr) : awaitable(get_awaiter(static_cast<T&&>(awaitable_arg)))
{
if constexpr (std::is_convertible_v<std::remove_reference_t<decltype(awaitable)>&, enable_await_cancellation&>)
{
if (promise)
{
static_cast<enable_await_cancellation&>(awaitable).set_cancellable_promise(promise);
awaitable.enable_cancellation(promise);
}
}
}

bool await_ready()
{
if (winrt_suspend_handler)
{
winrt_suspend_handler(this);
}

return awaitable.await_ready();
}

template <typename U>
auto await_suspend(coroutine_handle<U> handle)
{
return awaitable.await_suspend(handle);
}

decltype(auto) await_resume()
{
if (winrt_resume_handler)
{
winrt_resume_handler(this);
}

return awaitable.await_resume();
}
};
}

WINRT_EXPORT namespace winrt
{
[[nodiscard]] inline auto resume_background() noexcept
Expand Down Expand Up @@ -728,24 +638,13 @@ namespace std::experimental

suspend_never final_suspend() const noexcept
{
if (winrt_suspend_handler)
{
winrt_suspend_handler(this);
}

return{};
}

void unhandled_exception() const noexcept
{
winrt::terminate();
}

template <typename Expression>
auto await_transform(Expression&& expression)
{
return winrt::impl::notify_awaiter<Expression>{ static_cast<Expression&&>(expression) };
}
};
};
}
2 changes: 0 additions & 2 deletions strings/base_extern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
__declspec(selectany) int32_t(__stdcall* winrt_to_hresult_handler)(void* address) noexcept {};
__declspec(selectany) winrt::hstring(__stdcall* winrt_to_message_handler)(void* address) {};
__declspec(selectany) void(__stdcall* winrt_throw_hresult_handler)(uint32_t lineNumber, char const* fileName, char const* functionName, void* returnAddress, winrt::hresult const result) noexcept {};
__declspec(selectany) void(__stdcall* winrt_suspend_handler)(void const* token) noexcept {};
__declspec(selectany) void(__stdcall* winrt_resume_handler)(void const* token) noexcept {};
__declspec(selectany) int32_t(__stdcall* winrt_activation_handler)(void* classId, winrt::guid const& iid, void** factory) noexcept {};

extern "C"
Expand Down
8 changes: 4 additions & 4 deletions test/old_tests/UnitTests/async.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace

IAsyncAction NoSuspend_IAsyncAction()
{
co_await resume_after(0s);
co_await 0s;
Copy link
Collaborator

@kennykerr kennykerr Nov 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change have to do with ripping out the low-level hooks?

Copy link
Contributor Author

@sylveon sylveon Nov 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workaround was added because of the low level hooks preventing Clang from building this code.

So removing resume_after helps make sure that we have coverage for this functionality on Clang.


auto cancel = co_await get_cancellation_token();

Expand All @@ -34,7 +34,7 @@ namespace

IAsyncActionWithProgress<double> NoSuspend_IAsyncActionWithProgress()
{
co_await resume_after(0s);
co_await 0s;

auto cancel = co_await get_cancellation_token();

Expand All @@ -46,7 +46,7 @@ namespace

IAsyncOperation<uint32_t> NoSuspend_IAsyncOperation()
{
co_await resume_after(0s);
co_await 0s;

auto cancel = co_await get_cancellation_token();

Expand All @@ -60,7 +60,7 @@ namespace

IAsyncOperationWithProgress<uint64_t, uint64_t> NoSuspend_IAsyncOperationWithProgress()
{
co_await resume_after(0s);
co_await 0s;

auto cancel = co_await get_cancellation_token();

Expand Down
71 changes: 0 additions & 71 deletions test/test/async_ref_result.cpp
View file
Open in desktop

This file was deleted.

8 changes: 4 additions & 4 deletions test/test/async_throw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,26 @@ namespace

IAsyncAction Action()
{
co_await resume_after(10ms);
co_await 10ms;
throw hresult_invalid_argument(L"Async");
}

IAsyncActionWithProgress<int> ActionWithProgress()
{
co_await resume_after(10ms);
co_await 10ms;
throw hresult_invalid_argument(L"Async");
}

IAsyncOperation<int> Operation()
{
co_await resume_after(10ms);
co_await 10ms;
throw hresult_invalid_argument(L"Async");
co_return 1;
}

IAsyncOperationWithProgress<int, int> OperationWithProgress()
{
co_await resume_after(10ms);
co_await 10ms;
throw hresult_invalid_argument(L"Async");
co_return 1;
}
Expand Down
4 changes: 2 additions & 2 deletions test/test/disconnected.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace

IAsyncActionWithProgress<int> ActionProgress()
{
co_await resume_after(500ms);
co_await 500ms;
auto progress = co_await get_progress_token();
progress(123);
co_return;
Expand All @@ -29,7 +29,7 @@ namespace

IAsyncOperationWithProgress<int, int> OperationProgress()
{
co_await resume_after(500ms);
co_await 500ms;
auto progress = co_await get_progress_token();
progress(123);
co_return 123;
Expand Down
Loading