[libcxx-commits] [PATCH] D119589: [libc++][ranges] Implement ranges::min

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:48
+
+  template<forward_iterator I, sentinel_for<I> S, class Proj = identity,
+    indirect_strict_weak_order<projected<I, Proj>> Comp = ranges::less>
----------------
var-const wrote:
> Hmm, looking at the Standard, I see
> ```
> template<class T, class Proj = identity,
>          indirect_­strict_­weak_­order<projected<const T*, Proj>> Comp = ranges::less>
>   constexpr const T& ranges::min(const T& a, const T& b, Comp comp = {}, Proj proj = {});
> 
> // initializer_list overload looks the same
> 
> template<input_­range R, class Proj = identity,
>          indirect_­strict_­weak_­order<projected<iterator_t<R>, Proj>> Comp = ranges::less>
>   requires indirectly_­copyable_­storable<iterator_t<R>, range_value_t<R>*>
>   constexpr range_value_t<R>
>     ranges::min(R&& r, Comp comp = {}, Proj proj = {});
> ```
> 
> Should this be fixed (here and in the test file)?
Yikes, good catch! (The current PR has `constexpr I min(I first, S last, ...` presumably copy-pasted from `min_element`.)  It'll also be a very good idea to make sure we have at least one test for, like, `std::ranges::min(p, p+1)` (takes the min of the pointer values, not the min element of the range), and also a SFINAE test for `std::ranges::min(It, Sent)` where the `It` and `Sent` are of different types. If these tests already exist, awesome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119589