-
Notifications
You must be signed in to change notification settings - Fork 33
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
Signed type impls of Into<UInt> are now fallible #570
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #570 +/- ##
==========================================
- Coverage 83.30% 83.23% -0.08%
==========================================
Files 104 104
Lines 19576 19611 +35
Branches 19576 19611 +35
==========================================
+ Hits 16308 16323 +15
- Misses 1730 1750 +20
Partials 1538 1538
☔ View full report in Codecov by Sentry. |
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.
🗺️ PR tour
@@ -197,7 +197,7 @@ impl From<DecodedInt> for Coefficient { | |||
} = int; | |||
use types::Sign::{Negative, Positive}; | |||
let sign = if is_negative { Negative } else { Positive }; | |||
Coefficient::new(sign, value) | |||
Coefficient::new(sign, value.unsigned_abs()) |
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.
🗺️ All signed int primitives have an unsigned_abs()
method that returns their absolute value as an unsigned integer of the closest-sized type (i8
->u8
, i16
->u16
, etc). Now the Int
type has an unsigned_abs()
method that returns a UInt
.
@@ -690,7 +690,7 @@ mod tests { | |||
let mut buffer = BinaryBuffer::new(&[0b1000_0000]); | |||
let var_int = buffer.read_uint(buffer.remaining())?; | |||
assert_eq!(var_int.size_in_bytes(), 1); | |||
assert_eq!(var_int.value(), &UInt::from(128)); | |||
assert_eq!(var_int.value(), &UInt::from(128u64)); |
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.
🗺️ Signed integers can no longer be infallibly converted to a UInt
. In the absence of other type information, Rust treats integers as i32
s. This means that to call UInt::from(...)
, we need to mark our integers as an unsigned type.
/// | ||
/// Signed integers can infallibly implement `Into<Magnitude>` by using their | ||
/// absolute value. | ||
pub struct Magnitude(UInt); |
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.
🗺️ Previously, as described in #569, signed integers being converted to UInt
would use their absolute value. This wrapper type offers the same behavior, but is narrowly used in APIs where that wouldn't be surprising.
Fixes #569.
TryInto<UInt>
and fail if they are negative.Coefficient::new
now takesI: Into<Magnitude>
whereMagnitude
is a wrapper that signed integers infallibly convert to via absolute value.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.