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

Formally deprecate the constants superseded by RFC 2700 #78335

Closed
wants to merge 5 commits into from

Conversation

bstrie
Copy link
Contributor

@bstrie bstrie commented Oct 24, 2020

Tracking issue for RFC 2700: #68490

Previously, the new associated constants defined by RFC 2700 were stabilized for Rust 1.43. At the time it was decided not to formally deprecate the old superseded constants in the same release, in order to give stable users a chance to upgrade before being subjected to warnings. Thus the old constants were "soft deprecated", i.e. the documentation for each item strongly suggested against its use but no deprecation warnings were emitted.

This PR replaces the "soft deprecation" with, er, "actual deprecation", scheduled for Rust 1.49.

bstrie added 4 commits October 24, 2020 15:58
Because the int modules contain nothing other than these
now-deprecated constants, deprecate the modules themselves
as well.
@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2020
@bstrie
Copy link
Contributor Author

bstrie commented Oct 24, 2020

Looks like this PR is failing because some tests are still using the old constants. Looks like the prior PRs that removed their use from the compiler missed a few. :) I'll clean those up.

Care needs to be taken to leave the old consts where appropriate,
e.g. backcompat tests, shadowing tests.
@bstrie
Copy link
Contributor Author

bstrie commented Oct 24, 2020

I've updated the tests. I had to be judicious in places: some tests do want to refer to the old modules (for testing e.g. backcompat regressions, module shadowing). Two rustdoc tests referred to the old modules, and being unfamiliar with the semantics of the rustdoc tests I conservatively marked those as #[allow]. The doctests on the deprecated items themselves were given hidden #[allow]s. 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.

@@ -6,6 +6,7 @@ extern crate reexport_check;
// @!has 'foo/index.html' '//code' 'pub use self::i32;'
// @has 'foo/index.html' '//tr[@class="module-item"]' 'i32'
// @has 'foo/i32/index.html'
#[allow(deprecated, deprecated_in_future)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the right thing to do - i32 overlaps with the primitive namespace and this test is checking that the it still shows up as a module.

cc @GuillaumeGomez

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with it too.

@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

I've got some other tests to fix now, fortunately they look to just be ui tests that need their line/character reference numbers updated. And I only did one by hand before figuring out there's a --bless flag to automate that. :P

@tesuji
Copy link
Contributor

tesuji commented Oct 25, 2020

The libs team is quite not in favor of deprecating integer module constants.
Closed attempt: #72885 .
The recent discussion I can find on zulip: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Deprecate.20.7Binteger.7D.3A.3Amax_value.20and.20min_value.20methods.

@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

@lzutao If nothing else this PR is helping to uncover a bunch of places where the old constants are being used in the compiler still, if the libs team decides not to accept this then I'll break those commits out and submit them separately, which will make the next attempt to do this even easier.

@jyn514
Copy link
Member

jyn514 commented Oct 25, 2020

@bstrie another possible approach is to add an allow-by-default lint and then set it to warn-by-default for all the compiler crates.

@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

@jyn514 IMO if the libs team decides that deprecating these types is a step too far at this point, then it suggests to me that we need first-class support for "soft" deprecations that would allow users to opt-in before it becomes warn-by-default. I'd rather implement that (e.g. by adding a new "soft_since" field to the rustc_deprecated attribute) than do a one-off lint just for this.

@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

Actually, following up on my previous comment, it seems like we're already 95% of the way to the lint I'm envisioning, via the allow-by-default deprecated_in_future lint, which rustc already #[warn]s for. That moves the sticking point here to the fact that there's currently no way to enable that lint for a symbol without specifying an exact version at which the symbol will become deprecated; I think what we want here to break the gridlock is the ability to specify an indeterminate version for deprecation.

I believe that would satisfy all parties here, by both adorning the docs with big obvious notices without bothering older MSRV codebases. The decision of the exact version at which to deprecate these symbols could be left to the future.

@bstrie bstrie marked this pull request as draft October 25, 2020 21:57
@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

Given the size of the diff for fixing up the test suite, and since those commits aren't going to need discussion like the stabilization changes will, I'm going to go ahead and extract those commits into their own PR for the sake of avoiding bitrot. I've also stumbled across a place where the compiler is still suggesting some of the old consts, so I'm going to work on fixing that and then get back to this. I'll also go ahead and implement the "indeterminate soft deprecation" I describe in my previous comment, in the hopes that that will assuage concerns about deprecation here. Closing this in the meantime to be kind to the PR queue.

@bstrie bstrie closed this Oct 25, 2020
@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

Succeeded by #78380.

@bstrie
Copy link
Contributor Author

bstrie commented Oct 25, 2020

For discussing the first-class soft deprecation ideas suggested here, I've opened #78381 .

JohnTitor added a commit to JohnTitor/rust that referenced this pull request 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 pull request 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 pull request 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)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants