[libcxx-commits] [PATCH] D127211: [libc++] Implement ranges::{reverse, rotate}_copy

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__iterator/reverse_iterator.h:326
+template <class _Iter, class _Sent>
+_LIBCPP_HIDE_FROM_ABI inline _LIBCPP_CONSTEXPR
+pair<reverse_iterator<_Iter>, reverse_iterator<_Iter>> __reverse_range(_Iter __first, _Sent __last) {
----------------
`inline` is unnecessary since this is a template.
I suspect constexpr won't work in C++11/14/17 (but you could check 17, it might).


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.reverse/ranges.reverse_copy.pass.cpp:29
+#include "almost_satisfies_types.h"
+#include "test_iterators.h"
+
----------------
I would like an explicit test that `is_same<reverse_copy_result<....>, in_out_result<...>>`. In fact, I don't see that type mentioned anywhere in the tests, so we wouldn't even notice if we didn't define it. Please do that here but also for all the other algorithms that have been added and that had accompanying `foo_result` types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127211