Deprecate stdlib modules dedicated to numeric constants and move those constants to associated consts

[bstrie]

Add the relevant associated constants to the numeric types in the standard library, and consider a timeline for the deprecation of the corresponding (and originally intended to be temporary) primitive numeric modules and associated functions.

Rendered

[SimonSapin]

Thanks @bstrie for extensively documenting the intent that we collectively had all along. I don’t expect this to be controversial, but this is what the FCP period is for:

@rfcbot fcp merge

[rfcbot]

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @Kimundi
• @SimonSapin
• @alexcrichton
• @dtolnay
• @sfackler
• @withoutboats

Concerns:

• importing resolved by Deprecate stdlib modules dedicated to numeric constants and move those constants to associated consts #2700 (comment)
• more discussion before FCP start resolved by Deprecate stdlib modules dedicated to numeric constants and move those constants to associated consts #2700 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[Ekleog]

Does it really make sense to have f64::PI as an associated const? While I totally agree for min, max and the like (that really define the type), things like PI, LN_2, LN_10, FRAC_2_SQRT_PI etc. are less obvious to me.

[Centril]

Some thoughts...

Also, I think it's premature to jump to PFCP so quickly when no discussion has been had at all.

[liigo]

Replace each item with an associated const value on the appropriate type

a good rfc requires an explicite list

[SimonSapin]

I think it's premature to jump to PFCP so quickly when no discussion has been had at all.

I took the links in the motivation section as showing there had been ample discussion elsewhere already. That said, let’s register this as a concern to give more time:

@rfcbot concern more discussion before FCP start

[bstrie]

Does it really make sense to have f64::PI as an associated const? While I totally agree for min, max and the like (that really define the type), things like PI, LN_2, LN_10, FRAC_2_SQRT_PI etc. are less obvious to me.

@Ekleog , I have added language to the section on alternatives further elaborating on the rationale here.

Updates to the text regarding other concerns are forthcoming, as my busy schedule playing Minecraft with my girlfriend permits.

[newpavlov]

While I support deprecation of module-level constants in favor of associated constants (also I think we should add std::time::UNIX_EPOCH to the list), I think we should first allow import of associated constants or otherwise it may be an ergonomic hit, e.g. use std::time::SystemTime::UNIX_EPOCH; will not work today.

[sfackler]

UNIX_EPOCH has already been moved: https://doc.rust-lang.org/std/time/struct.SystemTime.html#associatedconstant.UNIX_EPOCH

[newpavlov]

Yes, I know. My point is about marking std::time::UNIX_EPOCH as deprecated and allowing associated constants import before module-level constants deprecation.

[bstrie]

Though note that the inability to bring associated items into scope via use doesn't preclude aliasing them in other ways. If someone's using un-prefixed UNIX_EPOCH in their code, then that would require them to already have a line like:

use std::time::UNIX_EPOCH;

...which, if std::time::UNIX_EPOCH were deprecated in favor of the associated const std::time::SystemTime::UNIX_EPOCH, then the one and only change that would need to be made to any module with the aforementioned import would be to change the use line to:

use std::time::SystemTime;
const UNIX_EPOCH: SystemTime = SystemTime::UNIX_EPOCH;

Furthermore, because all primitive types are already in the prelude, doing the same transition for the numeric types as proposed in this RFC would be even easier, requiring only a line such as:

use std::i32::MAX;

...to become:

const MAX: i32 = i32::MAX;

Finally, whereas I can totally see someone wanting to use UNIX_EPOCH as an un-prefixed identifier, I think it is much less likely that someone would import constants like MAX or MIN to use un-prefixed, precisely because there are no less than twelve different maxes and mins in the stdlib, and the types are very useful for mental disambiguation (in addition, the types are extremely concise, unlike SystemTime).

So while it would be nice to be able to import associated consts, (and associated items in general; has anyone ever submitted an RFC for this?) I don't necessarily think it will be such a hindrance in this case if we chose not to wait until that were possible.

[Centril]

Finally, whereas I can totally see someone wanting to use UNIX_EPOCH as an un-prefixed identifier, I think it is much less likely that someone would import constants like MAX or MIN to use un-prefixed, precisely because there are no less than twelve different maxes and mins in the stdlib, and the types are very useful for mental disambiguation (in addition, the types are extremely concise, unlike SystemTime).

@bstrie Sure, it seems clear that u8::MAX is more understandable than MAX. For other constants the matter seems different. I would suggest cutting back to just MAX & MIN.

[bstrie]

@Centril, note that all the integral modules contain only MAX and MIN, so reducing the RFC's scope to those constants would be equivalent to the alternative discussed in the RFC where these changes would be made only to the integral modules and not for the float modules. (Although personally I find the visual difference between f32::PI and f64::PI to be just as important, to say nothing of the hypothetical f16::PI, f80::PI, and f128::PI that we may have someday).

I'll summarize this discussion and add it to the Drawbacks section.

[Centril]

(please use linebreaks or semantic linefeeds...)

This RFC argues for giving the float modules the same treatment as the integral modules, both since associated consts are "obviously the right thing" in this case [...]

Please make an effort beyond "obviously the right thing". It's often not obvious, even if you think it is.

[scottmcm]

Yes please! I always just type u32::MAX then get sad when the compiler reminds me it doesn't work...

[avl]

Also, f32::PI would be very nice! The chance that someone ever types that intending something other than the mathematical constant Pi seems infinitesimal. I think Pi and other mathematical constants are fundamental enough that they don't deserve being shuffled out of sight to a 'constants' namespace.

[avl]

Couldn't deprecation wait til next edition (2021?). But introduce the new names now?

[Centril]

@SimonSapin I think it's fair to say that the discussion has slowed down at this point. :)

[SimonSapin]

Indeed!

@rfcbot resolve more discussion before FCP start

However I think that the regression of not being able to import these constants is a valid concern. At least for the math constants, since for example MAX without context is arguably not a good idea.

@rfcbot concern importing

@bstrie, What do you think of having the RFC propose adding associated constants now, but only deprecating the other constants after associated items can be imported with use?

I couldn’t find an RFC for that, anyone would like to write it? https://internals.rust-lang.org/t/importing-associated-constants/6610 asked for it, and https://rust-lang.github.io/rfcs/0195-associated-items.html#scoping-of-trait-and-impl-items suggests it as a future extension.

[Centril]

cc @petrochenkov @rust-lang/lang ^----

[faern]

If this is blocked on the import issue I agree with some other people above that suggest we add the associated constants now and deprecate the module constants when imports are possible.

We can deprecate min_value/max_value now since they also can't be imported in the same way associated constants can't.

[Lokathor]

Deprecation is just a warning by default right? And all that a person does to fix this particular deprecation is search and replace some paths for some constants?

Just do it. Rip the band-aid off.

[Ixrec]

Is rustfix in a state where we can use it to make "trivial" migrations like this actually trivial?

[Mark-Simulacrum]

To me "My primary reason for pushing this RFC is that I want writers to not have to think about this issue at all; what means do we have of encouraging writers to use these constants other than deprecation?" argues for not deprecating them immediately, as deprecation forces writers to think about which path they should be using.

I am not opposed to soft-deprecating these constants. I would not even be opposed to a deprecation date tied to an edition, with the lint being auto-fixable (with presumably some compiler magic); for that matter if we deprecate them (as I believe is usual) 2-3 releases after the replacements land so that crates on stable can transition without needing #![allow(deprecated)] or so, that would also be mostly fine with me.

But I don't think deprecating anything immediately makes sense, as that makes the transition painful for everyone.

[Lokathor]

But it's (1) not actually painful to have a warning (2) totally solvable with search and replace.

People aren't being told "use this new function that works slightly differently", it's an identical value just at a new item path.

[Mark-Simulacrum]

You're not wrong that it's likely solvable in an easy way, but to me that doesn't mean we should just deprecate immediately. I don't see any advantage to doing so -- indeed, would be fine with 3 ways to access max/min values (std::u32::MAX, u32::max_value() and u32::MAX) indefinitely. I don't really personally believe that this adds confusion to users -- it seems like when reading code, independent of which of these is used, it is obvious what is happening, the fact that there's 3 ways of doing it doesn't really hurt.

In any case, I think I've expressed my viewpoint sufficiently and we're spinning at this point.

I would personally be in favor of merging a PR implementing the new constants (I think there's one already up?) without deprecating the old ones immediately, and we can separately continue discussing, perhaps on an issue, when to deprecate the old variants.

[programmerjake]

One concern with immediate deprecation is that people supporting multiple versions of Rust with the same source would be forced to either disable deprecation warnings or immediately switch to the new version which wouldn't compile on older versions (since it's not there yet). Both of those don't seem like a good migration path.

[programmerjake]

I would be in favor of a PR implementing the new constants without marking the old constants
#[deprecated] just yet, but the documentation should say that the old constants are deprecated. They can be #[doc(hidden)] after adding the #[deprecated] annotation.

[faern]

I would personally be in favor of merging a PR implementing the new constants (I think there's one already up?)

Yes. I prepared and submitted rust-lang/rust#68325 with just this content.

[bstrie]

Am I correct in drawing the following conclusions from the deprecation thread above:

1. We should not deprecate the old constants immediately.
2. We should deprecate the old constants eventually.

If this seems agreeable, then I submit that this RFC could be approved and merged at any time. A timeframe for deprecation, however long it might be or whatever form it might take, can easily be left to the tracking issue.

[Mark-Simulacrum]

It is my opinion that (presuming that was the unresolved issue), this RFC is ready for merging, per your comment's summary of the deprecation question.

[dtolnay]

I concur; we have accepted the RFC, we can add the associated constants now, and we loosely plan to deprecate but will negotiate the details later.

Personally I hold out some hope that we can figure out a way to make std::i32 etc be a reexport of the type i32 instead of a module, at which point there is no longer anything to deprecate.

[LukasKalbertodt]

This RFC was approved!

Please use the tracking issue for further discussion. Thanks!