[libcxx-commits] [PATCH] D101316: [libcxx][ranges] Add `random_access_{iterator, range}`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

Nit: We need to update the synopsis in `<iterator>`.

LGTM, ship this once you feel like you've addressed reviewer's comments sufficiently. I'm not seeing anything that is serious enough to block you once you've addressed what you think should be addressed, how you think it should.



================
Comment at: libcxx/test/std/containers/sequences/deque/iterator_concept_conformance.compile.pass.cpp:26
 static_assert(std::bidirectional_iterator<iterator>);
+static_assert(std::random_access_iterator<iterator>);
 static_assert(std::indirectly_writable<iterator, value_type>);
----------------
I think this should go above the `std::bidirectional_iterator` check since we seem to be going from more specific to more general (from top to bottom) as far as the iterator concepts are concerned. This is really a nit though, feel free to skip if you disagree or if it's too tedious to change throughout.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.random.access/random_access_iterator.compile.pass.cpp:145
+    self operator+(difference_type n) const;
+ /* friend self operator+(difference_type n, self x); */
+
----------------
I really like this, makes it easy to see what's being tested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101316