[libcxx-commits] [PATCH] D119751: [libc++][ranges] Add ranges::min_max_result

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

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

Assuming there's nothing subtly surprising in the test file, LGTM!



================
Comment at: libcxx/include/__algorithm/min_max_result.h:26
+
+#ifndef _LIBCPP_HAS_NO_CONCEPTS
+
----------------
and likewise adjust the `#endif` comment. (Also, FYI, going forward it will help to use the form `#if !defined(_LIBCPP_HAS_NO_CONCEPTS)` consistently, instead of `#ifndef`, because greppability.)

(This is basically a merge conflict with D118736. Going forward, we should guard //everything// new in the `ranges` namespace with `_LIBCPP_HAS_NO_INCOMPLETE_RANGES`, unless it's a building block for something in the `std::` namespace, and until we are ready to kill off `_LIBCPP_HAS_NO_INCOMPLETE_RANGES` entirely, which will be a while.)


================
Comment at: libcxx/include/algorithm:23
   template <class I, class F>
     struct in_fun_result; // since C++20
 
----------------
Either space-align this comment too, or don't bother aligning comments? (I don't really care about the alignment personally. Seems like it just adds to the `git blame` spam.)


================
Comment at: libcxx/test/std/algorithms/algorithms.results/min_max_result.pass.cpp:72-74
+  auto [in1, in2] = std::ranges::min_max_result<int>{1, 2};
+  assert(in1 == 1);
+  assert(in2 == 2);
----------------
`s/in1/min/g`
`s/in2/max/g`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119751