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

Fix ranges::find_end return for bidi-common case #2270

Merged
merged 2 commits into from Nov 13, 2021

Conversation

CaseyCarter
Copy link
Member

...and add full test coverage.

Fixes #2268

@CaseyCarter CaseyCarter added the bug Something isn't working label Oct 12, 2021
@CaseyCarter CaseyCarter requested a review from a team as a code owner October 12, 2021 00:48
@CaseyCarter CaseyCarter added this to Initial Review in Code Reviews via automation Oct 12, 2021
Copy link
Contributor

@miscco miscco left a comment

Choose a reason for hiding this comment

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

Obligatory "everything that is not tested is broken"

Fwd2 needle{good_needle};
const same_as<subrange<iterator_t<Fwd1>>> auto result =
find_end(begin(haystack), end(haystack), begin(needle), end(needle), pred, get_first);
STATIC_ASSERT(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, is there a reason to shout STATIC_ASSERT, at least in ranges code we should be able to be a bit less shouty

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes there is: I refuse to manually map from line number to failure condition when I can make MSVC do it for me by capitalizing static_assert.

@StephanTLavavej StephanTLavavej self-assigned this Oct 13, 2021
@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Oct 13, 2021
@StephanTLavavej StephanTLavavej removed their assignment Oct 15, 2021
@StephanTLavavej StephanTLavavej moved this from Initial Review to Final Review in Code Reviews Oct 15, 2021
@StephanTLavavej StephanTLavavej self-assigned this Nov 12, 2021
@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.

@mnatsuhara mnatsuhara moved this from Final Review to Ready To Merge in Code Reviews Nov 12, 2021
@mnatsuhara mnatsuhara removed their assignment Nov 12, 2021
@StephanTLavavej StephanTLavavej merged commit d6abb23 into microsoft:main Nov 13, 2021
Code Reviews automation moved this from Ready To Merge to Done Nov 13, 2021
@StephanTLavavej
Copy link
Member

Thanks for finding the end of this bug! 🐞 🛑 😹

@CaseyCarter CaseyCarter deleted the find_end_fix branch November 14, 2021 19:53
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
No open projects
Development

Successfully merging this pull request may close these issues.

<algorithm>: The bidirectional common range version of ranges::find_end returns the wrong value
4 participants