-
Notifications
You must be signed in to change notification settings - Fork 778
P3111R8 Atomic Reduction Operations #8025
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
\end{note} | ||
For \tcode{store_max} and \tcode{store_min}, | ||
the maximum and minimum computation is performed | ||
as if by \tcode{max} and \tcode{min} algorithms\iref{alg.min.max}, respectively, |
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.
Might want to say "as if by the max
and min
algorithms", but I guess, either is technically okay.
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 phrasing is copied from fetch_min/max. I'd suggest to make a separate pull request after the motions merge.
@@ -4740,6 +5050,35 @@ | |||
constexpr @\placeholdernc{integral-type}@ fetch_min( @\placeholdernc{integral-type}@, | |||
memory_order = memory_order::seq_cst) noexcept; | |||
|
|||
void store_add(@\placeholdernc{integral-type}@, |
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.
Am I unfamiliar with something, or why is this using placeholder
instead of exposid
? According to the comments in macros.tex, placeholder
is for placeholders in the middle of identifiers, like N
in intN_t
, and integral-type
really just looks like an exposition-only type to me.
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 a meta-placeholder, because we stamp out a full specialization for each integer type. There is an implied "foreach" around this subclause. exposid
isn't like that.
\pnum | ||
\begin{itemize} |
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.
Missing \remarks
here. I actually happened to be in LWG at the time we've looked at this wording and pointed out it's weird to me that we have two separate Remarks specifications in the place, but it was not seen as an issue.
@jwakely having a second Remarks here is intentional, right?
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.
The automatic checking won't allow duplicate elements.
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 we can live with just one
722505e
to
02fe942
Compare
02fe942
to
23eada3
Compare
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.
I'm fine with merging as is, even though it feels like there are some loose threads here. Nothing we couldn't figure out after motions.
Fixes #7965
Fixes cplusplus/papers#1902