-
Notifications
You must be signed in to change notification settings - Fork 73
MCP: Resolve documentation links in rustc and store the results in metadata #584
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Closed
1 of 3 tasks
Labels
major-change
A proposal to make a major change to rustc
major-change-accepted
A major change proposal that was accepted
T-compiler
Add this label so rfcbot knows to poll the compiler team
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed. cc @rust-lang/compiler @rust-lang/compiler-contributors |
@rustbot second |
petrochenkov
added a commit
to petrochenkov/rust
that referenced
this issue
Feb 10, 2023
This commit implements MCP rust-lang/compiler-team#584 It also removes code that is no longer used, and that includes code cloning resolver, so issue rust-lang#83761 is fixed.
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 11, 2023
Resolve documentation links in rustc and store the results in metadata This PR implements MCP rust-lang/compiler-team#584. Doc links are now resolved in rustc and stored into metadata, so rustdoc simply retrieves them through a query (local or extern), Code that is no longer used is removed, and some code that no longer needs to be public is privatized. The removed code includes resolver cloning, so this PR fixes rust-lang#83761.
@rustbot label -final-comment-period +major-change-accepted |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Labels
major-change
A proposal to make a major change to rustc
major-change-accepted
A major change proposal that was accepted
T-compiler
Add this label so rfcbot knows to poll the compiler team
Proposal
What are doc links
Documentation links or intra-doc links are things like this:
Rustdoc extracts such links when processing doc comments (or
#[doc = "string"]
attributes), resolves Rust paths in them and generates HTML links to corresponding path destinations in the produced documentation.How rustdoc resolves them
Rustdoc resolves paths in the links
rustc_resolve
.Both ways are bad, mostly due to doc inlining.
Rustdoc has a thing called doc inlining which may insert an arbitrary set of items from dependency crates into documentation of the current crate.
So we have something like "rustdoc hygiene" very similar to macro 2.0 hygiene that makes it work.
In fact it works exactly through the partially implemented macro 2.0 support in the compiler, but it's pretty buggy (e.g. preludes are not supported correctly).
See rustdoc: Stop cloning the resolver rust#83761 for the history here.
Proposal
Resolve doc links during regular name resolution in
rustc_resolve
and store the results to metadata, so rustdoc can just use the readily available results for the current or any dependency crate, instead of doing the resolution itself.Pros:
Cons:
The currently used library is
pulldown-cmark
, it's MIT licensed and should be compatible with rustc.rustc_resolve
- doc comment gluing and whitespace tweaking, backtick/prefix/suffix stripping, path generic argument stripping (sort of emulation ofparse_path
fromrustc_parse
).The logic is currently about 300 lines total, and can be found in
compiler/rustc_resolve/src/rustdoc.rs
in Resolve documentation links in rustc and store the results in metadata rust#94857.The benchmarked prototype wasn't optimized in any way, and I'm pretty sure we can get this number to sub 1% (or even lower if some more filtering logic is moved from rustdoc to
rustc_resolve
).Mentors or Reviewers
@GuillaumeGomez @oli-obk
Process
The main points of the Major Change Process are as follows:
@rustbot second
.-C flag
, then full team check-off is required.@rfcbot fcp merge
on either the MCP or the PR.You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
The text was updated successfully, but these errors were encountered: