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

Clamp isize values returned by signed_difference #11

Merged
merged 8 commits into from
Jul 2, 2022

Conversation

Dr-Emann
Copy link
Contributor

Since the difference of two usizes may not fit in an isize, clamp the
values to isize::MIN/MAX when the difference cannot be represented.

Also, be careful to avoid subtracting with overflow when computing the
diff

I couldn't think of a more succinct way to perform a clamping difference,
I'd be happy if there's a way to do it cleaner.

Since the difference of two usizes may not fit in an isize, clamp the
values to isize::MIN/MAX when the difference cannot be represented.

Also, be careful to avoid subtracting with overflow when computing the
diff
@codecov
Copy link

codecov bot commented Jun 27, 2022

Codecov Report

Merging #11 (a6e84ee) into main (e1cc2ee) will increase coverage by 0.9%.
The diff coverage is 100.0%.

Impacted Files Coverage Δ
src/lib.rs 96.1% <100.0%> (+1.1%) ⬆️

@jonhoo
Copy link
Owner

jonhoo commented Jun 28, 2022

I think you can do:

if a > b {
  isize::try_from(a - b).unwrap_or(isize::MAX)
} else {
  isize::try_from(b - a).map(isize::saturaing_neg).unwrap_or(isize::MIN)
}

It would be a lot nicer with rust-lang/rust#87840, but for now I think you need ^.

@Dr-Emann
Copy link
Contributor Author

Ah, yeah. I think even map(|i| -i) would work, the value will always be positive, so there's no need for saturating_neg, which only acts differently for isize::MIN

Make other code common

Co-authored-by: Jon Gjengset <[email protected]>
Copy link
Owner

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

Thanks!

@jonhoo jonhoo merged commit d792a5d into jonhoo:main Jul 2, 2022
@jonhoo
Copy link
Owner

jonhoo commented Jul 2, 2022

Released as 0.1.9 🎉

jonhoo pushed a commit that referenced this pull request Feb 17, 2024
Put 1.70 in there (for instance if you want to pin against OnceLock stabilizing) and it will actually test 1.7 as it appears github auto converts this to a float?

Putting in quotes seems to do the right thing here
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.

2 participants