[coro.generator] Editorial fixes #6230
Conversation
Iterators can't have been "previously obtained from" a by-value parameter; they must have been obtained elsewhere (e.g. from `x` and then `other` was move-constructed from `x`, so now we have iterators into `x`). `std::exchange(x, nullptr)` for `unique_ptr x` is equivalent to `std::move(x)`.
Quuxplusone
force-pushed
the
generator-editorial
branch
from
8500fad
to
97a86d8
Compare
| @@ -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_)}. | |||
There was a problem hiding this comment.
I think it is beneficial to keep the consistency of always using exchange when defining a move constructor.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Iterators can't have been "previously obtained from" a by-value parameter; they must have been obtained elsewhere (e.g. from
xand thenotherwas move-constructed fromx, so now we have iterators intox).std::exchange(x, nullptr)forunique_ptr xis equivalent tostd::move(x).