[rustdoc] Don't try to print the value of a non-primitive const

[yotamofek]

r? @fmease maybe? (feel free to re-assign, ofc 😁)
Pretty new to these parts of the code, so I have no idea if this is an acceptable fix.
Fixes #150312

[rustbot]

fmease is not on the review rotation at the moment.
They may take a while to respond.

[yotamofek]

@GuillaumeGomez are you free to take a look, maybe?

[fmease]

NB: We might want to cease calling const_eval_poly outright to address #149635 (comment) (thereby fixing 3 GH issues) which would automatically fix the issue linked to this PR (so +1 = 4).

More concretely, for provided assoc consts we should just print = _ unless it's already a (HIR) literal (i.e., pre evaluation) in which case we can print = $LIT where $LIT is the literal (for cross-crate consts we might need to fall back to _ in all cases, not quite sure atm). Most importantly, we would stop eval'ing those consts via const_eval_poly which should also improve perf tremendously (cc #95316 (comment)).

[fmease]

Would you be interested in spinning up such an alternative PR?

[yotamofek]

Would you be interested in spinning up such an alternative PR?

Yes! Not sure when I'll get to it, but I'll be happy to take a look. Thanks!

[GuillaumeGomez]

Also please add a comment explaining what this is testing (we should maybe enforce that in all tests).

[GuillaumeGomez]

Fix looks good to me but my knowledge in this part of the code is quite limited so letting @fmease handle it. ;)

[yotamofek]

More concretely, for provided assoc consts we should just print = _ unless it's already a (HIR) literal (i.e., pre evaluation) in which case we can print = $LIT where $LIT is the literal (for cross-crate consts we might need to fall back to _ in all cases, not quite sure atm). Most importantly, we would stop eval'ing those consts via const_eval_poly which should also improve perf tremendously (cc #95316 (comment)).

Hey @fmease, I'm messing around with this, and want to make sure you've taken the consequences into account:
if we stop calling const_eval_poly, we'll be losing the ability to display the results of simple, compile-time, primitive-typed calculations. Stuff like const A: usize = 10 + 20; would be rendered in docs as const A: usize = _;, which seems a bit of a shame.

There are probably more examples even in the STD docs, but this is one that I found quite quickly: https://doc.rust-lang.org/stable/std/primitive.f32.html#associatedconstant.INFINITY

[fmease]

Thanks for reaching out! Yes, indeed, those are the consequences unfortunately. However note that prior to PR #95316 (2022) we didn't show anything; now we would at least still show simple literals.

If you think about it, starting to eagerly evaluate associated constants (even in generic contexts) was technically speaking a correctness regression. It's reasonable that users expect assoc consts to be evaluated post-mono & only when referenced just like during normal compilation. Currently we're rejecting valid patterns (like setting an assoc const to panic!()) that rustc accepts.

I knew about this issue for quite a while but I never got around to fixing it the way I'm proposing now because I've always hoped that we'd introduce new "control" #[doc(...)] attributes before that (see e.g., binarycat's draft RFC 3770 (there were discussions already years before that on Zulip)).

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[yotamofek]

@fmease took initiative and decided to try and also print hir::ExprKind::Path because.. why not? (you'll probably think of a good reason to not do that, haha)

marking as draft since it's not ready for proper review, but I would like to see if you like the direction this is going, specifically - if you could review the changes to the tests to see that this is what you meant.

[yotamofek]

BTW, print_evaluated_const (and const_eval_poly) is still used by rustdoc-json, and I'm not touching that right now.

[fmease]

Note that we could continue using c.value() for free const items that don't have non-lifetime (generic) parameters since rustc evaluates them already anyway. 🤷 As I've said in a bunch of comments, we definitely want to stop evaluating associated const items by default.

However, if you consider it not worth the complexity we can leave like this for the time being.

[yotamofek]

Right, sorry about that, managed to get a bit lost on the way :)
Let me try to implement it and we'll see just how complex it ends up.