num-traits
num-traits copied to clipboard
Add `round_ties_even` to `Float/Core` and `Real`
Objective
Since Rust 1.77, round_ties_even was added to f32 and f64 in the standard library. This function is used within WGPU, and as a part of a push for no_std support, this function will need to be shimmed. It could be shimmed within Naga, but this functionality may have value to other users of num-traits.
Solution
- Added
round_ties_eventoFloatCore,Float, andRealwith a default implementation (not a breaking change) - Added a
has_round_ties_evenconfiguration check to allow detectinground_ties_evenprovided by the standard library in Rust 1.77 and above (not a breaking change) - Added a test ensuring the fallback implementation of
round_ties_evenmatches the implementation instdfor thef32type (assuming it is equally valid forf64)
Notes
- ~~The
msrv_1_77feature increases MSRV to 1.77, which is a typical arrangement (e.g.,serdefeatures will typically elevate MSRV). If/whennum-traitsadvances past MSRV 1.77, this feature can become a no-op without any breaking changes.~~ Resolved usingbuild.rsandautocfg. - This is my first contribution to
num-traits, please let me know if there's anything I can do to help!
- The
msrv_1_77feature increases MSRV to 1.77, which is a typical arrangement (e.g.,serdefeatures will typically elevate MSRV). If/whennum-traitsadvances past MSRV 1.77, this feature can become a no-op without any breaking changes.
We have our own precedent for this kind of thing -- please use autocfg in the build script instead of a cargo feature, similar to the total_cmp test that's already there.
We should also add this method to Float and Real, as these traits are related but not actually connected together.
We have our own precedent for this kind of thing -- please use
autocfgin the build script instead of a cargo feature, similar to thetotal_cmptest that's already there.
Didn't notice that! I've added a similar test to check for round_ties_even and removed the new feature.
We should also add this method to
FloatandReal, as these traits are related but not actually connected together.
Makes sense! I've added it to all 3. Unfortunately they each need the default implementation provided to ensure this PR isn't a breaking change, which does triplicate the code.
Unfortunately they each need the default implementation provided to ensure this PR isn't a breaking change, which does triplicate the code.
Hmm, and that's not easily solved with a helper function, since the necessary implementation methods are also coming from each separate trait. Maybe a macro would be ok?
For doc purposes, can you move these directly after each fn round? The doc sidebar is already sorted by name, but the main page displays methods in declaration order.
Finally, I'd prefer this to see this PR rebased rather than having a merge commit in the middle.
Maybe a macro would be ok?
Tried and seems to work well! I've placed it in macros.rs with a little documentation explaining why it exists.
For doc purposes, can you move these directly after each
fn round? The doc sidebar is already sorted by name, but the main page displays methods in declaration order.
Done!
Finally, I'd prefer this to see this PR rebased rather than having a merge commit in the middle.
Apologies, I've switched to a rebase (and squashed as most of these commits were minor amendments anyway).