Fix excessive comparisons in minmax algorithms#6264
Fix excessive comparisons in minmax algorithms#6264mirion-dev wants to merge 9 commits intomicrosoft:mainfrom
Conversation
|
It's weird that the failed test |
|
@zacklj89 had also encountered |
There was a problem hiding this comment.
Pull request overview
This PR updates the STL implementations of std::minmax_element / std::ranges::minmax_element to meet the C++ standard comparison-count upper bound for even-sized inputs, and adds targeted tests that reproduce/regress the issue. It also slightly optimizes std::ranges::minmax.
Changes:
- Adjust
std::minmax_elementandstd::ranges::minmax_elementinitialization/loop structure to avoid the extra comparison whenNis even. - Optimize
std::ranges::minmaxinitialization to reduce comparisons. - Add new comparison-count regression tests for both legacy and ranges algorithms.
Show a summary per file
| File | Description |
|---|---|
stl/inc/algorithm |
Fixes minmax-element comparison counts for even N; also optimizes ranges::minmax initialization. |
tests/std/tests/P0896R4_ranges_alg_minmax/test.cpp |
Adds constexpr + runtime comparison-count tests for ranges::minmax and ranges::minmax_element, including an input-range case. |
tests/std/tests/Dev11_0532622_minmax_element/test.cpp |
Adds legacy std::minmax_element comparison-count regression tests (guarded to avoid debug-predicate effects). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Failed on |
|
Thanks (I will deal with the midpoint lerp failures, somehow). |
According to [alg.min.max#32], the algorithms should perform at most
max(floor(3/2 (N−1)),0)comparisons, but the current implementations ofstd::{ranges::}minmax_elementviolate this requirement whenNis even. This can be reproduced with the added test cases in this PR.Although
std::ranges::minmaxdoes not violate it because its upper bound is(3/2) N[alg.min.max#23], I have optimized it as well.