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: emit JS paths for struct-like variants #64724

Closed
wants to merge 2 commits into from

Conversation

tomjakubowski
Copy link
Contributor

On the backend, rustdoc now emits paths entries to a crate's search
index for struct-like enum variants, and index items of type structfield
which belong to such variants point to their variant parents in the
paths table, rather than their enum grandparents. The path entry for
a variant is the fully qualified module path plus the enum name.

On the frontend, the search code recognizes structfields belonging to
structlike variants in the paths table and re-constructs the URL to
the field's anchor on the enum documentation page.

closes #16017

On the backend, rustdoc now emits `paths` entries to a crate's search
index for struct-like enum variants, and index items of type structfield
which belong to such variants point to their variant parents in the
`paths` table, rather than their enum grandparents.  The path entry for
a variant is the fully qualified module path plus the enum name.

On the frontend, the search code recognizes structfields belonging to
structlike variants in the `paths` table and re-constructs the URL to
the field's anchor on the enum documentation page.

closes rust-lang#16017
@tomjakubowski
Copy link
Contributor Author

Before and after, same test crate.

before
Wrong display path, wrong URL (note extra Foo).

after
Right display path, right URL.

#![no_std]
#![crate_type = "lib"]

pub mod xxx {
    pub enum Foo {
        TupleVrnt(u8),
        Vrnt { baz: i32 },
        Vrnt2 { quux: f32 },
    }

    pub struct SFoo {
        pub sbaz: (),
    }
}

I'd like to add tests for this, but I could use help with how to write them. There are three major changes:

  1. Variants themselves are now emitted to the paths table when they have structfield children. It's hard to write this test with the only tool I know, a compiletest like @has search-index.js, because the test should verify that the variant name specifically appear in the paths table (without these changes, the variant's name would still appear in the search index items).
  2. Such variant-residing structfield search index items now point to the variant as their parent, rather than to the enum. Same problem as above - hard to test for this with @has search-index.js.
  3. The search code's render function buildHrefAndPath() is fixed to handle the peculiar structfield-variant case. I haven't ever written a test for the rustdoc frontend so I wouldn't know where to begin. src/test/rustdoc-js seems to be specialized for testing search queries.

@Mark-Simulacrum
Copy link
Member

For e.g. core or std, how much of a size increase is this?

@tomjakubowski
Copy link
Contributor Author

I'll measure to be sure, but was careful (at least, tried to be!) to make this "zero cost"* in size of search-index.js for crates that don't use struct variants, and I believe that core/std only has tuple-like variants in public enums.

*: Rustdoc prunes entries from the paths table that are unreferenced by search index items, i.e., any variants which don't have structfields shouldn't appear in the paths table.

@bors
Copy link
Contributor

bors commented Sep 25, 2019

☔ The latest upstream changes (presumably #64751) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 25, 2019
@tomjakubowski
Copy link
Contributor Author

tomjakubowski commented Sep 26, 2019

I ran ./x.py doc --stage 1 for fb74e62 and the master branch commit I based on, eb48d6b.

% tar tzvf docs-my-patch.tar.gz | grep search-index                                       
-rw-r--r-- tom/tom     1495263 2019-09-25 17:02 build/x86_64-unknown-linux-gnu/doc/search-index1.39.0.js

% tar tzvf docs-master-eb48d6bdee6c655d71f26594d47d232adf3e4e93.tar.gz | grep search-index
-rw-r--r-- tom/tom     1495143 2019-09-26 13:15 build/x86_64-unknown-linux-gnu/doc/search-index1.39.0.js

So the search index has a size increase of 220 bytes or 0.015%.

@tomjakubowski
Copy link
Contributor Author

The extra bytes come from adding a path for this variant: https://doc.rust-lang.org/nightly/std/collections/enum.TryReserveError.html#variant.AllocError

@Mark-Simulacrum
Copy link
Member

That size increase looks entirely reasonable :)

Thanks for investigating!

@JohnCSimon
Copy link
Member

JohnCSimon commented Oct 5, 2019

Ping from triage.
This PR has sat idle for the last 9 days
@tomjakubowski can you please address the merge conflict?
CC @Mark-Simulacrum @QuietMisdreavus
Thank you!

@JohnCSimon
Copy link
Member

Pinging again from triage.
This PR has sat idle for the last 15 days
@tomjakubowski can you please address the merge conflict?
CC @Mark-Simulacrum @QuietMisdreavus
Thank you!

@joelpalmer joelpalmer added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 21, 2019
@joelpalmer
Copy link

Ping from Triage: Closing due to inactivity. @tomjakubowski please reopen with any updates. Thanks for the PR.

@joelpalmer joelpalmer closed this Oct 21, 2019
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 1, 2020
…-search, r=kinnison

Struct variant field search

Fixes rust-lang#16017.

Reopening of rust-lang#64724.

cc @tomjakubowski
cc @ollie27

r? @kinnison
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Feb 1, 2020
…-search, r=kinnison

Struct variant field search

Fixes rust-lang#16017.

Reopening of rust-lang#64724.

cc @tomjakubowski
cc @ollie27

r? @kinnison
bors added a commit that referenced this pull request Feb 15, 2020
…ollie27

Struct variant field search

Fixes #16017.

Reopening of #64724.

cc @tomjakubowski
cc @ollie27

r? @kinnison
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc: Struct variant fields don't have the right path in the search index
6 participants