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

rustdoc: rich-text and visibility-aware formatting for constant values #98929

Open
fmease opened this issue Jul 5, 2022 · 6 comments
Open

rustdoc: rich-text and visibility-aware formatting for constant values #98929

fmease opened this issue Jul 5, 2022 · 6 comments
Assignees
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@fmease
Copy link
Member

fmease commented Jul 5, 2022

While types in rustdoc are richly formatted using HTML (most notably for hyperlinks to definitions),
constant expressions and values do not enjoy any such treatment and are printed as plain text (or
sometimes even entirely omitted from the documentation).
With the continuously growing language support for “const” (const fn, const generics, adt_const_params,
generic_const_exprs, const_trait_impl) which allows ever-more-complex constant expressions in
more and more places, it seems very fitting to improve rustdoc's output to help users navigate this complexity.

I propose the following:

  • We start showing the constant value of free const items no matter their type. At present, only integers, bools and string literals are displayed.
  • We try to use constant values as returned by tcx.const_eval_poly as much as possible for pretty-printing and only fall back on hir::Expr (unevaluated expressions) as a last resort. Currently for free and assoc. consts, rustdoc evaluates arbitrary const expressions of any type (arbitrary ADTs) but throws away the result if the type is not an integer (or similar) and instead prints the unevaluated expression.
  • We try to evaluate const arguments and pretty-print the const value. Right now, we just print their unevaluated form, a hir::Expr.
  • Regarding the formatting / pretty-printing of const values (ConstValue / ValTrees eventually (?)):
    • We print struct and enum literals (as one would expect). Examples: S { f: 0 }, Ok(())
    • However, we also hide private and doc(hidden) fields.
      In non-tuple structs, we skip those fields and add the symbol .. (mirroring the syntax of struct patterns).
      In tuple structs, we replace them with _ (just how it's already done today in the code blocks at the top of tuple struct pages)
    • We make paths and struct fields hyperlinks to their definition.
    • We print a placeholder like [/* N elements */] for large arrays to not clutter the documentation
    • We print a placeholder like maybe /* large string */ for large strings
    • ( Maybe we should also introduce a max-depth parameter similar to the one rustdoc has for types )

Unfortunately rustdoc's current reliance on const_eval_poly means that in a quite a lot of cases – namely those where the const expr contains type or const parameters (including the implicit type parameter Self) –, we aren't gonna get the benefits of this proposed richer pretty-printing since the evaluation will fail on those with the error “too generic“ and we'll have to fall back on printing the unevaluated hir::Expr. This is a known limitation of rustdoc and tracked in #82852.

Parts of this proposal aim to solve #97933 properly whereas #98814 is but a hotfix.
I have already partially implemented this proposal in a local patch that is yet to be published as a PR. You can get a visual taste of it below:

Stable Rustdoc Output Nightly Rustdoc Output (#98814) WIP Implementation of this Proposal
ppconst_stable ppconst_after_hotfix ppconst_linkified

@rustbot label T-rustdoc A-rustdoc-ui C-enhancement
@rustbot claim

@rustbot rustbot added A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 5, 2022
@jsha
Copy link
Contributor

jsha commented Jul 6, 2022

This seems like a good idea to me! Can you link to some real world crates with examples of complex const values (especially structs) so we can see if the proposed display works well for them? I'm thinking maybe structs should be split out onto multiple lines. AFAIK there is nowhere else in rustdoc that we try to cram them onto a single line and it seems like it could get crowded fast.

@jsha
Copy link
Contributor

jsha commented Jul 6, 2022

Oh, also, I think this is the main question:

We try to use constant values as returned by tcx.const_eval_poly as much as possible for pretty-printing and only fall back on hir::Expr (unevaluated expressions) as a last resort. Currently for free and assoc. consts, rustdoc evaluates arbitrary const expressions of any type (arbitrary ADTs) but throws away the result if the type is not an integer (or similar) and instead prints the unevaluated expression.

Or to put it another way: should we display what the doc author wrote, or what the compiler sees?

For instance, say the doc author writes:

pub struct MyThing {
  pub x: i32,
  pub y: i32,
  pub z: i32,
  pub name: String,
  ... many fields ...
}

impl const Default for MyThing { ... }

pub const EMPTY: MyThing = MyThing::default()

Should the docs show:

  • pub const EMPTY: MyThing = MyThing::default(), or
  • pub const EMPTY: MyThing = MyThing { x: 0, y: 0, z: 0, name: "" ... }

@GuillaumeGomez
Copy link
Member

I'd say the second. MyThing::default() doesn't provide any useful information after all.

@fmease
Copy link
Member Author

fmease commented Jul 23, 2022

Can you link to some real world crates with examples of complex const values

Sadly I don't know of any such crate. Maybe bnum? See bnum::BInt.
I must admit that my proposal is merely based on assumptions and speculations.

Or to put it another way: should we display what the doc author wrote, or what the compiler sees?

This is a very fair question. Obviously I lean towards printing evaluated constants but deep down I wish constants could be handled in a more nuanced way: I imagine a system where consts and const fns can be marked as “abstract” in which case they are not fully evaluated (in the documentation). That marker could very well be pub itself but I'd like it not to be.
I'd like to draw a parallel with rustdoc's current handling of private vs public type aliases (where public type aliases are not normalized / reduced / evaluated). In some sense, it's the same system.
I am aware that this type alias system does not work cross-crate atm but I've noticed @GuillaumeGomez works towards fixing this, right? Of course, it's another huge story to apply this to constants as well since their representation in rustc and rustdoc is completely different.

Re. pub vs. a potential #[abstract] or #[opaque] (ignoring syntax). I've seen some discussions on Zulip by the const-eval / const-generics WG (?) around introducing the concept of “abstract” (or “opaque”) constants that don't reduce outside the crate they are defined in. That would then of course apply to rustdoc, too. Prior art are dependently-typed languages like Idris or Agda. Idris makes a distinction between export (pseudo Rust: #[abstract] pub) and public export (Rust: pub). Adga offers something similar. This would also give users more control over the issues I've recently described here (tl;dr: the value of public constants and the body of public const fns are part of the API; with #[abstract] you could opt out of this (or we make pub consts abstract by default in a new edition)). I plan on discussing this again in the const-eval Zulip stream at some point.


Now, I think I will go ahead and bring my stalled WIP PR up to speed and just open it for you to discuss. Enough time has passed I think (sorry). Esp. since apparently the hotfix wasn't enough to cover it all: #99630.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jul 23, 2022

I am aware that this type alias system does not work cross-crate atm but I've noticed @GuillaumeGomez works towards fixing this, right? Of course, it's another huge story to apply this to constants as well since their representation in rustc and rustdoc is completely different.

Yes. #97974 is the WIP PR.

Now, I think I will go ahead and bring my stalled WIP PR up to speed and just open it for you to discuss. Enough time has passed I think (sorry). Esp. since apparently the hotfix wasn't enough to cover it all: #99630.

I think it's a good approach. That will at least allow us to have a UI we can actually go through that can be used to be debated upon.

@jsha
Copy link
Contributor

jsha commented Jul 24, 2022

Or to put it another way: should we display what the doc author wrote, or what the compiler sees?
Obviously I lean towards printing evaluated constants but deep down I wish constants could be handled in a more nuanced way: I imagine a system where consts and const fns can be marked as “abstract” in which case they are not fully evaluated (in the documentation).

I can't claim to have the definitive answer here but I lean towards displaying what the doc author wrote. This makes it possible to express either semantic. If the doc author wants to hide the details of a complex object, they write a const fn that evaluates to the thing they want, and use that in the assignment. If the doc author wants to show the details, they can evaluate it once and write an object literal in the assignment. No need for new syntax to tell rustdoc whether to show or hide the details.

core::f64::consts provides some useful examples to think about. Would FRAC_1_SQRT_2 be better expressed as 1 / sqrt(2) or as 0.707106781186547524400844362104849039_f64 like it is today? Should TAU be expressed as 2 * PI? (I'm guessing for numerical precision reasons we might prefer literals for these, so consider it a thought experiment rather than a proposal for change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants