[libcxx-commits] [PATCH] D127049: [libc++] Unwrap reverse_iterator<reverse_iterator<Iter>> in __unwrap_iter

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I like this approach! I think the test coverage should be a bit higher.



================
Comment at: libcxx/include/__algorithm/unwrap_iter.h:69
+inline _LIBCPP_HIDE_FROM_ABI constexpr
+auto __unwrap_iter(reverse_iterator<reverse_iterator<_Iter>> __iter) noexcept {
+    return std::__unwrap_iter(__iter.base().base());
----------------
In general we avoid using `auto` as return type.


================
Comment at: libcxx/test/libcxx/algorithms/unwrap_iter.compile.pass.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14
+
----------------
The unwrap iterator is supported in C++11 and C++14.


================
Comment at: libcxx/test/libcxx/algorithms/unwrap_iter.compile.pass.cpp:14
+#include <algorithm>
+#include <type_traits>
+
----------------
I would like to see some additional tests using Standard containers that use wrapped iterator.

I also would like to see some runtime tests for these containers verifying the result has the expected value. The copy test above only test with an array.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127049