core_float_math: Move functions to math folder
#141282
Open
+955
−930
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When these functions were added in rust-lang#138087 It made a relatively common pattern for emulating these functions using an extension trait (which internally uses `libm`) much more fragile. If `core::f32` happened to be imported by the user (to access a constant, say), then that import in the module namespace would take precedence over `f32` in the type namespace for resolving these functions, running headfirst into the stability attribute. We ran into this in Color - https://github.com/linebender/color - and chose to release the remedial 0.3.1 and 0.2.4, to allow downstream crates to build on `docs.rs`. As these methods are perma-unstable, moving them into a new module should not have any long-term concerns, and ensures that this breakage doesn't adversely impact anyone else.
rustbot
added
S-waiting-on-review
DJMcNab
commented
May 20, 2025
| @@ -1,3 +1,4 @@ | |||
| use core::f64; | |||
There was a problem hiding this comment.
Previously, these functions were actually testing the inherent methods on f64, as defined in std, rather than the functions from the f64 module.
Practically, that's the same thing, but the use of UFCS made that not clear.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When these functions were added in #138087 It made a relatively common pattern for emulating these functions using an extension trait (which internally uses
libm) much more fragile. Ifcore::f32happened to be imported by the user (to access a constant, say), then that import in the module namespace would take precedence over thef32in the type namespace for resolving these functions, running headfirst into the stability attribute.We ran into this in Color and chose to release the remedial 0.3.1 and 0.2.4, to allow downstream crates to build on
docs.rs.As these methods are perma-unstable, moving them into a new module should not have any long-term concerns, and ensures that this "breakage" doesn't adversely impact anyone else.
I believe that I've made the module unstable correctly. I presume that this does not require a test to make sure stable code can't depend on the module existing?
I've left the stability attribute on each function - happy to tweak this if a different pattern is more correct.
Tracking issue for
core_float_math: #137578.This PR is as requested in #138087.
r? @tgross35
Recommended reviewing with whitespace hidden.
(This is my first PR to
std/core/this repository, as far as I can remember)