[libcxx-commits] [PATCH] D100160: [libcxx] adds `std::input_or_output_iterator` and `std::sentinel_for` to <iterator>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM with @Mordante 's comment fixed and CI passing. Thanks!



================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.iterator/input_or_output_iterator.compile.pass.cpp:53-70
+static_assert(!std::input_or_output_iterator<int (S::*)()>);
+static_assert(!std::input_or_output_iterator<int (S::*)() noexcept>);
+static_assert(!std::input_or_output_iterator<int (S::*)() &>);
+static_assert(!std::input_or_output_iterator<int (S::*)() & noexcept>);
+static_assert(!std::input_or_output_iterator<int (S::*)() &&>);
+static_assert(!std::input_or_output_iterator < int (S::*)() && noexcept >);
+static_assert(!std::input_or_output_iterator<int (S::*)() const>);
----------------
It's not clear to me that testing *all* of those qualifiers adds that much value over just testing one of them. Non-blocking, but if I had written this myself, I wouldn't have tested all of them since it feels like diminishing returns to me.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.sentinel/sentinel_for.compile.pass.cpp:13
+
+// template<class In>
+// concept sentinel_for;
----------------
Mordante wrote:
> Please update this part to `template<class S, class I>`
You seem to have missed this comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100160/new/

https://reviews.llvm.org/D100160