-
Notifications
You must be signed in to change notification settings - Fork 804
P3815R1 Add scope_association concept to P3149 #8531
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
base: main
Are you sure you want to change the base?
Conversation
Fixes NB FI-392, CA-393 (C++26 CD).
source/exec.tex
Outdated
| using assoc_t = associate_data_t::assoc-t; | ||
| using sender_ref_t = associate_data_t::sender-ref; |
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.
| using assoc_t = associate_data_t::assoc-t; | |
| using sender_ref_t = associate_data_t::sender-ref; | |
| using assoc_t = associate_data_t::@\exposid{assoc-t}@; | |
| using sender_ref_t = associate_data_t::@\exposid{sender-ref}@; |
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.
Fixed.
source/exec.tex
Outdated
| explicit op_state(pair<assoc_t, sender_ref_t> parts, Rcvr& r) | ||
| : assoc(std::move(parts.first)) { | ||
| if (assoc) | ||
| ::new (voidify(op)) op_t(connect(std::move(*parts.second), std::move(r))); |
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.
| ::new (voidify(op)) op_t(connect(std::move(*parts.second), std::move(r))); | |
| ::new (@\exposid{voidify}@(op)) op_t(connect(std::move(*parts.second), std::move(r))); |
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.
We do \placeholdernc for voidify everywhere else, so doing this here as well. (Although it feels wrong.)
source/exec.tex
Outdated
| \begin{codeblock} | ||
| is_nothrow_constructible_v<remove_cvref_t<Sndr>, Sndr> && | ||
| is_nothrow_move_constructible_v<@\exposid{wrap-sender}@> && | ||
| (is_same_v<Sndr, remove_cvref_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.
is_same_v<Sndr, remove_cvref_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.
This is meant to be remove_cvref_t<Sndr>. Artifacts of the unescaped html tag can be found in the paper source.
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.
Fixed.
| \indexlibrarymember{\exposid{run}}{execution::\exposid{spawn-state}}% | ||
| \begin{itemdecl} | ||
| void @\exposid{run}@(); | ||
| void @\exposid{run}@() noexcept; |
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.
Forgot to make it noexcept in synopsis.
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.
Fixed.
source/exec.tex
Outdated
| requires(const Token token) { | ||
| { token.try_associate() } -> @\libconcept{same_as}@<bool>; | ||
| { token.disassociate() } noexcept -> @\libconcept{same_as}@<void>; | ||
| { token.try_associate() } -> @\libconcept{scope_association}@<bool>; |
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.
Maybe this fix would be worth a separate fixup commit; it's not in the paper at least.
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 should not have <bool>.
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.
"fixups" are squashed eventually. If we have non-obvious fixes beyond the paper's contents, those should go into separate non-fixup comments.
That said, this change is in the incoming paper, as far as I can say: "Let test-sender and test-env be unspecified types such that ..." (and the codeblock that follows).
(Fixed <bool> transcription error.)
source/exec.tex
Outdated
| }; | ||
|
|
||
| template<class Scope> | ||
| @\exposid{struct association-t}@; // \expos |
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.
| @\exposid{struct association-t}@; // \expos | |
| struct @\exposid{association-t}@; // \expos |
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.
Fixed.
| \item | ||
| if \tcode{assoc} is engaged, | ||
| then \tcode{assoc.try_associate()} is equivalent to | ||
| \tcode{assoc.scope->try-associate()}, and |
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.
| \tcode{assoc.scope->try-associate()}, and | |
| \tcode{assoc.scope->\exposid{try-associate}()}, and |
source/exec.tex
Outdated
| \tcode{assoc.scope->try-associate()}, and | ||
| \item | ||
| the association owned by \tcode{assoc} | ||
| is released by invoking \tcode{assoc.scope->disassociate()}. |
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.
| is released by invoking \tcode{assoc.scope->disassociate()}. | |
| is released by invoking \tcode{assoc.scope->\exposid{disassociate}()}. |
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.
Fixed.
source/exec.tex
Outdated
| \pnum | ||
| \effects | ||
| If \exposid{count} is equal to \tcode{max_associations}, | ||
| then no effects. Otherwise, if \exposid{state} is |
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.
| then no effects. Otherwise, if \exposid{state} is | |
| then no effects. | |
| Otherwise, if \exposid{state} is |
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.
Fixed.
Fixes NB FI-392, CA-393 (C++26 CD).
Fixes #8475
Fixes cplusplus/papers#2426
Fixes cplusplus/nbballot#967
Fixes cplusplus/nbballot#968