LukasKalbertodt commented 4 years ago

This is a tracking issue for the RFC 2700 (rust-lang/rfcs#2700): "Deprecate stdlib modules dedicated to numeric constants and move those constants to associated consts".

Steps:

[x] Add new constants (see #68325)
[x] Stabilize new constants (see instructions on rustc-guide)
[x] Update test suite to use new constants (#78380)
[x] Fix error messages using old symbols (#78382)
[x] Support for indeterminate deprecation dates (#78381)
[x] Deprecate-in-future the old items
[ ] Fully deprecate the old items
[x] Adjust documentation (see instructions on rustc-guide)

Unresolved questions:

[x] [Resolved: Yes] Should the old items be deprecated? See the RFC thread as well as "unresolved questions":

How long should we go before issuing a deprecation warning? At the extreme end of the scale we could wait until the next edition of Rust is released, and have the legacy items only issue deprecation warnings when opting in to the new edition; this would limit disruption only to people opting in to a new edition (and, being merely an trivially-addressed deprecation, would constitute far less of a disruption than any ordinary edition-related change; any impact of the deprecation would be mere noise in light of the broader edition-related impacts). However long it takes, it is the opinion of the author that deprecation should happen eventually, as we should not give the impression that it is the ideal state of things that there should exist three ways of finding the maximum value of an integer type; we expect experienced users to intuitively reach for the new way proposed in this RFC as the "natural" way these constants ought to be implemented, but for the sake of new users it would be a pedagogical wart to allow all three to exist without explicitly calling out the preferred one.
[x] [Resolved: No] Should constants from std::{f32, f64}::consts also be made associated consts? From the alternative question of the RFC:

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.

agausmann commented 4 years ago

Add new constants (see #68123)

68325 is the relevant PR.

faern commented 4 years ago

I have implemented the stabilization and documentation changes needed to show people which constant is the preferred one now. I will submit this as a PR in a few days when the unstable version has been out for a bit over a week.

You can find the code here: https://github.com/rust-lang/rust/compare/master...faern:stabilize-assoc-int-consts?expand=1

What parts of it I don't think will be controversial at all:

Stabilizing the unstable constants
Updating the documentation on the old constants to indicate there are now better alternatives
Update the integer/float documentation examples to actually use the new constants (the docs should reflect the preferred way)

Other changes that possibly might create some discussion:

Exactly how to formulate the documentation telling people to not use the old constants any longer.
Moved the min_value, max_value methods to the very bottom of the integer impl block, so they are not taking up the best spots in the documentation, but is rather moved away a bit. The MIN/MAX constants took their place. They can still of course easily be found, but they are not at the very top.

I opted for doing what Error::description() did with the documentation. Saying something like this:

//! **this module is soft-deprecated.**
//!
//! Although using these constants won’t cause compilation warnings,
//! new code should use the associated constants directly on
//! the [`i128` primitive type](../../std/primitive.i128.html) instead.

I added soft deprecation notices to:

Module level documentation of core and std modules for all integer and float types
The old free standing MIN/MAX constants in the integer modules.
The min_value/max_value methods on the integer types.

faern commented 4 years ago

The code/docs were changed a bit after I wrote that last comment just above. But the stabilization PR is live now.

faern commented 4 years ago

One thing that remains is to improve the compiler error that mentions the MIN and MAX constants. Try this code on nightly:

fn main() {
    match 5i32 {
        5 => (),
    }
}

It gives the following error:

error[E0004]: non-exhaustive patterns: `std::i32::MIN..=4i32` and `6i32..=std::i32::MAX` not covered

Now when the associated constants are the preferred ones the error should be

error[E0004]: non-exhaustive patterns: `i32::MIN..=4i32` and `6i32..=i32::MAX` not covered