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 range_step/range_step_inclusive #9199

Closed
wants to merge 4 commits into from
Closed

add range_step/range_step_inclusive #9199

wants to merge 4 commits into from

Conversation

thestinger
Copy link
Contributor

My focus was on getting these to be correct in all cases by handling overflow properly. I'll clean them up and work on the performance later.

@flaper87
Copy link
Contributor

FWIW and double check r+ 😄

bors added a commit that referenced this pull request Sep 15, 2013
My focus was on getting these to be correct in all cases by handling overflow properly. I'll clean them up and work on the performance later.
@bors bors closed this Sep 15, 2013
@bill-myers
Copy link
Contributor

I think this line is wrong: "if (self.rev && self.state > self.stop) || self.state < self.stop"

Shouldn't it be "if (self.rev && self.state > self.stop) || (!self.rev && self.state < self.stop)" or something equivalent instead?

The tests don't catch it because the stop value is hit exactly in the only reverse test.

@thestinger
Copy link
Contributor Author

@bill-myers: it was fixed in my follow-up commits, this is no longer the code used in the codebase

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 28, 2022
…rcho

unused_self: respect avoid-breaking-exported-api

```
changelog: [`unused_self`]: Now respects the `avoid-breaking-exported-api` config option
```

Fixes rust-lang#9195.

I mostly copied the implementation from `unnecessary_wraps`, since I don't have much understanding of rustc internals.
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.

4 participants