Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FloatConst members be const #213

Open
jturner314 opened this issue Apr 8, 2021 · 6 comments
Open

Make FloatConst members be const #213

jturner314 opened this issue Apr 8, 2021 · 6 comments

Comments

@jturner314
Copy link
Contributor

I'd like to propose the following breaking change (to be added to #47): make the members of FloatConst be either associated consts (my preference) or const fns, so that they can be used in constants.

@cuviper
Copy link
Member

cuviper commented Apr 8, 2021

How would that be used? Last I checked, you couldn't have generic constraints (like T: FloatConst) in constant contexts at all. If you're using a specific type, it can (and might already) have those associated consts itself.

@jturner314
Copy link
Contributor Author

jturner314 commented Apr 8, 2021

Unfortunately, f32 and f64 have very few inherent associated consts; most constants (including essential mathematical constants like E and PI) are in std::f32::consts/std::f64::consts. So, FloatConst is useful as an extension trait so that constants can be accessed through the names of the types, rather than through the separate std::f32::consts/std::f64::consts modules. For using FloatConst as an extension trait, it would be nice for the members of FloatConst to be const, for a couple of reasons:

  1. For code which uses a type alias to switch between f32 and f64, it's nice to have associated constants inherent to the type or as part of an extension trait, so that they can be easily accessed using the name of the type alias in constants:

    // Located in a common module/crate in the workspace.
    pub type Fxx = f64;
    
    // Located in a dependent module/crate.
    pub const ACTIONS: &[Action] = &[
        Action {
            speed: 20.,
            ang_vel: -Fxx::TAU / 15.,
        },
        Action {
            speed: 30.,
            ang_vel: 0.,
        },
        Action {
            speed: 20.,
            ang_vel: Fxx::TAU / 15.,
        },
    ];
  2. When writing a module which uses both f32 and f64 explicitly for different purposes (i.e. some variables need to be higher precision, but others can be lower precision), it is more concise to write use num_traits::FloatConst followed by f32::PI() and f64::PI() (or f32::PI and f64::PI if the members of FloatConst were made into associated constants), instead of use std::{f32, f64} followed by f32::consts::PI and f64::consts::PI. If the members of FloatConst were const, this would work in const contexts too.

Both of these could be solved by moving the consts in std::f32::consts/std::f64::consts to be associated consts on f32/f64, but that seems unlikely to happen, so a trait like FloatConst is a nice alternative.

The only disadvantage I see to making the members of FloatConst be const is that it would prevent FloatConst from being implemented for non-Copy types types which allocate.

Edit: Long-term, it seems likely that Rust will support generic parameters on const fns in the future (RFC 2632), which would make FloatConst useful in generic const fns if its members were const.

@cuviper
Copy link
Member

cuviper commented Apr 8, 2021

Unfortunately, f32 and f64 have very few inherent associated consts; most constants (including essential mathematical constants like E and PI) are in std::f32::consts/std::f64::consts.

Ah, yes, I forgot about this situation.

The only disadvantage I see to making the members of FloatConst be const is that it would prevent FloatConst from being implemented for non-Copy types.

I don't think const has to be Copy -- e.g. Vec::new is fine -- but it certainly is restrictive in other ways. For instance, a hypothetical BigFloat backed by Vec couldn't create most constants until const-allocation is possible. That said, the other trait Float does require Copy right now, for better or worse.

@jturner314
Copy link
Contributor Author

Right. I'd think that in generic code with a FloatConst bound, there'd almost always be a Float bound on the type, too. So, since Float already requires Copy, there doesn't seem to be much disadvantage to making FloatConst implementable only by non-allocating types.

Fwiw, I'd also suggest converting the "constant" methods of Float (nan, infinity, neg_infinity, neg_zero, min_value, min_positive_value, max_value, and epsilon) into associated constants, too. Since Float already requires Copy, there's effectively no disadvantage to doing so.

@jturner314
Copy link
Contributor Author

Fwiw, I found the RFC which moved most numeric constants to be associated constants on the numeric types: RFC 2700. The Alternatives section says the following regarding std::f32::consts and std::f64::consts:

Unlike the twelve integral modules, the two floating-point modules would not themselves be entirely deprecated by the changes proposed here. This is because the std::f32 and std::f64 modules each contain a consts submodule, in which reside constants of a more mathematical bent (the sort of things other languages might put in a std::math module). It is the author's opinion that special treatment for such "math-oriented constants" (as opposed to the "machine-oriented constants" addressed by this RFC) is not particularly precedented; e.g. this separation is not consistent with the existing set of associated functions implemented on f32 and f64, which consist of a mix of both functions concerned with mathematical operations (e.g. f32::atanh) and functions concerned with machine representation (e.g. f32::is_sign_negative). However, although earlier versions of this RFC proposed deprecating std::{f32, f64}::consts (and thereby std::{f32, f64} as well), the current version does not do so, as this was met with mild resistance (and, in any case, the greatest gains from this RFC will be its impact on the integral modules). Ultimately, there is no reason that such a change could not be left to a future RFC if desired. However, one alternative design would be to turn all the constants in {f32, f64} into associated consts as well, which would leave no more modules in the standard library that shadow primitive types. A different alternative would be to restrict this RFC only to the integral modules, leaving f32 and f64 for a future RFC, since the integral modules are the most important aspect of this RFC and it would be a shame for them to get bogged down by the unrelated concerns of the floating-point modules.

So, with another RFC, maybe we could achieve consensus to move everything from std::f32::consts and std::f64::consts to be associated constants on f32/f64. That would eliminate the need for FloatConst as an extension trait. Then, FloatConst would be useful only for generic code, and until Rust supports type parameters on const fns, there wouldn't be much need to change its members to be const.

@cuviper
Copy link
Member

cuviper commented Apr 9, 2021

The libs-team decision on that is here: rust-lang/rust#68490 (comment)

Should constants from std::{f32, f64}::consts also be made associated consts?

No. At least not as part of this tracking issue/RFC. That could be a separate proposal, with a separate discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants