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

Add missed increment of _Current in the early return path #1561

Merged
merged 3 commits into from
Jan 15, 2021

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Jan 7, 2021

Fixes #1560

When avoiding an superfluous comparison we missed to add the necessary increment to exclude the first element of the last group.

@miscco miscco requested a review from a team as a code owner January 7, 2021 21:04
@StephanTLavavej StephanTLavavej added bug Something isn't working ranges C++20/23 ranges labels Jan 8, 2021
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this correctness bug! I'll push a small comment change.

tests/std/tests/P0896R4_ranges_alg_unique/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_alg_unique/test.cpp Outdated Show resolved Hide resolved

auto result = unique(wrapped_input.begin(), wrapped_input.end(), countedEq, get_second);
STATIC_ASSERT(same_as<decltype(result), subrange<iterator_t<ReadWrite>>>);
assert(result.empty());
Copy link
Member

Choose a reason for hiding this comment

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

Observation, no change requested: This could be more strict, by verifying that the empty subrange is at the end. However, it's unlikely that we'll damage this (and this PR certainly isn't), so it's just something to keep in mind for future tests.

@miscco
Copy link
Contributor Author

miscco commented Jan 9, 2021

Thanks a lot @StephanTLavavej

@StephanTLavavej StephanTLavavej self-assigned this Jan 12, 2021
@StephanTLavavej StephanTLavavej merged commit 40b39d2 into microsoft:master Jan 15, 2021
@StephanTLavavej
Copy link
Member

Thanks for this uniquely awesome bugfix! 😹 🪲

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.

<algorithm>: std::ranges::unique returns the wrong value for already unique input
3 participants