Skip to content

Move rustdoc::clean::utils::find_nearest_parent_module to TyCtxt #80740

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
wants to merge 2 commits into from

Conversation

camelid
Copy link
Member

@camelid camelid commented Jan 6, 2021

find_nearest_parent_module was extracted from
rustdoc::passes::collect_intra_doc_links in #80368.

We (me and Joshua Nelson) thought at the time that it should be in rustc
instead, and Joshua suggested it be a method on TyCtxt.
However, since #80368 was fixing a significant regression that was about
to land on beta, we agreed that to be able to fix this quickly I should
leave the code in rustdoc.

This is a followup PR to move it to TyCtxt.

cc @jyn514

`find_nearest_parent_module` was extracted from
`rustdoc::passes::collect_intra_doc_links` in rust-lang#80368.

We (me and Joshua Nelson) thought at the time that it should be in rustc
instead, and Joshua suggested it be a method on `TyCtxt`.
However, since rust-lang#80368 was fixing a significant regression that was about
to land on beta, we agreed that to be able to fix this quickly I should
leave the code in rustdoc.

This is a followup PR to move it to `TyCtxt`.
@camelid camelid added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 6, 2021
@rust-highfive
Copy link
Contributor

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive
Copy link
Contributor

r? @ecstatic-morse

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 6, 2021
@camelid
Copy link
Member Author

camelid commented Jan 6, 2021

I think ecstatic-morse is no longer a reviewer?

r? @petrochenkov (re-assign if you want someone else to review)

@camelid
Copy link
Member Author

camelid commented Jan 6, 2021

Note that there's a somewhat similar method in rustc_resolve, however it seems it must be duplicated because resolve doesn't have a TyCtxt (see #77984 (comment)#77984 is the PR that added Resolver::nearest_mod_parent).

@camelid
Copy link
Member Author

camelid commented Jan 6, 2021

Probably someone on T-rustdoc should review the rustdoc changes.

@jyn514
Copy link
Member

jyn514 commented Jan 6, 2021

I would prefer not to have the unrelated doc changes here, but I don't feel strongly about it.

@camelid
Copy link
Member Author

camelid commented Jan 6, 2021

I would prefer not to have the unrelated doc changes here, but I don't feel strongly about it.

I moved that commit to a new PR: #80744

In general it's assumed that compiler APIs may panic on fake IDs.
@petrochenkov
Copy link
Contributor

This functionality is very specific and is not used outside of rustdoc, so it would be better to keep it in rustdoc, rustc_middle is already too large.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 7, 2021
rustdoc: Turn `next_def_id` comments into docs

Split out from rust-lang#80740.

r? `@jyn514`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 7, 2021
rustdoc: Turn `next_def_id` comments into docs

Split out from rust-lang#80740.

r? ``@jyn514``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants