Simplify FixedDecimal's rounding APIs #5028
Conversation
jedel1043
force-pushed
the
fd/new-rounding-ops
branch
from
739fee1 to
25e8578
Compare
| self.0.half_expand_to_increment(position, increment.into()) | ||
| #[diplomat::rust_link(fixed_decimal::FixedDecimal::round, FnInStruct)] | ||
| #[diplomat::rust_link(fixed_decimal::FixedDecimal::rounded, FnInStruct, hidden)] | ||
| pub fn round(&mut self, position: i16) { |
There was a problem hiding this comment.
nit: documentation on what round means here
There was a problem hiding this comment.
@eggrobin wdyt about this being the default round function? Alternatively we can call this round_half_even or similar.
There was a problem hiding this comment.
wdyt about this being the default
roundfunction?
Sounds reasonable to me (so long as it is documented of course, which it now is).
| self.0.half_expand_to_increment(position, increment.into()) | ||
| #[diplomat::rust_link(fixed_decimal::FixedDecimal::round, FnInStruct)] | ||
| #[diplomat::rust_link(fixed_decimal::FixedDecimal::rounded, FnInStruct, hidden)] | ||
| pub fn round(&mut self, position: i16) { |
There was a problem hiding this comment.
@eggrobin wdyt about this being the default round function? Alternatively we can call this round_half_even or similar.
|
2024-06-20 notes:
Conclusion: Land a LGTM: @sffc @jedel1043 @Manishearth @robertbastian Would like affirmation from @eggrobin |
| match mode { | ||
| RoundingMode::Ceil => self.ceil_to_increment_internal(position, increment), | ||
| RoundingMode::Expand => self.expand_to_increment_internal(position, increment), | ||
| RoundingMode::Floor => self.floor_to_increment_internal(position, increment), | ||
| RoundingMode::Trunc => self.trunc_to_increment_internal(position, increment), | ||
| RoundingMode::HalfCeil => self.half_ceil_to_increment_internal(position, increment), | ||
| RoundingMode::HalfExpand => self.half_expand_to_increment_internal(position, increment), | ||
| RoundingMode::HalfFloor => self.half_floor_to_increment_internal(position, increment), | ||
| RoundingMode::HalfTrunc => self.half_trunc_to_increment_internal(position, increment), | ||
| RoundingMode::HalfEven => self.half_even_to_increment_internal(position, increment), | ||
| } |
There was a problem hiding this comment.
Thought: I hope the compiler figures out that it should inline round_with_mode_and_increment and all these helper functions except the 3 that contain meaningful code (expand_to_increment_internal, floor_to_increment_internal, and half_even_to_increment_internal).
There was a problem hiding this comment.
I checked the assembly and it inlines everything correctly. half_even_to_increment_internal is also inlined but I think is mostly because it's only used in that method.
|
I'm going to merge this and we can revisit the method name if @eggrobin has concerns about it. Thanks @jedel1043! |
Related to #2902
Mainly removes all the
_to_incrementmethods, which are superseeded by theround_with_mode_and_incrementmethod.Additionally, adds a
roundfunction that does half to even rounding.