Skip to content

[coro.generator] Editorial fixes #6230

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[coro.generator] Editorial fixes #6230

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions source/ranges.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15835,11 +15835,11 @@
Initializes \exposid{coroutine_} with
\tcode{exchange(other.\exposid{coroutine_}, \{\})} and
\exposid{active_} with
\tcode{exchange(\brk{}other.active_, nullptr)}.
\tcode{std::move(\brk{}other.active_)}.
Copy link
Member

Choose a reason for hiding this comment

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

I think it is beneficial to keep the consistency of always using exchange when defining a move constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is beneficial to keep the consistency of always using exchange when defining a move constructor.

This is the only place in "ranges.tex" where exchange( is mentioned. Every other place where we define a move-constructor, we use std::move; e.g. https://eel.is/c++draft/range.elements.iterator#5.sentence-1 , https://eel.is/c++draft/range.elements#sentinel-2.sentence-1 , https://eel.is/c++draft/range.take.while.sentinel#2 , https://eel.is/c++draft/range.adjacent.sentinel#2 , ...

I'm not familiar with any other place in the standard that uses exchange when it doesn't need to. git grep 'Initializes.*exchange' has zero hits. git grep '\(^\|with \).tcode.exchange' has only three hits, all in this paragraph.


\pnum
\begin{note}
Iterators previously obtained from \tcode{other} are not invalidated;
Iterators previously into \tcode{other} are not invalidated;
Copy link
Member

Choose a reason for hiding this comment

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

The new phrasing doesn't read like English to me. Also, other is an rvalue reference, not a value, so it seems iterators could have been obtained from other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that "iterators (previously) into X" is a little vague. Other places (e.g. https://eel.is/c++draft/associative.reqmts.general#115.sentence-2 ) use "iterators pointing to elements of X", but we can't do that here because generators don't have elements. The phrase "iterators into X" is used in https://eel.is/c++draft/sequence.reqmts#37.sentence-3 , as well as on the left-hand side of this diff.

This note could be updated to say "this operation does not modify any iterator it in the program, regardless of whether it.coroutine_ == other.coroutine_ before the operation," but that's obviously tautological, as opposed to being subtly tautological. I think it would also be reasonable to just kill this note entirely.

Re other here (in the move-constructor) being an rvalue reference: True, I updated this wording only for consistency with the wording in operator=, where other is a value (not a reference). I think if the notes are kept, they should be kept parallel, and not with different wording in different places.

they become iterators into \tcode{*this}.
\end{note}
\end{itemdescr}
Expand Down Expand Up @@ -15889,7 +15889,7 @@

\pnum
\begin{note}
Iterators previously obtained from \tcode{other} are not invalidated;
Iterators previously into \tcode{other} are not invalidated;
they become iterators into \tcode{*this}.
\end{note}
\end{itemdescr}
Expand Down