[libcxx-commits] [PATCH] D100587: [libc++][ranges] iterator.concept.sizedsentinel: sized_sentinel_for and disable_sized_sentinel_for.
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Apr 20 10:20:33 PDT 2021
Quuxplusone accepted this revision.
Quuxplusone added a comment.
This revision is now accepted and ready to land.
LGTM % comments — fix the nits and land it!
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:39-41
+ friend constexpr bool operator==(int_sized_sentinel, int*);
+ friend constexpr long operator-(int_sized_sentinel, int*);
+ friend constexpr long operator-(int*, int_sized_sentinel);
----------------
Please remove `constexpr`.
Ditto lines 74-76 and lines 82-86.
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:44-46
+static_assert(
+ !std::sized_sentinel_for<
+ int*, int_sized_sentinel>); // int_sized_sentinel is not an iterator.
----------------
Nit: Either break the line between `;` and `//`, or simply s/int_sized_sentinel/IntSentinel/ so that you don't need to break the line.
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:53
+static_assert(
+ !std::sized_sentinel_for<no_default_ctor, std::vector<int>::iterator>);
+
----------------
`s/std::vector<int>::iterator/int*/`; then you don't need to break the line.
Ditto on line 60.
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:56
+struct not_copyable {
+ not_copyable(not_copyable const&) = delete;
+ bool operator==(std::input_or_output_iterator auto) const;
----------------
Add: `not_copyable() = default;`
Otherwise, this isn't significantly different from the `no_default_ctor` case. (Remember, a class with //any// declared ctors doesn't get a defaulted default ctor, unless you explicitly default the default ctor.)
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:109-110
+ static_assert(!std::sized_sentinel_for<const_iterator, reverse_iterator>);
+ static_assert(
+ !std::sized_sentinel_for<const_iterator, const_reverse_iterator>);
+
----------------
Remove this linebreak. Ditto through the end of the file.
================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sized_sentinel_for.compile.pass.cpp:130-131
+static_assert(
+ std::sized_sentinel_for<std::move_iterator<std::vector<int>::iterator>,
+ std::move_iterator<std::vector<int>::iterator> >);
+
----------------
`s/std::vector<int>::iterator/int*/g` so that you don't have to break the line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100587/new/
https://reviews.llvm.org/D100587
More information about the libcxx-commits
mailing list