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's treatment of cross-crate ("imported") files is suboptimal. #70757

Open
eddyb opened this issue Apr 4, 2020 · 4 comments
Open

rustdoc's treatment of cross-crate ("imported") files is suboptimal. #70757

eddyb opened this issue Apr 4, 2020 · 4 comments
Assignees
Labels
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

@eddyb
Copy link
Member

eddyb commented Apr 4, 2020

@bjorn3 noted in #70025 (comment) that single files missing disables [src] links for the entire crate.
That's not ideal UX, we should render as many source files as we can possible load.


And there's also the paths rustdoc uses (see #70025 (comment)), e.g.:

If we use the CrateNum of the original source file and reuse the existing rustdoc logic to get the relative path of that file in that crate, I believe we can even support linking to the source correctly even when the absolute paths are not usable (e.g. /rustc/$hash/src/libstd/thread/local.rs when using thread_local! and the rust-src rustup component isn't installed)

Because the downstream crate uses an exported macro from the original crate, the rendered source file should always have been generated, so we should never really need to re-generate it.

cc @rust-lang/rustdoc @Aaron1011

@eddyb eddyb added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 4, 2020
@bjorn3
Copy link
Member

bjorn3 commented Apr 4, 2020

Because the downstream crate uses an exported macro from the original crate, the rendered source file should always have been generated, so we should never really need to re-generate it.

It is possible to document a single crate.

@eddyb
Copy link
Member Author

eddyb commented Apr 4, 2020

@bjorn3 I think then they get generated at the same paths but by the current rustdoc (this logic already exists, it's just only used for the "cross-crate doc inlining mode", not "cross-crate source files").

@jonas-schievink jonas-schievink added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Apr 4, 2020
@ollie27
Copy link
Member

ollie27 commented Apr 5, 2020

#70642 may have fixed the compiler docs but it didn't fix the underlying regression caused by #66364. rustdoc is now trying to load files belonging to external crates which even without --remap-path-prefix, it won't always have access to those files.

If we use the CrateNum of the original source file and reuse the existing rustdoc logic to get the relative path of that file in that crate

Yeah, that look like it's the easiest way to actually fix the regression.

I've submitted #70828 to fix this.

@ollie27 ollie27 added C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Apr 5, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 8, 2020
… r=eddyb

rustdoc: Don't try to load source files from external crates

Local items defined in external macros shouldn't generate rendered source files and should link to the external crate's docs instead.

Part of rust-lang#70757

r? @GuillaumeGomez
cc @eddyb
@ollie27 ollie27 self-assigned this Apr 9, 2020
@ollie27 ollie27 added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Apr 9, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 27, 2020

For future readers: this is mostly fixed but was left open to track the edge case in #70828 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

No branches or pull requests

5 participants