-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Make some special member functions of promise
and packaged_task
defaulted
#3315
Make some special member functions of promise
and packaged_task
defaulted
#3315
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Thanks @frederick-vs-ja
|
||
_Promise(_Promise&& _Other) : _State(_STD move(_Other._State)), _Future_retrieved(_Other._Future_retrieved) {} | ||
_Promise(_Promise&&) = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this does not make _Promise<T>
trivially moveable since _State_manager<T>
is never trivially moveable (and therefore same for promise<T>
and packaged_task<T>
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think that makes this ABI-safe. (This is very close to the dangerous condition that I encountered in tuple<>
's copy ctor, though.)
|
||
_Promise(_Promise&& _Other) : _State(_STD move(_Other._State)), _Future_retrieved(_Other._Future_retrieved) {} | ||
_Promise(_Promise&&) = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think that makes this ABI-safe. (This is very close to the dangerous condition that I encountered in tuple<>
's copy ctor, though.)
packaged_task() noexcept : _MyPromise(0) {} | ||
packaged_task() = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No change requested: Wow, a 0
-as-null-pointer was still lurking in the codebase! Nice catch!
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thanks for these careful improvements! 😸 ✅ 🎉 |
Also update references to working draft to WG21-N4928 in
<future>
.This is a follow-up change of #3284, and partially addresses #3278.