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

Fix ICE in Definitions::create_def #99340

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

GoldsteinE
Copy link
Contributor

@GoldsteinE GoldsteinE commented Jul 16, 2022

Debug implementation for LocalDefId uses global Definitions. Normally it’s ok, but we can’t do it while holding a mutable reference to Definitions, since it causes ICE or deadlock (depending on whether parallel_compiler is enabled).

This PR effectively copies the Debug implementation into the problematic method. I don’t particularly love this solution (since it creates code duplication), but I don’t see any other options.

This issue was discovered when running rustdoc with RUSTDOC_LOG=trace on the following file:

pub struct SomeStruct;

fn asdf() {
    impl SomeStruct {
        pub fn qwop(&self) {
            println!("hidden function");
        }
    }
}

I’m not sure how to create a test for this behavior.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 16, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 16, 2022
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@GoldsteinE GoldsteinE force-pushed the fix-localdefid-debug-ice branch from 5d7200d to 6ac17f0 Compare July 31, 2022 18:45
@rust-log-analyzer

This comment has been minimized.

@lcnr
Copy link
Contributor

lcnr commented Aug 1, 2022

thanks 👍

@bors r+ rollup squash

@bors
Copy link
Contributor

bors commented Aug 1, 2022

📌 Commit b85c5f0417585cf77d8613a7d6d2e691da328aa5 has been approved by lcnr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2022
@klensy
Copy link
Contributor

klensy commented Aug 1, 2022

thanks 👍

@bors r+ rollup squash

Can't find exact PR, but recently r+ squash was tried and it didn't worked in rollup.

@GoldsteinE GoldsteinE force-pushed the fix-localdefid-debug-ice branch from b85c5f0 to d9f28b7 Compare August 1, 2022 13:16
@GoldsteinE
Copy link
Contributor Author

GoldsteinE commented Aug 1, 2022

I manually squashed the commits, could someone re-approve this?

@lcnr
Copy link
Contributor

lcnr commented Aug 1, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 1, 2022

📌 Commit d9f28b7 has been approved by lcnr

It is now in the queue for this repository.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#99340 (Fix ICE in Definitions::create_def)
 - rust-lang#99629 (Improve `cannot move out of` error message)
 - rust-lang#99864 (bootstrap: don't emit warn about duplicated deps with same/different features if some of sets actually empty)
 - rust-lang#99911 (Remove some uses of `guess_head_span`)
 - rust-lang#99976 (Make Rustdoc exit with correct error code when scraping examples from invalid files)
 - rust-lang#100003 (Improve size assertions.)
 - rust-lang#100012 (Avoid `Ty` to `String` conversions)
 - rust-lang#100020 (better error when python is not found in x - issue rust-lang#99648)

Failed merges:

 - rust-lang#99994 (Replace `guess_head_span` with `opt_span`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 58042bf into rust-lang:master Aug 1, 2022
@rustbot rustbot added this to the 1.64.0 milestone Aug 1, 2022
@GoldsteinE GoldsteinE deleted the fix-localdefid-debug-ice branch August 1, 2022 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants