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
[array.zero] Fix triple comparison and improve wording consistency #6807
base: main
Are you sure you want to change the base?
Conversation
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 seems like a net improvement, and editorial, but has a lot of overlap with LWG 2157
source/containers.tex
Outdated
In the case that \tcode{N == 0}, \tcode{begin() == end() ==} unique value. | ||
The return value of \tcode{data()} is unspecified. | ||
In the case that \tcode{N} equals \tcode{0}, | ||
the effect of calling \tcode{front()} or \tcode{back()} undefined. |
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.
Would this be better addressed by putting a precondition on the front
and back
functions that N > 0
. Does such a precondition fall out of the sequence container requirements already? If so, this should be a note, not a normative statement.
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.
https://eel.is/c++draft/sequence.reqmts#itemdecl:20 front()
and back()
dereference iterators.
To me, it seems like a better overall wording strategy is to make explicitly disallow this for empty arrays, like in LWG 2157.
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.
Is it intended not to mention operator[]
?
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.
My main point is that if this is already a precondition, the text here should be a Note, not (redundantly) normative. Do not make array<T, 0>
seem any more special than it needs to be.
\tcode{array} shall provide support for the special case \tcode{N == 0}. | ||
In the case that \tcode{N} equals \tcode{0}, | ||
\tcode{begin()} equals \tcode{end()} and | ||
the return value of \tcode{data()} is unspecified. |
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 a normative change, not editorial, as it gives up the requirement that different empty array objects return iterators with a different value. That point may be deemed moot as it is observed only by comparing pointers from different ranges.
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.
You could observe a difference if the iterator was of pointer type and std::less
compared iterators from two empty arrays. The intent of the current wording seems to be that the iterators from two separate arrays cannot be equivalent according to the total ordering of pointers.
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 intent of the current wording seems to be that the iterators from two separate arrays cannot be equivalent according to the total ordering of pointers.
My reading is that this paragraph intended to guarantee that
- an iterator obtained from
array<T, 0>
object and a value-initialized iterator never compare equal; and - two iterators obtained from two different
array<T, 0>
objects never compare equal.
And without such guarantees the comparison has (or can have, as decided by the library implementation) undefined behavior ([iterator.concept.forward]/2, [forward.iterators]/2).
However, it is not clear whether the requirement on unique value overrides the specification for forward iterators, and implementations now consistently return value-intialized iterators (libc++ was an exception and conformed to such guarantees, but is not now). I think we should open an LWG issue to drop "unique value".
Well thanks. I think it was editorial up to a point but removing the requirement for uniqueness of iterators is not. LWG 2157 overhauls the whole section anyway, but I still believe there would be value in some very minor editorial changes. That triple comparison in p2 should be cleaned up for example. I'll scale this PR down. |
Co-authored-by: Jonathan Wakely <github@kayari.org>
The uniqueness requirement for iterators is actually problematic for constexpr iterators. We don't have a pointer to a |
Co-authored-by: Jonathan Wakely <github@kayari.org>
[array.zero] p1
This paragraph is not normative and basically pointless. The existence of a section called "Zero-sized arrays" in itself implies that there is some special support for zero-sized arrays. It's also entirely unclear what it means to "provide support"; no concrete demand is being made here. Therefore, this paragraph should be deleted.
[array.zero] p2
This paragraph contains the broken expression
begin() == end() ==
unique value. Such a triple comparison would work in mathematical notation, but in C++, this implies nothing forbegin() == end()
, only that "unique value" is being equality-compared to an expression of typebool
.In general, the convention is to write "
x == y
istrue
" or "x
equalsy
" when something normative is expressed, since a C++ expression does not have any such implication, even if it is of typebool
.Furthermore, it is entirely unclear what "unique value" is in this context. Does it mean that every invocation of
begin()
andend()
produces a unique value, andbegin()
andend()
compare equal? Does it mean that the result of the expression ofbegin() == end()
is a unique value for each comparison?I'm not even sure if anything normative is being expressed here, and the intent is clearly just to say that
begin()
equalsend()
for empty arrays. Even if it said "begin()
returns a unique value", what would that actually mean? A value "being unique" does not have any obvious standard meaning, so it could be seen as fluff.[array.zero] p3
This paragraph is the least problematic, but uses language inconsistent with p2. It would be more consistent if all paragraphs began with the same prefix.
[array.zero] p4
This paragraph has a similar issue as p4. The intent is obviously not that
swap()
is alwaysnoexcept
because otherwise, it would benoexcept
in the synopsis. This paragraph is located in [array.zero] so presumably, it is only meant to apply to zero-length arrays. Let's prefix it and make this clear.