Skip to content

Reduce stack consumption if unable to switch to apartment_context #1276

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

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

oldnewthing
Copy link
Member

The old code would invoke the handle recursively. The new code returns false from await_suspend, allowing the calling coroutine to resume with no stack consumption. Use [[nodiscard]] to ensure I updated all callers, and to avoid future errors with new callers.

Also remove #ifdefs to work around code generation issues in MSVC versions less than 16.11. If you are on a version that old, you should stick with an older version of C++/WinRT.

The old code would invoke the handle recursively. The new
code returns `false` from `await_suspend`, allowing the calling
coroutine to resume with no stack consumption. Use `[[nodiscard]]`
to ensure I updated all callers, and to avoid future errors with
new callers.

Also remove `#ifdef`s to work around code generation issues in
MSVC versions less than 16.11. If you are on a version that old,
you should stick with an older version of C++/WinRT.
@@ -77,7 +77,7 @@ namespace winrt::impl
return 0;
};

inline void resume_apartment_sync(com_ptr<IContextCallback> const& context, coroutine_handle<> handle, int32_t* failure)
[[nodiscard]] inline bool resume_apartment_sync(com_ptr<IContextCallback> const& context, coroutine_handle<> handle, int32_t* failure)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do love nodiscard. 👍

@kennykerr kennykerr merged commit 419c33a into microsoft:master Feb 16, 2023
@oldnewthing oldnewthing deleted the remove_workaround branch February 17, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants