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.

Sign up for GitHub

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

Jump to bottom

rustdoc: inline all the impls #33133

Merged
merged 3 commits into from
Apr 26, 2016
Merged

rustdoc: inline all the impls #33133

merged 3 commits into from
Apr 26, 2016

Conversation

mitaa
Copy link
Contributor

@mitaa mitaa commented Apr 21, 2016

This used to be done to avoid inlining impls referencing private items, but is now unnecessary since we actually check that impls do not reference non-doc-reachable items.

fixes #32881
fixes #33025
fixes #33113

r? @alexcrichton

@petrochenkov
Copy link
Contributor

we actually check that impls do not reference non-doc-reachable items

I'm curious, is this done with a separate check somewhere in rustdoc?
Because exactly this information is gathered in rustc_privacy and returned by access_levels.is_doc_reachable(impl_id)

@mitaa
Copy link
Contributor Author

mitaa commented Apr 21, 2016 •
edited
Loading

That was introduced in #33002. AFAIU the access_levels gathered in rustc_privacy are only for the local crate - so rustdoc tries to also probe the crate metadata in a similar way (i.e. also takes doc(hidden) into account) to librustc_privacy::EmbargoVisitor and adds that to access_levels.

edit: What I meant with

we actually check that impls do not reference non-doc-reachable items

was that the impl trait and type are checked, nothing more.

@alexcrichton
Copy link
Member

@bors: r+ 8c302a00223abb46a2f42e5331fc27d1b1d35e1a

Nice!

@bors
Copy link
Contributor

bors commented Apr 24, 2016

⌛ Testing commit 8c302a0 with merge 326dd17...

@bors
Copy link
Contributor

bors commented Apr 24, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

mitaa added 2 commits April 24, 2016 08:17
@mitaa
Inline impls on traits too
cc615b2
@mitaa
Remove, now unnecessary, workaround
dca7f01 
This used to be done to avoid inlining impls referencing private items,
but is now unnecessary since we actually check that impls do not
reference non-doc-reachable items.
@mitaa mitaa force-pushed the rdoc-smth-smth-impl branch from 8c302a0 to 01f7c74 Compare April 24, 2016 12:37
@mitaa
Copy link
Contributor Author

mitaa commented Apr 24, 2016

(updated)

@alexcrichton
Copy link
Member

Looks like there's still an error on Travis?

failures:

---- [rustdoc] rustdoc/inline_local/issue-32343.rs stdout ----

error: htmldocck failed!
status: exit code: 1
command: "/usr/bin/python2.7" "/home/travis/build/rust-lang/rust/src/etc/htmldocck.py" "x86_64-unknown-linux-gnu/test/rustdoc/inline_local/issue-32343.stage2-x86_64-unknown-linux-gnu" "/home/travis/build/rust-lang/rust/src/test/rustdoc/inline_local/issue-32343.rs"
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------
21: @has check failed
    `XPATH PATTERN` did not match
    // @has - '//code/a' 'Bar'

Encountered 1 errors

------------------------------------------

thread '[rustdoc] rustdoc/inline_local/issue-32343.rs' panicked at 'explicit panic', /home/travis/build/rust-lang/rust/src/tools/compiletest/src/runtest.rs:1597
note: Run with `RUST_BACKTRACE=1` for a backtrace.


failures:
    [rustdoc] rustdoc/inline_local/issue-32343.rs

@mitaa mitaa force-pushed the rdoc-smth-smth-impl branch from 01f7c74 to 3c3ee35 Compare April 24, 2016 18:12
@mitaa
Check reachability for inlined extern links too
6603c95 
An item is inlined and recorded as inlined even if it is
`doc(hidden)`, leading to unchecked external links.
@mitaa mitaa force-pushed the rdoc-smth-smth-impl branch from 3c3ee35 to 6603c95 Compare April 24, 2016 18:33
@mitaa
Copy link
Contributor Author

mitaa commented Apr 24, 2016

Sorry, I guess checking local item links is a bit of an issue, and unnecessary. Because of that I've changed to only checking extern items.

@alexcrichton
Copy link
Member

@bors: r+ 6603c95

@bors
Copy link
Contributor

bors commented Apr 25, 2016

⌛ Testing commit 6603c95 with merge a5b5cab...

@bors
Copy link
Contributor

bors commented Apr 25, 2016

💔 Test failed - auto-win-msvc-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Apr 25, 2016 at 4:32 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-msvc-32-opt
http://buildbot.rust-lang.org/builders/auto-win-msvc-32-opt/builds/3158

—
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#33133 (comment)

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 25, 2016
@Manishearth
Rollup merge of rust-lang#33133 - mitaa:rdoc-smth-smth-impl, r=alexcr…
5fba2bf 
…ichton

rustdoc: inline all the impls

This used to be done to avoid inlining impls referencing private items, but is now unnecessary since we actually check that impls do not reference non-doc-reachable items.

fixes rust-lang#32881
fixes rust-lang#33025
fixes rust-lang#33113

r? @alexcrichton
@Manishearth Manishearth mentioned this pull request Apr 25, 2016
Merged
bors added a commit that referenced this pull request Apr 25, 2016
@bors
Auto merge of #33204 - Manishearth:rollup, r=Manishearth
149cf77 
Rollup of 7 pull requests

- Successful merges: #33107, #33133, #33160, #33167, #33194, #33196, #33200
- Failed merges:
bors added a commit that referenced this pull request Apr 26, 2016
@bors
Auto merge of #33204 - Manishearth:rollup, r=Manishearth
41028de 
Rollup of 7 pull requests

- Successful merges: #33107, #33133, #33160, #33167, #33194, #33196, #33200
- Failed merges:
@bors bors merged commit 6603c95 into rust-lang:master Apr 26, 2016
@mitaa mitaa deleted the rdoc-smth-smth-impl branch April 26, 2016 08:59
@jethrogb jethrogb mentioned this pull request May 10, 2016
Merged
@jethrogb
Copy link
Contributor

This PR should be backported into beta, to fix this regression:
https://doc.rust-lang.org/stable/std/any/trait.Any.html
https://doc.rust-lang.org/beta/std/any/trait.Any.html

@jethrogb jethrogb mentioned this pull request May 10, 2016
Closed
Munksgaard added a commit to Munksgaard/rust that referenced this pull request Oct 11, 2018
@Munksgaard
Include rustdoc tests that have been fixed by rust-lang#33133
1de8f51 
There was an issue (rust-lang#33025) which caused these tests to not work. The issue has
since been fixed in rust-lang#33133, and so we can now include them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@mitaa @petrochenkov @alexcrichton @bors @jethrogb