On Wed, 29 Jan 2020, Jonathan Wakely wrote:
On 21/01/20 17:26 -0500, Patrick Palka wrote:
It seems that in practice std::sentinel_for<I, I> is always true, and so
the
Doh, good catch.
test_range container doesn't help us detect bugs in ranges code in which we
wrongly assume that a sentinel can be manipulated like an iterator. Make
the
test_range container more strict by having end() unconditionally return a
sentinel<I>.
Yes, this seems useful.
Is this OK to commit after bootstrap+regtesting succeeds on
x86_64-pc-linux-gnu?
As you mentioned on IRC, this makes some tests fail.
Some of them are bad tests and this change reveals that, e.g.
std/ranges/access/end.cc should not assume that r.end()==r.end() is
valid (because sentinels can't be compared with sentinels).
In other cases, the test is intentionally relying on the fact the
r.end() returns the same type as r.begin(), e.g. in
std/ranges/access/rbegin.cc we want to test this case:
— Otherwise, make_reverse_iterator(ranges::end(t)) if both
ranges::begin(t) and ranges::end(t) are valid expressions of the same
type I which models bidirectional_iterator (23.3.4.12).
If the sentinel returned by ranges::end(r) is a different type, we
can't use test_range to verify this part of the ranges::rbegin
requirements.
I think we do want your change, so this patch fixes the tests that
inappropriately assume the sentinel is the same type as the iterator.
When we are actually relying on that property for the test, I'm adding
a new type derived from test_range which provides that guarantee and
using that instead.
There's one final change needed to make std/ranges/access/size.cc
compile after your patch, which is to make the test_range::sentinel
type satisfy std::sized_sentinel_for when the iterator satisfies
std::random_access_iterator, i.e. support subtracting iterators and
sentinels from each other.
Tested powerpc64le-linux, committed to trunk.
Thanks for the review and the testsuite fixes.
Please go ahead and commit your patch to test_range::end() after
re-testing. Thanks, and congratulations on your first libstdc++
patch.
Thanks! :) I am re-testing now, and will commit the patch after that's
done.
