Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround Devcom-1559808 in ranges::sort #2290

Merged
merged 5 commits into from
Nov 13, 2021

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Oct 21, 2021

If there's one thing that must work in C++, it's sorting vector =)

For ease of review: DevCom-1559808.

This also moves _Rewrap_subrange to <xutility> because it's already being used there:

return _Rewrap_subrange<subrange<_It1>>(_First1, _STD move(_UResult));

If there's one thing that _must_ work in C++, it's sorting `vector` =)
@CaseyCarter CaseyCarter added the bug Something isn't working label Oct 21, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner October 21, 2021 05:23
Copy link
Contributor

@AlexGuteniev AlexGuteniev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also occurs in:

STL/stl/inc/algorithm

Lines 2639 to 2643 in d8f03cf

auto [_Match, _Mid1] =
_Equal_rev_pred(_Candidate, _First2, _First2 + _Count2, _Pred, _Proj1, _Proj2);
if (_Match) {
return {_STD move(_Candidate), _STD move(_Mid1)};
}

STL/stl/inc/xutility

Lines 5715 to 5720 in d8f03cf

for (; _Count1 >= _Count2; ++_First1, (void) --_Count1) {
auto [_Match, _Mid1] = _RANGES _Equal_rev_pred(_First1, _First2, _Last2, _Pred, _Proj1, _Proj2);
if (_Match) {
return {_STD move(_First1), _STD move(_Mid1)};
}
}

STL/stl/inc/ranges

Lines 3780 to 3784 in d8f03cf

auto [_Begin, _End] = _RANGES search(subrange{_Current, _Last}, _Parent->_Pattern);
if (_Begin != _Last && _RANGES empty(_Parent->_Pattern)) {
++_Begin;
++_End;
}

STL/stl/inc/ranges

Lines 3826 to 3830 in d8f03cf

auto [_Begin, _End] = _RANGES search(subrange{_It, _Last}, _Pattern);
if (_Begin != _Last && _RANGES empty(_Pattern)) {
++_Begin;
++_End;
}

stl/inc/algorithm Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_alg_find_end/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_alg_find_end/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_alg_search/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_views_split/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

It looked like this was ready for review, so I went ahead and pushed minor changes. Please meow if you were still working on this! 🐱

@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Oct 30, 2021
@CaseyCarter
Copy link
Member Author

Reviewer note: I just inadvertently pushed a commit into this branch, and then force-pushed it back to e88474f. Hopefully this didn't reset incremental review.

@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed, or if more work is required.

@StephanTLavavej StephanTLavavej merged commit e89128e into microsoft:main Nov 13, 2021
@StephanTLavavej
Copy link
Member

Thanks for fixing these bugs! 🔧 🐞 😻

@CaseyCarter CaseyCarter deleted the sort_bug branch November 14, 2021 19:51
@mnatsuhara
Copy link
Contributor

Did a post-commit review and all looks good! (Just for the record 📝)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<xutility>: Use of _Rewrap_subrange which is defined in <algorithm>
6 participants