[libcxx-commits] [PATCH] D129859: [libc++][ranges] Implement `ranges::{prev, next}_permutation`

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked 2 inline comments as done.
var-const added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.permutation.generators/ranges.prev_permutation.pass.cpp:91
+
+  auto iterator_overload = [](auto&& first, auto&& last) { return std::ranges::prev_permutation(first, last); };
+  auto range_overload    = [](auto&& first, auto&& last) {
----------------
ldionne wrote:
> philnik wrote:
> > var-const wrote:
> > > Is the output of `prev_permutation` deterministic? If yes, I'd prefer several simple test cases that check the exact output.
> > Yes, the output is deterministic. This test checks the exact output of `prev_permutation`. It checks after every  call to `ranges::prev_permutation` that it's lexicographically less than the input. It's also checked that there were N! rounds. Since there are N! permutations of a range, every input to `prev_permutation` has to have resulted in the permutation before the input.
> I think this is a clever way to test this. Maybe a little bit too clever to be the only test we have. I'd be OK with this approach if it were also augmented with a small number (e.g. 10 with boundary conditions) of explicit input/output comparisons. Does that sound reasonable?
> 
> Also, a comment explaining what the testing methodology here would be really helpful.
I refactored this function into several and added comments. However, it adds a lot of complexity and takes a while to run (I had to increase the number of `constexpr` steps and reduce the length of the input array since the growth factor is factorial).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129859