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
[span.iterators] Fix cross-reference to container iterators #6183
Conversation
The current cross-reference is to [containers.requirements], which is the whole containers requirements clause, including not just general containers, but also allocator-aware, reversible, sequence, associative, and unodered associative containers. It seems very unlikely that the cross-reference expects to be the intersection of all of those. Rather, the reference seems to intend just the [containers.reqmts] subclause, which adds just two useful constraints: random access iterators should support the 3-way comparison operator, and the interoperabiity of container iterators and const_iterators. As a drive-by, fixed the \indexlibrarymember macro by ordering the type, span, before the member, iterator, as is the convention. This has no impact on the index, as entries in both directions are created. However, it makes the wording of index macros across the clause consistent.
Looking to @CaseyCarter to confirm cross-ref change is correct, and @jwakely to confirm this change is editorial. |
If that's the case, there's another place equally affected:
|
There's more similarly affected:
|
Yes, would be nice to fix them all.
Not really; please "grep indexlibrarymember" and you'll see the convention is usually "member, type". Please split this off into a separate commit or a separate pull request if you continue to feel this needs alignment. |
source/containers.tex
Outdated
@@ -18364,7 +18364,7 @@ | |||
|
|||
\rSec4[span.iterators]{Iterator support} | |||
|
|||
\indexlibrarymember{iterator}{span}% | |||
\indexlibrarymember{span}{iterator}% |
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 not what we usually do (globally); please split off into a separate commit.
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.
Reverted this drive-by, no separate commit forthcoming at this time.
Actually, these cases are slightly different. Span is not a container, so it is unclear which of the various container requirements are intended to apply. All of these other cases are containers, and the category of container is known, so the whole subclause does apply additional constraints. I could apply the broader edit if requested, but suggest that should be under a different ticket as the reasoning is more involved. |
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.
squash before merge
The current cross-reference is to [containers.requirements], which is the whole containers requirements clause, including not just general containers, but also allocator-aware, reversible, sequence, associative, and unodered associative containers. It seems very unlikely that the cross-reference expects to be the intersection of all of those.
Rather, the reference seems to intend just the [containers.reqmts] subclause, which adds just two useful constraints: random access iterators should support the 3-way comparison operator, and the interoperabiity of container iterators and const_iterators.
As a drive-by, fixed the \indexlibrarymember macro by ordering the type, span, before the member, iterator, as is the convention. This has no impact on the index, as entries in both directions are created. However, it makes the wording of index macros across the clause consistent.