-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rustdoc: Format constant values as rich-text and visibility-awarely #99688
Rustdoc: Format constant values as rich-text and visibility-awarely #99688
Conversation
cc @rust-lang/wg-const-eval |
Is this necessary? The current const pretty-printing is not the result of a lot of careful design, it's just "good enough" for now. It is barely ever seen by users, so there is little pressure to improve it. Given that the rustdoc one will probably be polished, and is seen much more, I think it'd be great if the code could be shared, so that consts as printed in MIR can be (almost) as nice as what rustdoc does. |
Thanks for giving some context. I get where you are coming from and I understand your concern about a potential lack of code sharing / potential code duplication. I think, however, that rustdoc's version of the pretty-printer is going to heavily diverge from the original one both over the course of this PR and over the general lifetime of rustdoc. Already, there are some parts that drastically differ. In the beginning, I did indeed copy & paste the printing logic but it was a mere starting point – and the only one I'd argue. I really hope that some of my advancements will flow back into the original pretty-printer. Although most of the polishing you speak of will be very specific to rustdoc. Right now it consists of hiding private and hidden fields, hyperlink anchors and ellipses for long arrays and strings. I am not sure whether they fit your use case but I'd be happy to be corrected :) |
Overall it's really great! I didn't review too much into the details since we need to have everyone on board with this change but I'm really hopeful about this. cc @rust-lang/rustdoc |
@fmease that does sound reasonable; I wanted to make sure you thought about this. :) Would be good to ensure the PR description reflects the final reaosning when the PR lands (as that will be embedded in the git history, too). I'm not familiar enough with the code to really make any concrete suggestions here. Let's see if @oli-obk has any ideas. |
e094a0a
to
097a3ff
Compare
This comment has been minimized.
This comment has been minimized.
ee5d85f
to
33570cf
Compare
Some changes occurred in src/librustdoc/clean/types.rs cc @camelid Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @Folyd, @jsha rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing |
33570cf
to
2385e24
Compare
d8a007a
to
5266047
Compare
b7e11fe
to
637e156
Compare
☔ The latest upstream changes (presumably #100318) made this pull request unmergeable. Please resolve the merge conflicts. |
637e156
to
c5a4d38
Compare
☔ The latest upstream changes (presumably #100346) made this pull request unmergeable. Please resolve the merge conflicts. |
c5a4d38
to
6ef267a
Compare
This is not changing the const-eval interpreter or its const representation in any way, and presumably uses the existing APIs in much the same way as the existing pretty-printer, so I don't think there is much I can contribute here in terms of reviewing -- unless you have questions about specific const-eval / interpreter APIs. So, I leave the decision here up to the primary maintainers of the rustdoc crate. @lcnr and @oli-obk know better than me what is planned for const representation and valtrees, which affect this much more than my own work on the interpreter internals. |
Let's wait to see if one of the two has something to add then. :) |
my current goal would result in constants in patterns always being fully opaque using This means that to print associated constants, you will have to continue using the ctfe representation as they might not be representable using valtrees. Constants in the type system, i.e. array lengths or arguments for const parameters, will all be representable using valtrees but the compiler has to be able to convert them back to the ctfe representation anyways as they might be used as values by const eval. I think printing mir constants for rustdoc isn't great, but it seems unavoidable if you want to print associated consts. Also using that routine for type system consts is better than having two routines, one for valtrees and one for associated consts. The approach therefore feels appropriate to me as long as the rustdoc team has to capacity to maintain this printing routine. |
Ok, thanks for approving it. This is now something we need to discuss with @fmease and the other rustdoc team members then. |
☔ The latest upstream changes (presumably #100511) made this pull request unmergeable. Please resolve the merge conflicts. |
6ef267a
to
85c0d9e
Compare
☔ The latest upstream changes (presumably #100595) made this pull request unmergeable. Please resolve the merge conflicts. |
So we already have two approvals from here. The discussion on zulip seems stalled so do you think it's okay to merge this as a first step @rust-lang/rustdoc ? |
This PR is currently blocked by an RFC I am in the process of writing as per Zulip discussion. @rustbot blocked |
I don't think it makes sense to keep this PR open, it's too bit-rotten. Further, this feature won't land as is anyway but hopefully as part of a more general and flexible mechanism dubbed |
In rustdoc, we now attempt to display arbitrary constant values (e.g. structs, arrays). However, we do so intelligently by not showing private or
#[doc(hidden)]
fields and by creating hyperlinks to the definitions of structs, enums, enum variants, and struct & variant fields. Further, we won't show overly long arrays and (byte) string literals. Instead, we will display a placeholder.As an aside, a constant value is an evaluated constant expression.
Fixes #98929.
Fixes #99630.
I expect that the UI changes will need some tweaking over time. Consider them an MVP.
The basic framework of the formatter is borrowed from the pretty-printers
pretty_print_const{,value}
inrustc_middle
.For now, we only pretty-print
rustc_middle::ty::Const
s and MIRConstValue
s. Valtrees are just displayed as_
as I don'tthink they surface in rustdoc at the moment since anonymous constants / constant arguments are not evaluated at the time of this writing (see #82852) and just stored as
hir::Expr
s.@rustbot label T-rustdoc A-rustdoc-ui
CC @jsha
r? @GuillaumeGomez
You can browse the generated docs of
src/test/rustdoc/const-value.rs
here (points of major interest:Struct
andProtocol
).