Details
- Reviewers
philnik huixie90 ldionne - Group Reviewers
Restricted Project - Commits
- rG14cf74d65d9f: [libc++][ranges] Implement `ranges::shuffle`.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/test/std/algorithms/alg.modifying.operations/alg.random.shuffle/ranges_shuffle.pass.cpp | ||
---|---|---|
243 | The standard requirement that the number of swaps is "exact" instead of "at most" doesn't make sense to me. Is this a wording issue, or am I missing something? |
libcxx/test/std/algorithms/alg.modifying.operations/alg.random.shuffle/ranges_shuffle.pass.cpp | ||
---|---|---|
243 | It is somewhat unusual indeed -- I don't think anybody would complain for making fewer swaps than specified. |
libcxx/test/std/algorithms/alg.modifying.operations/alg.random.shuffle/ranges_shuffle.pass.cpp | ||
---|---|---|
243 | There are some algorithms that enforce exactly a certain number of swaps (like swap_ranges), but I think the wording should be changed to say "at most" rather than "exactly" for ranges::shuffle. I filed https://github.com/cplusplus/draft/issues/5636 for clarification. |
LGTM
libcxx/test/support/test_iterators.h | ||
---|---|---|
828 | not sure how this is used in the test but usually iter_swap would swap the underlying references i.e std::swap(*a.ptr_, *b.ptr_) |
libcxx/include/__algorithm/ranges_shuffle.h | ||
---|---|---|
70 | not an issue but just write it down as I had thought about it. Not sure how this could possibly go wrong but the original requirement is that you can do std::invoke(__gen). I guess it is ok to call __gen() instead of but I'd still prefer to stick to original requirement |
clang-format not found in user’s local PATH; not linting file.