rustdoc: Fix inconsistencies in const value rendering

[camelid]

The topic of this issue has changed since it was first opened. See #83035 (comment) for a more up-to-date summary.

---

The way they're displayed is both different (one use a BodyId whereas the other is forced to use its DefId. But ultimately, it'd be nice to be able to merge them.

Originally posted by @GuillaumeGomez in #82873 (comment)

This will be done after #82873 is merged.

[camelid]

PR is merged!

[GuillaumeGomez]

Do you want to work on it? Otherwise I'll do it. :)

[camelid]

I'm hoping to work on it, but if I don't get to it I'll try to let you know so you can do it :)

[GuillaumeGomez]

Well, please tell me if you change your mind. A lot of other things to do. ;)

[camelid]

I'm working on this now, and I noticed that statics use only a BodyId. When they come from another crate, their value is set to None. Why are constants and statics treated differently here?

[camelid]

I think the code would be greatly simplified if we always evaluated constants. I.e., currently something like const FOO: usize = 1 + 2; will render as

const FOO: usize = 1 + 2; // 3usize

I don't think showing the 1 + 2 is that valuable—unless there are cases where it's valuable? Instead, I suggest we just show

const FOO: usize = 3;

I think this is the difference between ConstantKind::expr and ConstantKind::value: expr shows the pretty-printed source, while value shows the evaluated constant.

[camelid]

One weird thing is that we print the values of some constants, but not others. E.g., we seem to print &str constants, but not custom struct constants.

[camelid]

Wow, we literally store rendered constant initializers in crate metadata just for rustdoc. So, not showing the pretty-printed version, or only showing it for local items, would probably reduce crate metadata size somewhat significantly.

[camelid]

I think as a team we should have a discussion about if we want to change constant rendering. I think the current logic is confusing both to end users and to people who work on the code.

For example, the initializer of const FOO: i32 = 42; is rendered, but the initializer of const FOO: &'static i32 = &42; is not (no value is shown, just a semicolon). That behavior seems arbitrary and confusing.

As another example, const FOO: () = (); renders as just const FOO: (); for some reason.

Furthermore, a re-export of std::i32::MAX renders as pub const MAX: i32 = i32::MAX; // 2_147_483_647i32. The i32::MAX is not clickable.

A re-export of std::f32::consts::PI renders as pub const PI: f32 = 3.14159265358979323846264338327950288f32; // 3.14159274f32. I'm not sure why it shows both the pretty-printed constant as well as the evaluated constant, which is different from the behavior of const FOO: u32 = 42;.

[GuillaumeGomez]

I don't think showing the 1 + 2 is that valuable—unless there are cases where it's valuable? Instead, I suggest we just show

I agree, printing the value directly would be more interesting. If they want to see the original code, they can see the source.

Overall, there is a lot of cleanup to do apparently. I'm surprised we didn't detect all those weird behaviours before but I'm glad you did. So except for keeping the original value in the const and show the computed value as a code comment, I think you can just ahead for the rest.

[camelid]

I agree, printing the value directly would be more interesting. If they want to see the original code, they can see the source.

👍

So except for keeping the original value in the const and show the computed value as a code comment, I think you can just ahead for the rest.

Hmm, what do you mean? Are you saying I should show only the computed value, or should not?

[GuillaumeGomez]

The computed value, you said:

I think as a team we should have a discussion about if we want to change constant rendering. I think the current logic is confusing both to end users and to people who work on the code.

So I guess it's up to debate and not decided yet. ;)

[camelid]

Yeah, perhaps let's cc @rust-lang/rustdoc for discussion.

[jyn514]

I think we should normalize the constant wherever possible. Ideally I'd like the show the original source as a comment, but I would rather omit that than have it be inconsistent if it's a cross crate reexport.

[Nemo157]

Normalization sounds good to me too, except for byte strings, these do have some major utility from being shown as their written form.

const FOO: &[u8] = [102, 111, 111];
const BAR: &[u8] = b"bar";

[camelid]

Showing evaluated strings and slices seems to be trickier since you have to deal with rustc Allocations.

Normalization sounds good to me too, except for byte strings, these do have some major utility from being shown as their written form.

Yeah, I agree. One issue is that we can't get the information of whether a &[u8] came from a byte string from the type system. Would we get that information by looking it up in the HIR?

[jyn514]

Yeah, I agree. One issue is that we can't get the information of whether a &[u8] came from a byte string from the type system. Would we get that information by looking it up in the HIR?

Could we instead show both by using span_to_snippet or something to show the original?

[camelid]

I feel somewhat opposed to showing both computed and source/pretty-printed constants. In many cases, it's redundant, and it also adds complexity to both the implementation and the behavior of rustdoc.

[jyn514]

I would rather it be complicated than inconsistent, using HIR means it will be different if you re-export it. If you want rustdoc could hide the comment if it's the same as the pretty-printed expression?

[camelid]

I would rather it be complicated than inconsistent, using HIR means it will be different if you re-export it.

Hmm, HIR will be have different behavior but the source map will have the same behavior? How does the source map interact with #![doc(html_no_source)]?

If you want rustdoc could hide the comment if it's the same as the pretty-printed expression?

The issue with that is that floating-point numbers will have different precision depending on whether they came from source or from const-eval: https://doc.rust-lang.org/nightly/std/f32/consts/constant.PI.html. Also, it seems weird that changing the way the constant is computed will change the display in the docs.

[camelid]

Interestingly, we seem to never show the value of statics, re-export or not. So, another option is to just never show the value of a constant—though I'm guessing we wouldn't want to do this.

[jyn514]

How does the source map interact with #![doc(html_no_source)]?

It doesn't. html_no_source is about the HTML file rustdoc generates, it has no relation to metadata (right now rustdoc doesn't even store it across crates: #55957).

The issue with that is that floating-point numbers will have different precision depending on whether they came from source or from const-eval: https://doc.rust-lang.org/nightly/std/f32/consts/constant.PI.html. Also, it seems weird that changing the way the constant is computed will change the display in the docs.

I feel like that's useful information though? Hiding that the behavior is inconsistent doesn't make it any less inconsistent: that's something that affects the end-behavior, as opposed to just displaying the docs inconsistentluy.

[camelid]

Following up on this: I'd like to make rustdoc consistent about always using rustc_middle::ty (& co.) data. That would mean always normalizing constants.

There are basically three ways we can show constants, and each have pros and cons for both implementation and UX:

• Source code:
  • Pros:
    • Easy to implement
    • Works across crates (I think)
  • Cons:
    • Doesn't work if source code isn't available (cf. rustdoc: Regression in display of macro_rules! matchers #86208)
    • Exposes potentially funky formatting
    • May look weird if macros are involved in generating the const (not sure about this)
    • Exposes implementation details of how the constant was computed and may not show the actual value
• HIR:
  • Pros:
    • Relatively easy to implement
    • Consistent formatting
  • Cons:
    • Doesn't work cross-crate
    • Exposes implementation details of how the constant was computed and may not show the actual value
• ty:
  • Pros:
    • Consistent formatting
    • Doesn't expose implementation details; shows actual value
    • Works cross-crate
  • Cons:
    • May render e.g. byte strings unintuitively
    • Harder to implement
    • Floating-point precision issues? (Not sure if there are issues)

My understanding is that currently, rustdoc uses a combination of ty (except for byte strings) and HIR (if it's available).

Does switching to always use ty (and only ty) sound good to you?

[jyn514]

That seems ok to me but we should try to display byte strings as strings instead of arrays if possible - maybe we could cheat by looking at the original source to see if it used b""?

[camelid]

That seems ok to me but we should try to display byte strings as strings instead of arrays if possible - maybe we could cheat by looking at the original source to see if it used b""?

That won't work if the byte string is computed (e.g., the constant's body is a call to a const fn)—but it could be good enough in most circumstances.

In the future, we could maybe record in the metadata whether a constant was written as a byte string.

[camelid]

Another inconsistency: free-standing consts have their value shown, but (inherent, not sure about trait) associated consts don't:

• https://doc.rust-lang.org/nightly/std/f32/constant.DIGITS.html
• https://doc.rust-lang.org/nightly/std/primitive.f32.html#associatedconstant.DIGITS

[camelid]

That seems ok to me but we should try to display byte strings as strings instead of arrays if possible - maybe we could cheat by looking at the original source to see if it used b""?

What if we just displayed them as hex arrays? It would be a lot simpler than trying to recover whether the byte slice was originally a byte string.

Although then user-written arrays will be displayed in hex... maybe we can just display byte strings and arrays as decimal arrays? I imagine in many cases seeing the raw byte values for a byte string might actually be good enough.

[camelid]

One issue is that for, e.g., const generics, what happens if we don't know how to pretty-print the ty::Const? E.g., if it's an ADT?

[sosthene-nitrokey]

I think the code would be greatly simplified if we always evaluated constants. I.e., currently something like const FOO: usize = 1 + 2; will render as

const FOO: usize = 1 + 2; // 3usize

I don't think showing the 1 + 2 is that valuable—unless there are cases where it's valuable? Instead, I suggest we just show

const FOO: usize = 3;

I think this is the difference between ConstantKind::expr and ConstantKind::value: expr shows the pretty-printed source, while value shows the evaluated constant.

Showing 1 + 2 might not be useful, however there are cases where showing the original value makes more sense. For example all things that involve hexadecimal.

• hex_literal::hex!
• hexadecimal arrays
• hexadecimal constants

Currently there are also inconsistencies between how module level constants are rendered and how associated constants are rendered.

For example,

pub const CONSTANT: u16 = 0x103040;

pub struct Testing;
impl Testing {
    pub const CONSTANT: u16 = 0x103040;
}

pub trait Test {
    const CONSTANT: u16 = 0x103040;
}

impl Test for Testing {
    const CONSTANT: u16 = 0x111111;
}

gives the following renders:

---

---