[libcxx-commits] [PATCH] D100587: [libc++][ranges] iterator.concept.sizedsentinel: sized_sentinel_for and disable_sized_sentinel_for.

[Christopher Di Bella via Phabricator via libcxx-commits]

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

`std::list` hasn't had its conformance test updated, but otherwise I have one nit.



================
Comment at: libcxx/test/std/containers/associative/map/iterator_concept_conformance.compile.pass.cpp:33-34
 static_assert(!std::sentinel_for<iterator, const_reverse_iterator>);
+static_assert(!std::sized_sentinel_for<const_iterator, reverse_iterator>);
+static_assert(!std::sized_sentinel_for<const_iterator, const_reverse_iterator>);
 
----------------
This should be for `iterator`, and is missing checks for `iterator` and `const_iterator` on the RHS. Same for other files too.


================
Comment at: libcxx/test/std/containers/associative/map/iterator_concept_conformance.compile.pass.cpp:44-45
 static_assert(!std::sentinel_for<const_iterator, const_reverse_iterator>);
+static_assert(!std::sized_sentinel_for<const_iterator, reverse_iterator>);
+static_assert(!std::sized_sentinel_for<const_iterator, const_reverse_iterator>);
----------------
Missing checks for `iterator` and `const_iterator` on the RHS. Same for other files too.


================
Comment at: libcxx/test/std/containers/sequences/array/iterator_concept_conformance.compile.pass.cpp:42-60
+static_assert(std::sized_sentinel_for<iterator, iterator>);
+static_assert(std::sized_sentinel_for<iterator, const_iterator>);
+static_assert(std::sized_sentinel_for<const_iterator, iterator>);
+static_assert(std::sized_sentinel_for<const_iterator, const_iterator>);
+
+static_assert(!std::sized_sentinel_for<iterator, reverse_iterator>);
+static_assert(!std::sized_sentinel_for<iterator, const_reverse_iterator>);
----------------
Could we order these as if they were a set of increasing two digit numbers please?
Where

iterator: 0
const_iterator: 1
reverse_iterator: 2
const_reverse_iterator: 3

I think that will make it easier to track that all have been accounted for in the right permutation.


================
Comment at: libcxx/test/std/containers/unord/unord.map/iterator_concept_conformance.compile.pass.cpp:61
+static_assert(!std::sized_sentinel_for<const_iterator, iterator>);
+static_assert(!std::sized_sentinel_for<const_iterator, const_iterator>);
----------------
`local_iterator` and `const_local_iterator` need checking too please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100587