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

Mark std integral modules as deprecated (std::u32, std::i16, etc.) #107587

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Feb 2, 2023

This PR seeks to officially deprecate the number modules in core and std, which is the last remaining open item on #68490. It also marks them as #[doc(hidden)] to avoid confusion.

Motivation

The main reason to hide the documentation rather than just deprecating is user confusion: it seems like users have not infrequent questions about why Rust is deprecating integers (@Nilstrieb mentioned in chat and @jsha linked in this comment #107579 (comment)).

It also isn't great for the optics of language stability that the first result for some common-ish searches link to a page stamped with "deprecation planned" all over.

image

Jsha linked this relevant RFC, which indicates that removing these modules from the docs was part of the original plan: #107579 (comment)

Links:

Newly deprecated & hidden modules

  • core::u16
  • core::u32
  • core::u64
  • core::u128
  • core::usize
  • core::i8
  • core::i16
  • core::i32
  • core::i64
  • core::i128
  • core::isize
  • core::f32::{DIGITS, EPSILON, INFINITY, MANTISSA_DIGITS, MAX, MAX_10_EXP, MAX_EXP, MIN, MIN_10_EXP, MIN_EXP, MIN_POSITIVE, NAN, NEG_INFINITY, RADIX}
  • core::f64::{DIGITS, EPSILON, INFINITY, MANTISSA_DIGITS, MAX, MAX_10_EXP, MAX_EXP, MIN, MIN_10_EXP, MIN_EXP, MIN_POSITIVE, NAN, NEG_INFINITY, RADIX}
  • std reexports of these modules & consts

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). A good long term plan could be to re-export the contents of consts into their parent modules of f32 and f64, then deprecate the consts module. There is some more discussion of this in the original post at #68490, but no decision is needed here at this time.

Alternatives

If the team is still against full deprecation (as indicated in the old zulip thread), I think it at least makes sense to still mark the items #[doc(hidden)] to help resolve the user confusion mentioned above.

cc @bstrie, original author of the RFC

@rustbot label +T-libs-api

@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

r? @Mark-Simulacrum

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 2, 2023
@Noratrieb
Copy link
Member

It also marks them as #[doc(hidden)] to avoid confusion.

While this does remove some confusion, it also adds confusion. Someone might be reading old code and see std::i32::MAX used. They want to know more about it so they look up the docs, only to not find it at all. I think it would be better to instead have a small note saying that the deprecation is about the modules and not the types.

@pitaj
Copy link
Contributor

pitaj commented Feb 2, 2023

I agree it could add confusion in addition to breaking legacy links to these items. I think it would be better to have rustdoc exclude or derate deprecated items for search engines if that is a major concern.

I see the other discussion now. One option not brought up there is making the canonical links for deprecated items point to the replacement.

@pitaj
Copy link
Contributor

pitaj commented Feb 2, 2023

Currently there is no indication that an item is deprecated in the rustdoc search results. That could also be improved as currently std::char::MAX is the top constant result for MAX.

@est31
Copy link
Member

est31 commented Feb 2, 2023

I think they can be deprecated now, the MSRV concerns are gone, even debian 11 has rustc 1.43.

I don't think one should completely hide the files from the docs. One could think about changing rustdoc's search results to show if an item was deprecated. To do SEO to make the constants not appear in google searches for "i32 max" that's tougher. I think the issues were correctly identified on zulip, google prefers to link to "dedicated" pages where the name is part of the URL instead of "collection" pages where the thing is just a section in the page. The associated consts do not have dedicated pages, while std::u32::MAX has, so it wins.

One could maybe add a <meta name="robots" content="noindex"> to the html page of std::u32::MAX. This will likely require rustdoc changes. E.g. one could add an ability to add arbitrary html tags to the top of the page, or alternatively one could add one option just for noindex (say #![doc(html_noindex)] or something). At the extreme end, rustdoc could emit it for all deprecated items.

Generally I'm not a fan of hiding too much deprecated stuff though, because there are use cases that benefit from having pages even for deprecated items show in search results. E.g. if it's not as simple as in this instance, if the renamed item is called differently, and you are during a migration from deprecated items to their new equivalents, then you might be interested in the rustdoc page for the deprecated item. I think most people would use rustdoc's builtin search, but some might also expect Google's search to work.

One option not brought up there is making the canonical links for deprecated items point to the replacement.

It would generally be good to be able to run rustfix on deprecated items, if there is a 1:1 correspondence to an API in another place (it's just been a rename). See also rust-lang/rustfix#151 with some discussion about this.

@pitaj
Copy link
Contributor

pitaj commented Feb 2, 2023

Maybe there's been a misunderstanding. When I was talking about canonical links, I was talking about URL Canonicalization. This would essentially mark the std::int:: constants as duplicate pages (hiding them from search results) and may increase the rank of the canonical page, since any links to std::int:: constants would count towards the int:: constants.

But maybe you were just saying that it could use the same metadata as rustdoc. In which case, I agree.

@est31
Copy link
Member

est31 commented Feb 2, 2023

@pitaj using canonical pages feels like a hack to me as std::u8::MAX is a different item in Rust's terminology.

Reading the discussion in #107579, i doubt that mirrors like mit.edu will be shown instead if you add a noindex tag. Google already now shows the right page right below the MAX page.

Another idea would be to add (Deprecated) to a page's title if the entire item is deprecated.

@jsha
Copy link
Contributor

jsha commented Feb 2, 2023

@pitaj said:

When I was talking about canonical links, I was talking about URL Canonicalization.

That won't work. When search engines process <link rel="canonical"> they check to make sure the source and dest are nearly duplicates of each other. Since the pages in question are very different, search engines would disregard <link rel="canonical">.

There are three problems here:

  1. When readers navigate the documentation via https://doc.rust-lang.org/std/#modules, the modules list is cluttered by 12 numeric modules that aren't really needed anymore. This is the problem described in the motivation section of RFC 2700.
  2. When readers navigate the documentation via https://doc.rust-lang.org/std/#modules, they come to the incorrect conclusion that the numeric primitives are deprecated, because they see i8 [Deprecation planned]. This problem was introduced by TBD-deprecating the modules.
  3. When readers search for documentation on MAX or MIN constants via Google, they wind up on a page that has all the correct information but also has a deprecation warning.

(1) and (2) cannot be fixed by any noindex or search engine changes.

(1) can only be fixed by #[doc(hidden)].

(2) is not made better or worse by moving these modules from TBD-deprecated to actually-deprecated.

In my mind, (2) is the worst of these problems. In a few minutes I found three different threads from confused users asking why Rust is deprecating the integers:

@pitaj
Copy link
Contributor

pitaj commented Feb 2, 2023

Alright, canonicalization isn't going to work.

Is there a way to hide the modules from "discovery" (hide them from the module list) without hiding the actual docs (still allow directly navigating to the page and show them in rustdoc search)?

That would solve (1) and (2) directly, and perhaps hiding them from search engines (3) could be attached to that same flag.

A separate flag may not be necessary, maybe it's tied to whether the deprecation has a direct replacement.

@tgross35
Copy link
Contributor Author

tgross35 commented Feb 2, 2023

@jsha put it quite succinctly, thanks for the summary.

I think there has to be a path forward to remove these items from the module listing and search engine results to help new users. It's very likely that new users stumble across these pages / module listings and get confused - this is cited a few places. I believe it's much less likely that somebody trying to understand old code actually goes to the documentation to verify what std::i32::MAX might mean. With new users growing and code with std::i32 dwindling, I think it's significantly more important to cater to that first case.

It is important to note that hiding items from the docs should not be a precedent for basically anything. This is a special exception that needs to be addressed, because this is a pretty awful user experience to have on the main docs landing page:

image

Is there a way to hide the modules from "discovery" (hide them from the module list) without hiding the actual docs (still allow directly navigating to the page and show them in rustdoc search)?

That sounds like a great option to me, perhaps via a #![doc(nolist)] attribute. Combined with a #![doc(html_noindex)] mentioned by @est31 (or just combined, from what @pitaj said) I would consider the issues I see resolved, and I think it addresses many of the listed concerns.

There is an open issue for decreasing the rank of deprecated items within the rustdoc search, but it hasn't gotten any attention: #98759. That would also help things

When I get the chance, I'll change the PR contents to only deprecate the items without hiding them, then open a separate issue for adding something like #![doc(nolist)] (if consensus says that's a good way forward).

@GuillaumeGomez
Copy link
Member

Adding a doc(noindex) (or equivalent) seems like a good idea. The page still exists but the item doesn't show up in the search. Sounds like a good compromise.

It'll need to go through at least FCP since we're adding a new feature but otherwise sounds good to me.

@Pita
Copy link

Pita commented Feb 2, 2023

Hi @pitaj

@tgross35

This comment was marked as off-topic.

@jsha
Copy link
Contributor

jsha commented Feb 2, 2023

FWIW, I don't think doc(noindex) is a good idea. We already have doc(hidden). We shouldn't have various shades of more-hidden and less-hidden. If we something gone from the documentation, great, let's do that. If we want something to be included in the documentation, it's confusing to say "this is included but we don't want you to find it."

@GuillaumeGomez
Copy link
Member

But that allows to keep links to these items to continue to work. Also, it can come in handy for the duplicates between core and std.

@tgross35
Copy link
Contributor Author

tgross35 commented Feb 2, 2023

My other thought was something like #![doc(hidden, redirect = "i128")] if the pages were to be totally hidden, so existing links still go somewhere.

But I do think #![doc(noindex)] is a better solution than that. I am imagining that this would:

  1. Hide the item from listing in the parent module
  2. Remove the item from search engine results with a noindex meta
  3. Allow direct links at the item to point at the right page
  4. Leave the item available to find within rustdoc search, and for direct linking to. Something to implement Rustdoc reduce search rank for deprecated items #98759 would make it less prominent here

So all in all it seems like a good solution for things that you don't want to explicitely promote, but also don't want to make completely impossible to view.

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-13 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:ac593985615ec2ede58e132d2e21d2b1cbd6127c)
Download action repository 'rust-lang/simpleinfra@master' (SHA:055e3b93d15803815fe6f9cbc1b02b11be094e54)
Complete job name: PR (x86_64-gnu-llvm-13, false, ubuntu-20.04-xl)
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-llvm-13
---
   Compiling rand_xorshift v0.3.0
   Compiling std v0.0.0 (/checkout/library/std)
   Compiling core v0.0.0 (/checkout/library/core)
   Compiling alloc v0.0.0 (/checkout/library/alloc)
error: use of deprecated constant `std::f32::NAN`: replaced by the `NAN` associated constant on `f32`
    |
    |
657 |     let array3: [f32; 3] = [1.0, std::f32::NAN, 3.0];
    |
    |
    = note: `-D deprecated` implied by `-D warnings`

error: use of deprecated module `std::i128`: all constants in this module replaced by associated constants on `i128`
 --> library/core/tests/num/i128.rs:1:13
  |
1 | int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:12:25
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
12  | |                 assert!(MAX > 0);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:13:25
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
13  | |                 assert!(MIN <= 0);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:14:28
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
14  | |                 assert_eq!(MIN + MAX + 1, 0);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:14:34
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
14  | |                 assert_eq!(MIN + MAX + 1, 0);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:24:50
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
24  | |                 assert_eq!((-1 as $T).rem_euclid(MIN), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:24:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
24  | |                 assert_eq!((-1 as $T).rem_euclid(MIN), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:101:28
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
101 | |                 assert_eq!(MAX.leading_ones(), 0);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:104:28
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
104 | |                 assert_eq!(MAX.trailing_ones(), $T::BITS - 1);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:184:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
184 | |                 assert_eq!((MAX - 2).saturating_abs(), MAX - 2);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:184:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
184 | |                 assert_eq!((MAX - 2).saturating_abs(), MAX - 2);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:185:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
185 | |                 assert_eq!((MAX - 1).saturating_abs(), MAX - 1);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:185:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
185 | |                 assert_eq!((MAX - 1).saturating_abs(), MAX - 1);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:186:28
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
186 | |                 assert_eq!(MAX.saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:186:50
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
186 | |                 assert_eq!(MAX.saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:187:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
187 | |                 assert_eq!((MIN + 2).saturating_abs(), MAX - 1);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:187:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
187 | |                 assert_eq!((MIN + 2).saturating_abs(), MAX - 1);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:188:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
188 | |                 assert_eq!((MIN + 1).saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:188:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
188 | |                 assert_eq!((MIN + 1).saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:189:28
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
189 | |                 assert_eq!(MIN.saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:189:50
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
189 | |                 assert_eq!(MIN.saturating_abs(), MAX);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:197:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
197 | |                 assert_eq!((MAX - 2).saturating_neg(), MIN + 3);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:197:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
197 | |                 assert_eq!((MAX - 2).saturating_neg(), MIN + 3);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:198:29
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
198 | |                 assert_eq!((MAX - 1).saturating_neg(), MIN + 2);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MIN`: replaced by the `MIN` associated constant on this type
   --> library/core/tests/num/int_macros.rs:198:56
1   | / macro_rules! int_module {
1   | / macro_rules! int_module {
2   | |     ($T:ident) => {
3   | |         #[cfg(test)]
4   | |         mod tests {
...   |
198 | |                 assert_eq!((MAX - 1).saturating_neg(), MIN + 2);
...   |
368 | |     };
369 | | }
    | |_- in this expansion of `int_module!`
    | |_- in this expansion of `int_module!`
    |
   ::: library/core/tests/num/i128.rs:1:1
    |
1   |   int_module!(i128);


error: use of deprecated constant `std::i128::MAX`: replaced by the `MAX` associated constant on this type
   --> library/core/tests/num/int_macros.rs:199:28

@est31
Copy link
Member

est31 commented Feb 3, 2023

Two further proposals to improve the situation, that might be enough on their own to not need #![doc(noindex)] but might also be good ideas regardless whether there is #![doc(noindex)]/#![doc(hidden)].

changing the title

Google's search results reproduce the title in the most prominent position. If you google for "rust u32 max", you are shown a top level link with the title "MAX in std::u32 - Rust". What about changing rustdoc to include the fact that the item is deprecated in the title? Maybe "MAX (deprecated) in std::u32 - Rust"? It shouldn't be the first word as when you have many tabs open with deprecated items, then it will remove the rightmost parts first, so you might get into the situation where you have multiple tabs with "Deprec", "Deprec", "Deprec" as substrings of "Deprecated MAX in std::u32 - Rust".

I think this might actually improve the situation for all deprecated items.

changing the module-level docs

Right now it only indicates the difference from the native types indirectly:

Screenshot_20230203_173542

One could make it more explicit that the construct here is a module by mentioning it directly in the first line, and not the type:

Screenshot_20230203_173706

The confusion about rust deprecating the type instead of the module is probably partially caused by the fact that both module and type have the same name down to the casing. u32 and friends are oddities, as they don't follow the normal rust naming conventions. There is less confusion between std::string and String for example. Thus, maybe some of the confusion can be avoided by making it more explicit that this is a module, and not the type itself, by:

  • explicitly saying "constants module" and
  • directly linking to the primitive type

@pitaj
Copy link
Contributor

pitaj commented Feb 4, 2023

@est31 hope you don't mind, but I went ahead and implemented your suggestion to reword the module description in #107654. I figure this is useful as a small PR separate from this one, as it can probably be merged without much fanfare.

@jsha
Copy link
Contributor

jsha commented Jun 14, 2023

I like @dtolnay's ideas about taking a non-deprecation approach to this.

For users, it's not clear that deprecating these constants solves a problem. And it introduces some other rather complicated problems - like how to present the deprecation information without making it look like the primitive types themselves are deprecated.

To the extent the deprecation solves a user problem, it's this from RFC 2700:

Documentation. On the front page of the standard library API docs, 12 of the 60 modules in the standard library (20%) are the aforementioned numeric modules which exist only to namespace two constants each.

(today it's 12 out of 67, 18%)

I think this is not a large problem. It's still very straightforward to skim the list of modules in std, and it's easy to see that all the integer modules are related to each other and skim past them.

On the other hand, attempting to solve this documentation problem is a big deal because it requires us to create a new category of item that is both deprecated and hidden. Right now we don't hide deprecated items from the documentation, because those items are used in running code and people need to find out what they do.

I propose not deprecating, and removing the Deprecation planned annotation. That removes the confusion about deprecating primitive types, and doesn't lose anything significant from a user's point of view. It loses a little from a language design purity point of view, but I think that is an okay tradeoff. To guide people towards the preferred style, we could introduce a clippy lint.

@m-ou-se
Copy link
Member

m-ou-se commented Jun 20, 2023

We discussed this briefly in the libs-api meeting again. We think it might be valuable to have a (clippy) lint for this. (And then later uplift it to rustc.)

@joshtriplett
Copy link
Member

Nit/bikeshed: severity seems like the wrong word for this. Perhaps just a why = "legacy" field? That would also fit the suggested why = "should-be-safe", and other potential reasons in the future.

@pitaj
Copy link
Contributor

pitaj commented Jun 20, 2023

Opened a clippy issue for this: rust-lang/rust-clippy#10995

Planning on working on it in the coming weeks but if someone else wants to jump on it, go right ahead.

@joshtriplett
Copy link
Member

@SnoozeThis rust-lang/rust-clippy#10995 && wait 4 months

Let's revisit this after the clippy lint has been around for a while.

@SnoozeThis
Copy link

(https://snoozeth.is/XWplGyz9_wI) I will wait until rust-lang/rust-clippy#10995 is closed and wait until Fri, 27 Oct 2023 15:53:04 UTC and then add a comment.

@Amanieu Amanieu removed the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jul 4, 2023
@dtolnay
Copy link
Member

dtolnay commented Sep 8, 2023

@rfcbot fcp cancel

@dtolnay dtolnay added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Sep 8, 2023
@rfcbot
Copy link

rfcbot commented Sep 8, 2023

@dtolnay proposal cancelled.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Sep 8, 2023
@dtolnay dtolnay assigned dtolnay and unassigned Mark-Simulacrum Sep 8, 2023
@bors
Copy link
Contributor

bors commented Feb 19, 2024

☔ The latest upstream changes (presumably #121295) made this pull request unmergeable. Please resolve the merge conflicts.

@SnoozeThis
Copy link

Resolved.

@est31
Copy link
Member

est31 commented Mar 31, 2024

The clippy lint has just been merged (rust-lang/rust-clippy#12312 (comment)), so I suppose now we should be waiting 4 months to see it spread in the ecosystem. August 1st 2024?

@SnoozeThis wait 4 months

@SnoozeThis
Copy link

(https://snoozeth.is/HLD_dIaUAH0) I will wait until Wed, 31 Jul 2024 23:17:59 UTC and then add a comment.

@rustbot claim.

@rustbot rustbot assigned SnoozeThis and unassigned dtolnay Mar 31, 2024
@Dylan-DPC Dylan-DPC assigned dtolnay and unassigned SnoozeThis Jul 26, 2024
@SnoozeThis
Copy link

Resolved.

@rustbot assign @dtolnay.

@dtolnay
Copy link
Member

dtolnay commented Aug 1, 2024

@rustbot author

Please rebase and update since attributes to 1.82.0.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 1, 2024
@pitaj
Copy link
Contributor

pitaj commented Aug 1, 2024

Search for allows of that lint yields only 11 results.

@alex-semenyuk alex-semenyuk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.