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

Move significant_drop_in_scrutinee into nursey #9302

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

Jarcho
Copy link
Contributor

@Jarcho Jarcho commented Aug 8, 2022

The current suggestion of extending the lifetime of every sub-expression is not great and doesn't fix the error given in the lint's example, though it does make the potential deadlock easier to see, but it can also cause it's own issues by delaying the drop of the lock guard.

e.g.

match x.lock().foo {
    ..
}
// some stuff
let y = x.lock();

The suggestion would create a deadlock at the second x.lock() call.

This also lints even when a significant drop type isn't created as a temporary. (#9072)

I agree @kpreid (#8987 (comment)) that this should be back-ported before the lint hits stable.

changelog: Move significant_drop_in_scrutinee into nursey

@rust-highfive
Copy link

r? @llogiq

(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 Aug 8, 2022
@flip1995
Copy link
Member

flip1995 commented Aug 8, 2022

The Vec::drain bug was already fixed AFAIK. The for loop message was improved.

But yeah the suggestion of just moving it in front of the loop/match has other deadlock potential. This should probably be addressed by scoping the match block.

That it warns even if the mutex is not used inside the loop/match was intentional, but maybe should also be revisited.

But moving it back to nursery SGTM. Beta was already branched though. I try to backport it anyway.

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 8, 2022

📌 Commit aa0b0af has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 8, 2022

⌛ Testing commit aa0b0af with merge 97a0cf2...

@bors
Copy link
Collaborator

bors commented Aug 8, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 97a0cf2 to master...

@bors bors merged commit 97a0cf2 into rust-lang:master Aug 8, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2022
…Simulacrum

[stable] 1.63.0 release

Includes cherry picks of:

* rust-lang#100207
* rust-lang/rust-clippy#9302
*  Avoid ICE in rustdoc when using Fn bounds rust-lang#100205

r? `@Mark-Simulacrum`
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 9, 2022
…mulacrum

[beta] 1.64.0 branching

Includes cherry picks of:

* rust-lang#100207
* rust-lang/rust-clippy#9302
* rust-lang/rust@49b1904 (explicit_auto_deref into nursery)
*  Avoid ICE in rustdoc when using Fn bounds rust-lang#100205

r? `@Mark-Simulacrum`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants