Implement remaining rounding to increment methods

[jedel1043]

Depends on #4270

With trunc_to_increment and expand_to_increment implemented, this finishes implementing the rest of the rounding methods.

[sffc]

Seems good but I think it's worth some additional exploration on how to do more code sharing. I left an idea for how that might be able to work.

[sffc]

Technically you need to look at two digits because this is periodic every 20 not every 10:

• 1 => down to 0
• 3 => up to 4
• 5 => down to 4
• 7 => up to 8
• 9 => down to 8
• 11 => up to 12
• 13 => down to 12
• 15 => up to 16
• 17 => down to 16
• 19 => up to 20
• 21 => down to 20
• ...

[sffc]

Suggestion: it may be helpful to add the following private function on FixedDecimal and use it wherever you need it:

enum HalfAtMagnitude {
    LessThanHalf,
    ExactlyHalf,
    GreaterThanHalf,
}

fn half_at_magnitude(&self, magnitude: i16) -> HalfAtMagnitude {
    match (self.nonzero_magnitude_end().cmp(magnitude)), self.digit_at(magnitude).cmp(5)) {
        (Ordering::Less, Ordering::Less) => LessThanHalf, // 0.1x
        (Ordering::Less, _) => GreaterThanHalf, // 0.5x, 0.9x
        (Ordering::Equal, Ordering::Less) => LessThanHalf, // 0.1
        (Ordering::Equal, Ordering::Equal) => ExactlyHalf, // 0.5
        (Ordering::Equal, Ordering::Greater) => GreaterThanHalf, // 0.9
        (Ordering::Greater, _) => debug_assert!(false, "don't call this function in this case"),
    }
}

You can use this function as part of checking for boundaries at 0.5, 2.5, 7.5, 12.5, 37.5, 62.5, 87.5

[jedel1043]

Had to slightly modify the method since it was more useful to use the digits at the right of magnitude, and opted for using Ordering because I think it carries the same semantics as HalfAtMagnitude.

[dpulls]

🎉 All dependencies have been resolved !

[sffc]

Thanks! This looks great! Two requests:

1. I think there may be extra diffs that would be resolved by merging main into your branch
2. It would be nice to see a different type of test that is more data-driven, more like the ECMAScript rounding table test but for increments.

[sffc]

Looks good! Thank you!

I have a suggestion on the new ecma402 table test but you can do it later if you prefer.

@robertbastian How are we doing on the 1.4 release? Should we land this PR? It completes the set of experimental rounding increment functions.

[sffc]

Nit: Make sure the test cases on each row are unique. For example, currently "ceil" and "half_ceil" have the same expectations, so the test does not distinguish between those two cases. You might need to change the input numbers to make this work.

[jedel1043]

I have a suggestion on the new ecma402 table test but you can do it later if you prefer.

Yeah, I reused the current test to ensure this is at least considered for 1.4, just to not deliver an incomplete, even if experimental, feature. I'll modify the test to be more exhaustive before stabilizing the new API.

[robertbastian]

.

[robertbastian]

Reapproving based on Shane's approval, happy to get this in 1.4 as it's all experimental.

[robertbastian]

Please don't force push once a review has started, it's not possible to tell if anything changed.

[jedel1043]

Please don't force push once a review has started, it's not possible to tell if anything changed.

Oh, I rebased because I assumed this cannot be merged without being based off of main. Should I use merge commits in that case?

[robertbastian]

I don't think it had any conflicts, in which case it could have been merged (we squash-merge).

If there are conflicts please use merge commits.

[jedel1043]

I don't think it had any conflicts, in which case it could have been merged (we squash-merge).

Good to know! I have rebased previous PRs a couple of times thinking it hadn't been merged because it wasn't based off of main 😅

[robertbastian]

This one hasn't been merged since my approval because I was waiting for CI to finish. The force push retriggered it 😉