[libcxx-commits] [PATCH] D122002: [libc++][ranges] Implement ranges::max

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_max.h:51
+    _LIBCPP_ASSERT(__il.begin() != __il.end(), "initializer_list must contain at least one element");
+    auto __comp_lhs_rhs_swapped = [&](auto&& __lhs, auto&& __rhs) { return std::invoke(__comp, __rhs, __lhs); };
+    return *ranges::__min_element_impl(__il.begin(), __il.end(), __comp_lhs_rhs_swapped, __proj);
----------------
philnik wrote:
> var-const wrote:
> > I think there's an important nuance in how equivalent elements are handled. IIUC, `min_element` returns the rightmost of equivalent elements, whereas `max` needs to return the leftmost -- therefore, with the arguments swapped, we will get the correct behavior for `max`. Can you please add a comment to that effect (assuming my reasoning is correct)?
> Both should return the leftmost element. I'm not sure how you got to the rightmost element by swapping the arguments. Did you think of less-or-equal maybe?
Never mind, I misread that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122002