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 post increment of counted iterator #1664

Merged

Conversation

YehezkelShB
Copy link
Contributor

When using conuted_iterator with an input iterator that has void-returning post increment operator, the post incrementing of counted_iterator doesn't decrement its counter.

This PR adds a test that demonstrates the issue and then adds the missing line to fix it.

@YehezkelShB
Copy link
Contributor Author

Working on a wider change in counted_iterator we want to propose (see P2406 for more details), we noticed this issue.
We hope this supports the claim that no one is using counted_iterator with input iterators right now (at least with post increment operator), which makes it easier to propose other, possibly-breaking, changes we consider for P2406 :)

@YehezkelShB
Copy link
Contributor Author

@ericniebler I'd appreciate your feedback here. This is just adding one line + a simple test to demonstrate the bug/fix

@ericniebler
Copy link
Owner

Apologies, running your tests now, but change looks good.

@ericniebler
Copy link
Owner

Why are tests still pending?

@YehezkelShB
Copy link
Contributor Author

Why are tests still pending?

Can't find any online and idle self-hosted runner in the current repository, account/organization that matches the required labels: 'ubuntu-16.04'

Seems like 16.04 isn't available there anymore? Tests running on 20.04 did run

@YehezkelShB
Copy link
Contributor Author

GitHub Actions : Ubuntu 16.04 LTS virtual environment will be removed on September 20, 2021

https://github.blog/changelog/2021-04-29-github-actions-ubuntu-16-04-lts-virtual-environment-will-be-removed-on-september-20-2021/

@YehezkelShB
Copy link
Contributor Author

YehezkelShB commented Nov 2, 2021

Fixing it with #1671

@YehezkelShB
Copy link
Contributor Author

@ericniebler Tests passed now. Do you want me to rebase this branch on latest master or the merge is OK?

@ericniebler ericniebler merged commit 3e1ff2a into ericniebler:master Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants