This is an archive of the discontinued LLVM Phabricator instance.

Implement part of P0031; adding `constexpr` to `reverse_iterator`
ClosedPublic

Authored by mclow.lists on Oct 12 2016, 3:13 PM.

Details

Summary

This just does the reverse_iterator bits of http://wg21.link/P0031 - not any of the other parts.
This duplicates some (but not all) of the work that was done in D20915 and D22584.

The win here (I think) is the extensive tests.

Diff Detail

Event Timeline

mclow.lists retitled this revision from to Implement part of P0031; adding `constexpr` to `reverse_iterator`.
mclow.lists updated this object.
mclow.lists added a subscriber: cfe-commits.
EricWF accepted this revision.Oct 13 2016, 4:25 PM
EricWF edited edge metadata.

LGTM modulo inline comments.

include/iterator
95

Missing an = here.

test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.op=/reverse_iterator.pass.cpp
53

This actually tests the copy assignment operator since std::make_reverse_iterator(p) returns reverse_iterator<const Derived *>.
Maybe:

using BaseIt = std::reverse_iterator<const Base *>;
BaseIt it2 = (BaseIt{nullptr} = it1);

It would also be nice if p was non-null so we could test that the actual assignment took place.

54

The static assert is missing a message and is poorly aligned.

test/std/iterators/predef.iterators/reverse.iterators/reverse.iter.ops/reverse.iter.opref/op_arrow.pass.cpp
113

How about just:

constexpr const char *p = "123456789";
constexpr auto it = std::make_reverse_iterator(p+1);
static_assert(it.operator->() == p, "");
This revision is now accepted and ready to land.Oct 13 2016, 4:25 PM
mclow.lists closed this revision.Oct 19 2016, 8:22 AM
mclow.lists marked 3 inline comments as done.

Landed as revision 284602