[libcxx-commits] [PATCH] D142864: [libc++] `<algorithm>`: `ranges::minmax` should dereference iterators only once

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Feb 13 08:27:07 PST 2023


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

LGTM % test comment.



================
Comment at: libcxx/include/__algorithm/ranges_minmax.h:102
 
+      // In initialization members can refer to previous members, similar to the N4928 [dcl.init.aggr]/6 example
       ranges::minmax_result<_ValueT> __result = {*__first, __result.min};
----------------
WDYT?


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.min.max/ranges.minmax.pass.cpp:352-360
   {
-    // check that the iterator isn't moved from multiple times
+    // check that the random access iterator isn't moved from multiple times
     std::shared_ptr<int> a[] = { std::make_shared<int>(42) };
     auto range = std::ranges::subrange(std::move_iterator(a), std::move_iterator(a + 1));
     auto [min, max] = std::ranges::minmax(range);
     assert(a[0] == nullptr);
     assert(min != nullptr);
----------------
Could you remove this and instead use the `constexpr` test case with `forward_iterator`s, so the we actually test for the bug now, instead of waiting for `move_iterator` to be random-access?


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

https://reviews.llvm.org/D142864



More information about the libcxx-commits mailing list