Maintenance #460
Maintenance #460
Conversation
| @@ -26,7 +26,7 @@ mod diesel_mysql { | |||
| // internal types. | |||
| let bytes = numeric.ok_or("Invalid decimal")?; | |||
| let s = std::str::from_utf8(bytes)?; | |||
| Decimal::from_str(&s).map_err(|e| e.into()) | |||
| Decimal::from_str(s).map_err(|e| e.into()) | |||
| @@ -460,7 +460,7 @@ mod diesel_postgres { | |||
| } | |||
|
|
|||
| #[cfg(any(feature = "db-postgres", feature = "db-tokio-postgres"))] | |||
| mod postgres { | |||
| mod _postgres { | |||
There was a problem hiding this comment.
Clippy stuff. Feel free to choose another name if desired
There was a problem hiding this comment.
I had this in another project too. for now this should be fine since it's a private mod.
| @@ -94,7 +94,7 @@ pub mod float { | |||
| /// | |||
| /// #[derive(Serialize, Deserialize)] | |||
| /// pub struct StringExample { | |||
| /// #[serde(with = "rust_decimal::serde::string")] | |||
| /// #[serde(with = "rust_decimal::serde::str")] | |||
There was a problem hiding this comment.
CI is not currently testing the serde-with-str feature
There was a problem hiding this comment.
Oh, interesting. This is the documentation tests in particular... I wonder if this would've been picked up in publish?
I may see if we can capture this. Less blind spots the better!
| ArrayString::<MAX_STR_BUFFER_SIZE>::try_from(format_args!("{}", value)) | ||
| .map_err(S::Error::custom)? | ||
| .serialize(serializer) |
There was a problem hiding this comment.
There is no need to perform a heap allocation.
to_string comes from Display, which is the same as format_args!("{}", value).
|
|
||
| // `u64 as u32` are safe because of widening and 32bits shifts | ||
| #[inline] | ||
| pub(crate) fn manage_add_by_internal<const N: usize>(initial_carry: u32, value: &mut [u32; N]) -> u32 { |
There was a problem hiding this comment.
Used by add_one_internal and add_by_internal_flattened as both are almost the same. Other similar functions could also leverage this common entry point to remove duplication.
There was a problem hiding this comment.
Yes, I think after we optimized from_str_radix_n these functions could be removed.
| @@ -9,7 +9,7 @@ pub const UNSIGN_MASK: u32 = 0x4FFF_FFFF; | |||
| // contain a value between 0 and 28 inclusive. | |||
| pub const SCALE_MASK: u32 = 0x00FF_0000; | |||
| pub const U8_MASK: u32 = 0x0000_00FF; | |||
| pub const U32_MASK: u64 = 0xFFFF_FFFF; | |||
| pub const U32_MASK: u64 = u32::MAX as _; | |||
| // Scaling further isn't possible since we got an overflow | ||
| // In this case we need to reduce the accuracy of the "side to keep" | ||
|
|
||
| // Now do the necessary rounding | ||
| let mut remainder = 0; | ||
| while diff > 0 { | ||
| while let Some(diff_minus_one) = diff.checked_sub(1) { |
There was a problem hiding this comment.
The same branch condition of diff > 0. Removes one arithmetic operation and assembly output shouldn't change
| @@ -15,42 +15,46 @@ pub(crate) fn rescale_internal(value: &mut [u32; 3], value_scale: &mut u32, new_ | |||
| } | |||
|
|
|||
| if *value_scale > new_scale { | |||
| let mut diff = *value_scale - new_scale; | |||
| let mut diff = value_scale.wrapping_sub(new_scale); | |||
There was a problem hiding this comment.
wrapping_sub is explicitly telling the intended arithmetic operation behavior, i.e., it is self documented. This will help external users to have a better glimpse of what is going on
| @@ -209,7 +216,7 @@ pub(crate) fn mul_part(left: u32, right: u32, high: u32) -> (u32, u32) { | |||
| } | |||
|
|
|||
| // Returns remainder | |||
| pub(crate) fn div_by_u32(bits: &mut [u32], divisor: u32) -> u32 { | |||
| pub(crate) fn div_by_u32<const N: usize>(bits: &mut [u32; N], divisor: u32) -> u32 { | |||
| if divisor == 0 { | |||
| // Divide by zero | |||
| panic!("Internal error: divide by zero"); | |||
There was a problem hiding this comment.
Should probably signal an overflow instead of aborting the thread
| pub(crate) fn is_all_zero<const N: usize>(bits: &[u32; N]) -> bool { | ||
| bits.iter().all(|b| *b == 0) | ||
| } |
There was a problem hiding this comment.
I think this will help the compiler unroll the loop. Not sure if it is already optimizing but such addition doesn't hurt.
The same reasoning applies to the other <const N: usize> functions
There was a problem hiding this comment.
Since this function is inline, it actually was unrolled (I checked on master).
|
Thank you for doing this. Reducing CI warnings is a good thing and will make things easier going forward! |
No description provided.