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

Tracking issue for RFC 2700: numeric constants as associated consts #68490

Open
9 of 10 tasks
LukasKalbertodt opened this issue Jan 23, 2020 · 18 comments
Open
9 of 10 tasks
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@LukasKalbertodt
Copy link
Member

LukasKalbertodt commented Jan 23, 2020

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:

Unresolved questions:

  • [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.

  • [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.

@LukasKalbertodt LukasKalbertodt added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jan 23, 2020
@jonas-schievink jonas-schievink added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Jan 23, 2020
@agausmann

This comment has been minimized.

@faern
Copy link
Contributor

faern commented Feb 5, 2020

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
Copy link
Contributor

faern commented Feb 8, 2020

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

bors added a commit that referenced this issue Mar 4, 2020
Stabilize assoc_int_consts associated int/float constants

The next step in RFC rust-lang/rfcs#2700 (tracking issue #68490). Stabilizing the associated constants that were added in #68325.

* Stabilize all constants under the `assoc_int_consts` feature flag.
* Update documentation on old constants to say they are soft-deprecated and the new ones should be preferred.
* Update documentation examples to use new constants.
* Remove `uint_macro` and use `int_macro` for all integer types since the macros were identical anyway.

r? @LukasKalbertodt
@faern
Copy link
Contributor

faern commented Jun 9, 2020

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

RalfJung added a commit to RalfJung/rust that referenced this issue Jun 11, 2020
…, r=dtolnay

Migrate to numeric associated consts

The deprecation PR is rust-lang#72885

cc rust-lang#68490
cc rust-lang/rfcs#2700
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 12, 2020
…, r=dtolnay

Migrate to numeric associated consts

The deprecation PR is rust-lang#72885

cc rust-lang#68490
cc rust-lang/rfcs#2700
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 12, 2020
…, r=dtolnay

Migrate to numeric associated consts

The deprecation PR is rust-lang#72885

cc rust-lang#68490
cc rust-lang/rfcs#2700
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jun 13, 2020
Prefer the associated constants for pattern matching error

Resolved this comment: rust-lang#68490 (comment)
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 23, 2020
…, r=dtolnay

Migrate to numeric associated consts

The deprecation PR is rust-lang#72885

cc rust-lang#68490
cc rust-lang/rfcs#2700
@KodrAus KodrAus added I-nominated Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@bstrie
Copy link
Contributor

bstrie commented Oct 24, 2020

As the original author of RFC 2700, I propose the following resolution to the two currently-listed unresolved questions:

  1. "Should the old items be deprecated?" I propose yes. I have a PR here for further discussion: Formally deprecate the constants superseded by RFC 2700 #78335

  2. "Should constants from std::{f32, f64}::consts also be made associated consts?" I propose no. As mentioned in the RFC, the majority of the benefit of this change concerns the integer types, which have no analogue to the consts module on the float types. While it remains my opinion that having a stdlib type shadowing a primitive type is undesirable, I think it's reasonable to leave this topic to a future RFC.

@Lokathor
Copy link
Contributor

if I can't just write f32::NAN then what the heck is the point of any of this.

@bstrie
Copy link
Contributor

bstrie commented Oct 24, 2020

Unsure if that was meant to be sarcasm, but if you take the time to read the original RFC you will see that the point of all this is that std::u8::MAX and u8::max_value() are both substandard alternatives to u8::MAX that only exist because of extremely temporary historical technical limitations. When fixing this for the integral types it was natural to preserve symmetry with the floating point types as well. Some people objected to all the constants defined for the floating point types being given this treatment. Since the point of the RFC was to fix the integral types, we conceded so as to avoid gridlock over tangential concerns. The libs team could, of course, decide to change this policy (it would not be out of the realm of possibility to simply amend the existing RFC), but I am not on the libs team. My PR fully deprecates the constants that were superseded in #68952 . I'm content to take this one step at a time.

@Lokathor
Copy link
Contributor

I was not being sarcastic at all.

If floating types don't get the same design fixes applied them, then that's dumb for all the reasons that std::u8::MAX is dumb.

@bstrie
Copy link
Contributor

bstrie commented Oct 24, 2020

You're preaching to the choir; as mentioned in the accepted RFC, the original RFC text had proposed this treatment for all the float constants. I'd be happy if the libs team decided to deprecate the float shadow modules as well, but in the absence of that decision there's no need to put off deprecating the integral shadow modules any longer. This RFC has gone on for quite long already.

@bstrie
Copy link
Contributor

bstrie commented Oct 24, 2020

(It's also worth noting that your specific example of f32::NAN does indeed already exist as a result of this RFC, and the alternative std::f32::NAN would be deprecated by my PR. It's things like std::f32::consts::PI that wouldn't be.)

@bstrie
Copy link
Contributor

bstrie commented Oct 25, 2020

In my deprecation PR I ran into some obstacles and have updated the issue here with references to new issues and PRs. I'll hold off on proposing deprecation until those are addressed.

m-ou-se pushed a commit to m-ou-se/rust that referenced this issue Nov 18, 2020
Part of rust-lang#68490.

Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.

For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 18, 2020
… r=jyn514

Update tests to remove old numeric constants

Part of rust-lang#68490.

Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.

For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
bstrie pushed a commit to bstrie/rust that referenced this issue Nov 29, 2020
Part of rust-lang#68490.

Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.

For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 29, 2020
…=jyn514

Update tests to remove old numeric constants

Part of rust-lang#68490.

Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.

For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 6, 2020
Part of rust-lang#68490.

Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.

For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
@m-ou-se
Copy link
Member

m-ou-se commented Dec 16, 2020

We discussed the open questions in the library meeting last week. Our conclusions were:

Should the old items be deprecated?

Yes.

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.

@bstrie
Copy link
Contributor

bstrie commented Dec 17, 2020

Checking off "adjust the documentation" (found no uses of the old consts in the reference, the book, or Rust By Example, which I presume faern fixed when originally adjusting the stdlib docs). In addition #79877 has landed, which finishes checking off all the new issues that were filed after the closing of the prior deprecation effort (#78335). Once #79877 rides the train into beta later this month I'll file a new deprecation PR. And once that's all done I'll write up a new RFC to discuss the fate of the floating-point const modules.

@bstrie bstrie self-assigned this Dec 17, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Deprecate-in-future the constants superceded by RFC 2700

Successor to rust-lang#78335, re-opened after addressing the issues tracked in rust-lang#68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via `rustc_deprecated(since="TBD"`, see rust-lang#78381). We might call this *soft deprecation*; unlike with deprecation, users will *not* receive warnings when compiling code that uses these items *unless* they opt-in via `#[warn(deprecated_in_future)]`. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see rust-lang#68490 (comment)).
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 14, 2021
Deprecate-in-future the constants superceded by RFC 2700

Successor to rust-lang#78335, re-opened after addressing the issues tracked in rust-lang#68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via `rustc_deprecated(since="TBD"`, see rust-lang#78381). We might call this *soft deprecation*; unlike with deprecation, users will *not* receive warnings when compiling code that uses these items *unless* they opt-in via `#[warn(deprecated_in_future)]`. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see rust-lang#68490 (comment)).
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 21, 2021
Deprecate-in-future the constants superceded by RFC 2700

Successor to rust-lang#78335, re-opened after addressing the issues tracked in rust-lang#68490.

This PR makes use of the new ability to explicitly annotate an item as triggering the deprecated-in-future lint (via `rustc_deprecated(since="TBD"`, see rust-lang#78381). We might call this *soft deprecation*; unlike with deprecation, users will *not* receive warnings when compiling code that uses these items *unless* they opt-in via `#[warn(deprecated_in_future)]`. Like deprecation, soft deprecation causes documentation to formally acknowledge that an item is marked for eventual deprecation (at a non-specific point in the future).

With this new ability, we can sidestep all debate about when or on what timeframe something ought to be deprecated; as long as we can agree that something ought to be deprecated, we can receive much of the benefits of deprecation with none of the drawbacks. For these items specifically, the libs team has already agreed that they should be deprecated (see rust-lang#68490 (comment)).
@bstrie
Copy link
Contributor

bstrie commented Feb 6, 2021

With the merge of #80958 , I'm happy with where this is at. I will leave this issue open until such time as the old items are marked as fully deprecated rather than just deprecated-in-future, but making that change is not a priority of mine at this moment.

@pitaj
Copy link
Contributor

pitaj commented Feb 2, 2023

RE from #107587

The float modules are a bit weird since all the constants mentioned above are redundant, but they also contain the core::{f32, f64}::consts modules, and these do contain information that isn't available elsewhere (constants like E, PI, SQRT_2, etc)

Can we move them into a module like core::math or something?

  • core::f32::consts would become core::math::f32
  • core::f64::consts would become core::math::f64

Then we could fully deprecate core::f32 and core::f64

@pitaj
Copy link
Contributor

pitaj commented Feb 3, 2023

Putting it here because I haven't see much discussion elsewhere. small previous discussion

It appears that the std::char constants haven't been marked as "deprecated" or "deprecation planned", despite all being on the char primitive type as well. It seems weird to keep duplicate constants around. Is this something needing a separate RFC (or maybe just ACP) or can we loop it in here?

@faern
Copy link
Contributor

faern commented Feb 13, 2023

Can we move them into a module like core::math or something?

* `core::f32::consts` would become `core::math::f32`
* `core::f64::consts` would become `core::math::f64`

I don't think creating yet another copy of the constants is the right direction. Since the old ones can't be removed, only deprecated, the API surface will inevitably grow. I'm all for moving constants to better places when the new place is actually better. Here it feels like they are equally bad/good but just duplicated for the sake of deprecation.

Personally I always wanted to move the float constants onto the actual float types as well. But I know this was controversial back in RFC 2700. I know there was some talk about being able to put modules in types, so one could have f32::math::PI but that the language did not at the time support it. I can't find that comment now so I'm not sure what the status of this is.

It appears that the std::char constants haven't been marked as "deprecated" or "deprecation planned", despite all being on the char primitive type as well. It seems weird to keep duplicate constants around. Is this something needing a separate RFC (or maybe just ACP) or can we loop it in here?

I'm all for deprecating that as well, but not in this RFC/issue. This issue seems to be controversial as is and moves way too slowly. Anything that can just have this be merged asap would be great. Then further similar work can happen in other RFCs. If we let RFCs scope creep it will greatly slow down their progress.

@pitaj
Copy link
Contributor

pitaj commented Feb 13, 2023

You can kind of emulate a module in this case by using inherent_associated_types: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=1498d15e059ff14dd69ae46c5a10ee94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants