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

Allow to disable auto and blanket trait impls retrieval in rustdoc if it is in parallel_compiler mode #107121

Closed
wants to merge 1 commit into from
Closed

Allow to disable auto and blanket trait impls retrieval in rustdoc if it is in parallel_compiler mode #107121

wants to merge 1 commit into from

Conversation

GuillaumeGomez
Copy link
Member

Part of #106745.

I was asking for @oli-obk to explain to me what they meant in #106745 (comment) and they explained to me that the current issue for parallel_compiler seemed to come from the auto/blanket impls retrieval in rustdoc. So this PR proposes to simply disable it in this case.

I'm not sure if you need this to be merged (or if it's even a good idea to merge it in the first place) but at least you have a commit you can use to try to move forward on this feature.

cc @m-ou-se

@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 2023

r? @notriddle

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 20, 2023
@m-ou-se
Copy link
Member

m-ou-se commented Jan 20, 2023

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned notriddle Jan 20, 2023
@m-ou-se m-ou-se mentioned this pull request Jan 20, 2023
Closed
oli-obk
oli-obk reviewed Jan 20, 2023
View reviewed changes
src/librustdoc/clean/utils.rs
Comment on lines +478 to +480
// FIXME: To be removed once `parallel_compiler` bugs are fixed!
// More information in <https://github.com/rust-lang/rust/pull/106745>.
if !cfg!(parallel_compiler) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably just make this an early return to reduce churn when we remove it again.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 20, 2023

This doesn't seem to work. If I add this change to #106745, set parallel-compiler = true and run ./x.py doc --stage 2 compiler/rustc_middle, it still gets stuck on rustc_middle(doc). (But maybe I'm testing the wrong thing?)

Noratrieb
Noratrieb reviewed Jan 20, 2023
View reviewed changes
src/librustdoc/clean/utils.rs
.run(|| BlanketImplFinder { cx }.get_blanket_impls(item_def_id));
auto_impls.into_iter().chain(blanket_impls)
// FIXME: To be removed once `parallel_compiler` bugs are fixed!
// More information in <https://github.com/rust-lang/rust/pull/106745>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you link #106930 instead?

@m-ou-se m-ou-se mentioned this pull request Jan 20, 2023
Open
46 tasks
@m-ou-se
Copy link
Member

m-ou-se commented Jan 24, 2023

cfg(parallel_compiler) doesn't work in librustdoc. It seems to work with m-ou-se@809eec5

I wonder if we'll just run into failing tests for the parallel compiler builder. (Maybe it doesn't run the tests?)

@m-ou-se m-ou-se mentioned this pull request Jan 24, 2023
Closed
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jan 24, 2023
@m-ou-se m-ou-se force-pushed the rustdoc-disable-on-parallel_compiler branch from 5805f04 to 01d15b4 Compare January 24, 2023 14:04
@oli-obk
Copy link
Contributor

oli-obk commented Jan 25, 2023

We found a workaround and are debugging this issue in parallel

@oli-obk oli-obk closed this Jan 25, 2023
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-disable-on-parallel_compiler branch January 25, 2023 10:06
@nnethercote
Copy link
Contributor

@oli-obk Any progress? A workaround was found for #106745, but #104754 is still blocked on the problem :(

@oli-obk
Copy link
Contributor

oli-obk commented Feb 2, 2023

Nothing new since my last comment in #106930

@lqd lqd mentioned this pull request Feb 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Noratrieb Noratrieb Noratrieb left review comments

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@GuillaumeGomez @rustbot @m-ou-se @oli-obk @nnethercote @Noratrieb @notriddle