Stream: rustdoc

Topic: issues #32077


view this post on Zulip Quy Nguyen (Jan 03 2021 at 02:50):

Hello, I've recently run into #32077, and I wanted to see what I could do to help fix it. Looking at the issue, I see it's blocked on #14072, but I'm not really sure where to get started on this.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:30):

@Quy Nguyen I am unclear what's going on in that issue. I modified the example in the OP:

pub struct Event<T>;

/// a key was pressed/released
pub type KeyPressEvent = Event<usize>;

impl KeyPressEvent {
    /// The keycode (a number representing a physical key on the keyboard) of the key
    /// which was pressed.
    pub fn detail(&self) {}

    /// Time when the event was generated (in milliseconds).
    pub fn time(&self) {}
}

which rendered this documentation:
image.png
which looks right to me. What is the bug you're running into?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:31):

someone mentioned a further bug that rustdoc doesn't follow type aliases, but I see https://github.com/rust-lang/rust/pull/42027 which was merged and claimed to fix it

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:31):

so I am unclear what the actual bug is

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:33):

ok, so one difference is that Event is public in my code but not the OP - is that what you mean?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:33):

what can you actually do with a public alias to a private type?

view this post on Zulip Quy Nguyen (Jan 03 2021 at 03:51):

#42027 fixed that simple case, but not this

pub struct Event<T>;
/// a key was pressed/released
pub type KeyPressEvent = Event<usize>;

impl<T> Event<T> {
    /// The keycode (a number representing a physical key on the keyboard) of the key
    /// which was pressed.
    pub fn detail(&self) {}

    /// Time when the event was generated (in milliseconds).
    pub fn time(&self) {}
}

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:53):

ok, the issue is that the documentation appears on Event but not KeyPressEvent, because the impl block used the name Event<T>

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:53):

unfortunately that probably means you won't be able to reuse much of the code from the simple fix

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:54):

I would see how Deref docs are rendered, that does something similar where it looks at accessible associated items instead of just on the current item

view this post on Zulip Joshua Nelson (Jan 03 2021 at 03:59):

I can't speak to whether a fix for this would be accepted without first fixing https://github.com/rust-lang/rust/issues/14072

view this post on Zulip Quy Nguyen (Jan 03 2021 at 04:13):

Hmm funnily enough this works

pub struct FooA<T>;
pub type FooB = FooA<usize>;

impl<T> FooA<T> {
    pub fn happy(&self) {}
}

impl FooB {
    pub fn happyer(&self) {}
}


pub struct Bar;
impl std::ops::Deref for Bar {
    type Target = FooB;
    fn deref(&self) -> &FooB { unimplemented!() }
}

view this post on Zulip Quy Nguyen (Jan 03 2021 at 04:13):

But the regular docs don't render

view this post on Zulip Joshua Nelson (Jan 03 2021 at 04:14):

(btw you can mark the code block as ```rust and it will highlight as rust)

view this post on Zulip Joshua Nelson (Jan 03 2021 at 04:16):

Quy Nguyen said:

Hmm funnily enough this works

honestly that doesn't suprise me too much - rustdoc has two ways that it collects data, one through the HIR ('what syntax you used') and one through metadata ('what is available to downstream crates'). Deref uses metadata, which normalizes a lot of things to be consistent.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 04:17):

so that's one possible way to fix this, use metadata for rendering info on type aliases

view this post on Zulip Quy Nguyen (Jan 03 2021 at 06:06):

Did the changes so you don't have to build the compiler go through? Also, what's the difference (code-wise) between the two? Everything is stuffed into the clean::types module according to the docs.

view this post on Zulip GuillaumeGomez (Jan 03 2021 at 13:17):

Quy Nguyen said:

Did the changes so you don't have to build the compiler go through? Also, what's the difference (code-wise) between the two? Everything is stuffed into the clean::types module according to the docs.

It's still in progress. We are also working on reducing the size of the clean::types module (by relying more and more upon the rustc internals API. For the moment, we achieved great improvements in both speed and memory usage. About the difference betweeen HIR and clean, the second is a "wrapper" over the first to put things simply. It makes manipulating HIR items simpler from rustdoc point of view.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 15:19):

Quy Nguyen said:

Did the changes so you don't have to build the compiler go through?

you can track that at https://github.com/rust-lang/rust/pull/79540

view this post on Zulip Quy Nguyen (Jan 03 2021 at 20:02):

How do I turn a DefId into the metadata definition?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:03):

what do you mean by 'metadata definition'?

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:04):

DefId is the metadata, in some sense

view this post on Zulip Quy Nguyen (Jan 03 2021 at 20:04):

There's a trait represented as a ResolvedPath, and I need the original definition of the trait to replace the generics.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:05):

https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TyCtxt.html#method.trait_def

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:05):

(most things are on TyCtxt)

view this post on Zulip Quy Nguyen (Jan 03 2021 at 20:27):

Does the TyCtxt in SharedCtxt work? It's not used anywhere in Rustdoc.

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:29):

SharedCtxt is probably too late - this is in render, right? right now all the computation happens in clean

view this post on Zulip Joshua Nelson (Jan 03 2021 at 20:30):

and render should just be for actually generating the HTML, not looking up what the types are

view this post on Zulip Quy Nguyen (Jan 04 2021 at 03:28):

How do I get a runnable rustdoc off your fork? Just x.py build and pay the tax?

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:29):

you mean from the no-xpy branch? git fetch origin pull/79540/head && git rebase -i FETCH_HEAD, then set [rust] download-stage1 = true in config.toml

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:29):

and then x.py should work like normal

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:30):

(no-xpy is now a misnomer but github doesn't allow renaming branches without closing the PR)

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:33):

oh when you rebase make sure to drop all commits except your own, git gets confused and tries to apply all commits to master since I rebased the PR last

view this post on Zulip Quy Nguyen (Jan 04 2021 at 03:36):

downloading https://ci-artifacts.rust-lang.org/rustc-builds/a44774c3a9739b2eea8923e09d67b14312c78ef3/rust-std-nightly-x86_64-pc-windows-msvc.tar.xz failed with 404 - did I screw up my git or is that build not published?

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:37):

hmm, that's weird

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:37):

oh wow that is a long way back

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:37):

you need to rebase to a commit that's less than 6 months out or the artifacts will have been deleted

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:38):

so I would guess you probably did something strange with git, yeah

view this post on Zulip Joshua Nelson (Jan 04 2021 at 03:39):

merge-base with my PR and master is bb178237c5539c75e1b85ab78a8ab902b1f333d5 (1 week ago) so it's not left over from my experimenting with old commits

view this post on Zulip Quy Nguyen (Jan 04 2021 at 05:18):

Nice, got it working. What's the mystery boolean in TypeDefItem?

view this post on Zulip Joshua Nelson (Jan 04 2021 at 05:18):

whether it's an associated item, apparently

view this post on Zulip Joshua Nelson (Jan 04 2021 at 05:18):

that bit is still mostly a mystery to me

view this post on Zulip Joshua Nelson (Jan 04 2021 at 05:19):

but I did make some minor cleanups: https://github.com/rust-lang/rust/pull/80661

view this post on Zulip Quy Nguyen (Jan 04 2021 at 05:47):

Does Rustdoc expand type aliases? I know HIR wise it doesn't, but whenever it inlines from another crate does it expand it then?

view this post on Zulip Joshua Nelson (Jan 04 2021 at 05:58):

Right, so that's what I found: ty::Ty automatically expands aliases but hir::Ty does not

view this post on Zulip Joshua Nelson (Jan 04 2021 at 05:59):

So the fix in all those places was to use tcx.type_of or hir_ty_to_ty

view this post on Zulip Quy Nguyen (Jan 04 2021 at 07:31):

So really I just need to substitute every generic with the actual parameter. I could modify the cleaned structs in place, but that might mess with what the json backend or other parts of Rustdoc assume. The other way is to store the (raw, unsubstituted) generics and sub them in during render time, but that'll likely bloat memory use.

view this post on Zulip Joshua Nelson (Jan 04 2021 at 13:48):

I would start by modifying in place and see what breaks

view this post on Zulip Joshua Nelson (Jan 04 2021 at 13:49):

I would hope rustdoc isn't depending on the names for anything

view this post on Zulip Quy Nguyen (Jan 05 2021 at 23:12):

Hmm so I can't modify in place because not all of the items are available to me then, for instance if I impl a trait and don't override any defaults, all the MethodItems are stored on the trait and not actually on the ImplItem.

view this post on Zulip Quy Nguyen (Jan 05 2021 at 23:15):

Althoguh the implemntation also splices stuff in from Deref impls, so I'd also need to keep where I get the methods for those too. Probably going to do traits first as most everything is self contained tho.

view this post on Zulip Joshua Nelson (Jan 06 2021 at 03:14):

@Quy Nguyen can you do this at the same time these items are generated? Then you don't have to modify them after the fact

view this post on Zulip Quy Nguyen (Jan 08 2021 at 20:48):

Default method implemntations for traits aren't contained in an impl, so there's no way for me to change them without cloning everything. I'm just gonna try substitution at render time for now.


Last updated: Oct 11 2021 at 22:34 UTC