This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Update all the pre-defined iterator types for C++20
ClosedPublic

Authored by ldionne on May 27 2021, 11:23 AM.

Details

Summary

Make sure we provide the correct It::difference_type member and update
the tests and synopses to be accurate.

Supersedes D102657 and D103101 (thanks to the original authors).

Diff Detail

Event Timeline

ldionne requested review of this revision.May 27 2021, 11:23 AM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2021, 11:23 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante requested changes to this revision.May 27 2021, 12:04 PM
Mordante added a subscriber: Mordante.

Thanks a lot for working on this! After this has landed I can remove some work-arounds from my <format> branch.

libcxx/include/iterator
253

std::ptrdiff_t -> ptrdiff_t here and other places.

280–281

I miss constexpr front_insert_iterator() noexcept = default; // since C++20 and the implementation.
back_insert_iterator has the same issue.

305–306

I miss insert_iterator() = default; // since C++20 and the implementation. (This one has no constexpr nor noexcept.)

392–400

Remove std:: here and similar places.

This revision now requires changes to proceed.May 27 2021, 12:04 PM
ldionne updated this revision to Diff 348506.May 28 2021, 6:04 AM
ldionne marked 4 inline comments as done.

Address review comments.

ldionne added inline comments.
libcxx/include/iterator
280–281

Good catch, somehow I missed those. Thanks.

502
libcxx/include/iterator
502

It'll be DRed out of existence by p2325r3, so I bet it's not worth fixing on cppreference.
(See my LWG reflector thread of last week, "Must ostreambuf_iterator<char>().failed() == false?".)
There's an executive design decision to be made here as to whether we should just go ahead and implement p2325 (although it seems like both me and Louis have relatively independently made decisions to implement not-p2325). Either p2325 or not-p2325 would suffice to unblock @Mordante AIUI.

899

I strongly recommend removing the redundant constexpr and noexcept, for cleanliness' sake. (Defaulted functions are always constexpr-auto and noexcept-auto.)
Ditto line 908, 1075, 1200; already OK on line 954.

libcxx/test/std/iterators/iterator.primitives/iterator.traits/cxx20_iterator_traits.compile.pass.cpp
58

Nice! :)

Mordante accepted this revision as: Mordante.May 28 2021, 9:35 AM

LGTM! I'll leave the final approval for @Quuxplusone.

libcxx/include/iterator
502

Yeah when P2325 is applied the default constructors will be removed, but an output_iterator should no longer require default_constructible. For <format> I only need the back_insert_iterator to be an output_iterator.

899

I've no strong opinion about this, but I slight prefer to use the wording the standard provides. Thus including constexpr and noexept.

ldionne accepted this revision as: Restricted Project.May 31 2021, 8:57 AM
ldionne marked 3 inline comments as done.
ldionne added inline comments.
libcxx/include/iterator
899

Same as @Mordante , which is why I did it that way.

This revision is now accepted and ready to land.May 31 2021, 8:57 AM