-
Notifications
You must be signed in to change notification settings - Fork 57
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
Align with core
/std
on overflowing_sh*
#430
Conversation
Changes all shift functions which return an overflow flag (as a `Choice` or `ConstChoice`) to use the `overflowing_sh*` name prefix, which aligns with similar APIs in `core`/`std`. In their place, adds new `Uint::{shl, shr}` functions which provide the trait-like behavior (i.e. panic on overflow) but work in `const fn` contexts (and can now panic at compile time on overflow).
Note: the name changes were done using automated refactoring with IntelliJ, so I still haven't carefully gone through and determined what locations which are using |
//! pub const MODULUS_SHR1: U256 = MODULUS.shr(1).0; | ||
//! pub const MODULUS_SHR1: U256 = MODULUS.shr(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like an ergonomics improvement
There are a few API gaps which should probably be filled before merging if we're happy this, like every
|
@@ -5,9 +5,22 @@ use core::ops::{Shl, ShlAssign}; | |||
|
|||
impl<const LIMBS: usize> Uint<LIMBS> { | |||
/// Computes `self << shift`. | |||
/// | |||
/// Panics if `shift >= Self::BITS`. | |||
pub const fn shl(&self, shift: u32) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we even need these methods? Primitive types don't have them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's what makes this possible, i.e. using them in a const fn
context (and even better, doing compile-time overflow checking).
Primitive types are a bit magical in that arithmetic operators like <<
and >>
work in const contexts even though for all other types they'd thunk through the Shl
/Shr
traits which won't be able to do that until const impl
is available.
LGTM, except that I think |
Will fix. Thanks! |
Changes all shift functions which return an overflow flag (as a
Choice
orConstChoice
) to use theoverflowing_sh*
name prefix, which aligns with similar APIs incore
/std
.In their place, adds new
Uint::{shl, shr}
functions which provide the trait-like behavior (i.e. panic on overflow) but work inconst fn
contexts (and can now panic at compile time on overflow).