Skip to content
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

[container.reqmts] Fix cross-references to contiguous container #6192

Merged
merged 1 commit into from Mar 19, 2023

Conversation

AlisdairM
Copy link
Contributor

This edit deserves describing in detail, to establish that it is purely editorial.

As part of the mission to clean up tables in the standard, the general container requirements were split into 5 more focused leaf nodes. Due to its location in the original text, the definition for a contiguous container fell into the subclause for reversible containers, to which it is entirely unrelated. There is no requirement that a contiguous container be reversible, only that it has the correct category for its iterators.

Meanwhile, all 3 of the existing cross-references point to the wrong leaf node, that simply provides a key to the identifiers used throughout the specification of this clause.

The fix has two parts. First move the definition of contiguous container, and a container with special iterators, into the [container.reqmts] clause where it best belongs. This move is appended to the end so that there can be no ambiguity that any existing text could be confused with requirements only on contiguous
containers after the edit. The other part is to fix up the three cross-references to point to [container.reqmts] rather than its sibling that has no information on contiguous containers.

A grep of the .tex source files confirms that these three references (array,basic_string, and vector) are the only current uses of contiguous container, even though basic_stacktrace would also meet the requirements.

This edit deserves describing in detail, to establish that it
is purely editorial.

As part of the mission to clean up tables in the standard,
the general container requirements were split into 5 more
focused leaf nodes.  Due to its location in the original
text, the definition for a contiguous container fell into
the subclause for reversible containers, to which it is
entirely unrelated.  There is no requirement that a
contiguous container be reversible, only that it has the
correct category for its iterators.

Meanwhile, all 3 of the existing cross-references point
to the wrong leaf node, that simply provides a key to
the identifiers used throughout the specification of this
clause.

The fix has two parts.  First move the definition of
contiguous container, and a container with special
iterators, into the [container.reqmts] clause where it
best belongs.  This move is appended to the end so that
there can be no ambiguity that any existing text could
be confused with requirements only on contiguous
containers after the edit.  The other part is to fix up
the three cross-references to point to [container.reqmts]
rather than its sibling that has no information on
contiguous containers.

A grep of the .tex source files confirms that these
three references (array,basic_string, and vector) are
the only current uses of contiguous container, even
though basic_stacktrace would also meet the requirements.
@AlisdairM
Copy link
Contributor Author

Hoping @jwakely can take a look to confirm this PR is both editorial and correct.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This moves a self-contained definition and adjusts cross-references accordingly. This feels low-risk to me.

@tkoeppe tkoeppe merged commit ae8ec6f into cplusplus:main Mar 19, 2023
2 checks passed
@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 19, 2023

Thank you, nice catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants