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

RFC: Saturating and checking integer wrapper types #1534

Closed
wants to merge 5 commits into from

Conversation

m4rw3r
Copy link

@m4rw3r m4rw3r commented Mar 7, 2016

Implement two wrapper-types in std::num which provide two different types of behavior on overflow: saturating arithmetic and always checked arithmetic with the latter signalling a thread panic on overflow.

Rendered

@SoniEx2
Copy link

SoniEx2 commented Mar 7, 2016

Doesn't borrow ideas from #1530 ?

@m4rw3r
Copy link
Author

m4rw3r commented Mar 7, 2016

@SoniEx2 This is just the wrapper-types themselves, not how they are implemented (eg. the proposed Wrapping* traits from #1530 ). I try to avoid the issue about specific operators since that has been brought up before and would probably become the main point people would bikeshed about if it was included in the RFC.

As for traits to identify if a type is wrapping or not, that could be put in the num crate since it has other traits which allow you to be generic over different integer types.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Mar 7, 2016

* `/` and `%` cannot overflow on unsigned integers. For signed integers `MIN / -1` and `MIN % -1`
can overflow since signed types follow two's complement: `-1 * MIN = MAX + 1` and `MIN` is
always even. The proposed results are `MIN / -1 = MAX` and `MIN % -1 = MAX`.
Copy link
Contributor

Choose a reason for hiding this comment

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

MIN % -1 is well defined as 0. IIRC the only reason it's not this way in Rust is that it requires an additional branch for every op, but that's required here anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

(#1276)

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Changing to 0 for MIN % -1.

@comex
Copy link

comex commented Mar 15, 2016

👍 on changing the name to avoid confusion with checked_*.

@Zoxc
Copy link

Zoxc commented Apr 3, 2016

I want a Checked type which gives None if you get an overflow. This would correspond to the checked_* methods and probably give more efficient code than the Checked type proposed here anyway.

@brendanzab
Copy link
Member

I would prefer to get an Option back too. Might I suggest making a proof-of-concept crate?

@aturon aturon self-assigned this May 9, 2016
@alexcrichton
Copy link
Member

alexcrichton commented Aug 23, 2016

🔔 This RFC is now entering its final comment period 🔔

The @rust-lang/libs team is inclined to close this RFC. It's unfortunately been inactive for quite some time now and it seems that the desire here may not be enough to move into the standard library yet. It would likely be worth prototyping this on crates.io and seeing how it plays out there before moving.

@rust-lang/libs if you could check off your name below or write a comment below to the contrary:

@alexcrichton alexcrichton added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Aug 23, 2016
@alexcrichton
Copy link
Member

Ok, the libs team has decided to close, so closing. Thanks regardless though for the RFC @m4rw3r!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants