-
Notifications
You must be signed in to change notification settings - Fork 804
P3663R3 Future-proof submdspan_mapping
#8526
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 PL-009, US 66-117 (C++26 CD).
That comment is asking to rename two things, one of which was added by this paper. This commit doesn't address that comment. |
source/containers.tex
Outdated
| \tcode{SubExtents::rank()} equals the number of $k$ such that | ||
| $S_k$ does not model \tcode{\libconcept{convertible_to}<IndexType>}; and | ||
| \tcode{SubExtents::rank()} equals | ||
| \tcode{\exposid{MAP_RANK}(slices, Extents::rank())}; 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.
I think we have the same issue as above here with Extents::rank(). The existing text below says "for each rank index $k$ of \tcode{Extents}", so it may pay to introduce a name for the extents type. For example, we could change paragraph 1 like this
Let
Edenoteextents<IndexType, Extents...>, and letslicesbe the pack ....
and then use E throughout, e.g., in paragraph 2.
That being said, perhaps it's better just to merge the text as-is and then file an LWG issue with the above as a proposed fix. What do you think?
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.
That sounds too invasive to do as part of applying the paper.
mhoemmen
left a comment
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 for doing the wording for this! : - )
| \tcode{M::index_type} | ||
| if the function is a member of a class \tcode{M}, | ||
| the declaration \tcode{auto [...ls] = std::move(s);} is well-formed | ||
| for some object \tcode{s} of type $S$, |
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.
S was not in math font in the paper, but I agree that it should be, as was done here.
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 paper requests a drive-by fix from layout_left to layout_right here, but that was already done in f2b0254
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 same commit also fixed _rank to rank_ as also requested by the paper.
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 paper also asks for two drive-by fixes here, which were already done by 82bf75b
This comment was marked as resolved.
This comment was marked as resolved.
…es like punctuation
This title was not in the paper and was suggested by the author during review of the PR.
Fixes #8464
Fixes NB PL-009, US 66-117 (C++26 CD).
Also fixes cplusplus/papers#2292
Also fixes https://github.com/cplusplus/nbballot/issues/695
Also fixes https://github.com/cplusplus/nbballot/issues/818
Somehow relates to https://github.com/cplusplus/nbballot/issues/815 but I'm not sure how