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 |
| //! | ||
| //! // Compute `MODULUS` shifted right by 1 at compile time | ||
| //! pub const MODULUS_SHR1: U256 = MODULUS.shr(1).0; | ||
| //! pub const MODULUS_SHR1: U256 = MODULUS.shr(1); |
There was a problem hiding this comment.
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
|
| /// Computes `self << shift`. | ||
| /// | ||
| /// Panics if `shift >= Self::BITS`. | ||
| pub const fn shl(&self, shift: u32) -> Self { |
There was a problem hiding this comment.
Do we even need these methods? Primitive types don't have them.
There was a problem hiding this comment.
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
ChoiceorConstChoice) 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 fncontexts (and can now panic at compile time on overflow).