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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/ranges_minmax.h:77
       auto __result = std::__minmax_element_impl(__first, __last, __comp, __proj);
+      // We can dereference normal pointers many times but some iterators could move their value after
+      // dereferencing so we should dereference them only once. The if constexpr is a pure optimization, we
----------------
IMO we should special-case the one element case here instead. In no other case will the first and second iterator point to the same element.


================
Comment at: libcxx/include/__algorithm/ranges_minmax.h:78
+      // We can dereference normal pointers many times but some iterators could move their value after
+      // dereferencing so we should dereference them only once. The if constexpr is a pure optimization, we
+      // don't want an additional branch for normal pointers.
----------------
You have to add a regression test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142864