Add checked shifts #21
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add traits `CheckedShl` and `CheckedShr` that correspond to the standard
library's `checked_shl` and `checked_shr` functions. Implement the trait
on all primitive integer types by default, akin to what the standard
library does.
The stdlib is somewhat inconsistent when it comes to the type of the
shift amount. The `checked_*` functions have a `u32` shift amount, but
the `std::ops::{Shl,Shr}` traits are generic over the shift amount. Also
the stdlib implements these traits for all primitive integer types as
right-hand sides. Our implementation mimics this behaviour.
cuviper
requested changes
Jan 13, 2018
src/int.rs
Outdated
| @@ -21,6 +21,8 @@ pub trait PrimInt | |||
| + CheckedSub<Output=Self> | |||
| + CheckedMul<Output=Self> | |||
| + CheckedDiv<Output=Self> | |||
| + CheckedShl<usize, Output=Self> | |||
| + CheckedShr<usize, Output=Self> | |||
There was a problem hiding this comment.
Unfortunately, this is a breaking change. We don't control all implementations of PrimInt, so adding new constraints would break others, however theoretical such others may be.
src/ops/checked.rs
Outdated
| @@ -90,3 +90,54 @@ checked_impl!(CheckedDiv, checked_div, i32); | |||
| checked_impl!(CheckedDiv, checked_div, i64); | |||
| checked_impl!(CheckedDiv, checked_div, isize); | |||
|
|
|||
| /// Performs a left shift that returns `None` on overflow. | |||
| pub trait CheckedShl<RHS>: Sized + Shl<RHS, Output=Self> { | |||
There was a problem hiding this comment.
I think for now, I'd rather keep these consistent with the other Checked traits that are not parameterized, especially since the standard library methods are only for u32. That also moots your comment below about needing checked conversions to u32.
(We could decide to expand it later using a default, like CheckedShl<RHS = u32>.)
Make the checked left and right shifts take a `u32` as right-hand side, which is more consistent with the other checked operations. Also drop `CheckedShl` and `CheckedShr` from the `PrimInt` trait, to not break existing code. Add doctests for the two traits.
|
How about now? |
cuviper
approved these changes
Jan 18, 2018
|
Looks good, thanks! bors r+ |
bors bot
added a commit
that referenced
this pull request
Jan 18, 2018
17: Add AsPrimitive trait for generic casting with `as` r=cuviper a=Enet4 This is my personal attempt at #7. It is fairly similar to what can be found in `asprim`, although implemented from scratch. Please let me know of what you think. Could it use more tests? Should I also leave a safety notice that some conversions with `as` are currently UB (rust-lang/rust#10184)? 21: Add checked shifts r=cuviper a=fabianschuiki Add traits `CheckedShl` and `CheckedShr` that correspond to the standard library's `checked_shl` and `checked_shr` functions. Implement the trait on all primitive integer types by default, akin to what the standard library does. The stdlib is somewhat inconsistent when it comes to the type of the shift amount. The `checked_*` functions have a `u32` shift amount, but the `std::ops::{Shl,Shr}` traits are generic over the shift amount. Also the stdlib implements these traits for all primitive integer types as right-hand sides. Our implementation mimics this behaviour.
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add traits
CheckedShlandCheckedShrthat correspond to the standardlibrary's
checked_shlandchecked_shrfunctions. Implement the traiton all primitive integer types by default, akin to what the standard
library does.
The stdlib is somewhat inconsistent when it comes to the type of the
shift amount. The
checked_*functions have au32shift amount, butthe
std::ops::{Shl,Shr}traits are generic over the shift amount. Alsothe stdlib implements these traits for all primitive integer types as
right-hand sides. Our implementation mimics this behaviour.