Skip to content

Fix mut_mutex_lock when reference not ultimately mutable #13122

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

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

rshearman
Copy link
Contributor

@rshearman rshearman commented Jul 18, 2024

When there is are multiple references where one of the references isn't mutable then this results in a false-positive for mut_mutex_lock as it only checks the mutability of the first reference level.

Fix this by using peel_mid_ty_refs_is_mutable which correctly determines whether the reference is ultimately mutable and thus whether Mutex::get_lock() can actually be used.

Fixes #9854

changelog: [mut_mutex_lock]: No longer lints if the mutex is behind multiple references and one of those references isn't mutable

@rustbot
Copy link
Collaborator

rustbot commented Jul 18, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @blyxyas (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 18, 2024
@blyxyas
Copy link
Member

blyxyas commented Jul 20, 2024

I've just put the source code that the reporter talks about in a test case, and it seems to give the same error in this branch. Could you add a regression test and work off of it? 🌈

@bors
Copy link
Contributor

bors commented Sep 22, 2024

☔ The latest upstream changes (presumably #13440) made this pull request unmergeable. Please resolve the merge conflicts.

When there is are multiple references where one of the references
isn't mutable then this results in a false-positive for
`mut_mutex_lock` as it only checks the mutability of the first
reference level.

Fix this by using `peel_mid_ty_refs_is_mutable` which correctly
determines whether the reference is ultimately mutable and thus
whether `Mutex::get_lock()` can actually be used.

Fixes rust-lang#9854
@blyxyas
Copy link
Member

blyxyas commented Oct 1, 2024

Sorry for the delay, I've corrected the merge conflict and this should be ready to merge!

If everything is green and [lintcheck] doesn't show anything weird, this will be merged today! ❤️

@blyxyas
Copy link
Member

blyxyas commented Oct 1, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 1, 2024

📌 Commit c88cb08 has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 1, 2024

⌛ Testing commit c88cb08 with merge d578f6a...

@bors
Copy link
Contributor

bors commented Oct 1, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing d578f6a to master...

@bors bors merged commit d578f6a into rust-lang:master Oct 1, 2024
8 checks passed
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.

clippy::mut_mutex_lock after an immutable reference is held
4 participants