Skip to content

Emit if_let_mutex in presence of other mutexes #13174

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 2 commits into from
Jul 29, 2024

Conversation

y21
Copy link
Member

@y21 y21 commented Jul 28, 2024

Currently (master, not nightly nor stable) if_let_mutex does not emit a warning here:

let m1 = Mutex::new(10);
let m2 = Mutex::new(());

if let 100..=200 = *m1.lock().unwrap() {
  m2.lock();
} else {
  m1.lock();
}

It currently looks for the first call to .lock() on any mutex receiver inside of the if/else body, and only later (outside of the visitor) checks that the receiver matches the mutex in the scrutinee. That means that in cases like the above, it finds the m2.lock() expression, stops the visitor, fails the check that it's the same mutex (m2 != m1) and then does not look for any other .lock() calls.

So, just make the receiver check also part of the visitor so that we only stop the visitor when we also find the right receiver.

The first commit has the actual changes described here. The sceond one just unnests all the if lets


changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Jul 28, 2024

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

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

llogiq commented Jul 29, 2024

Looks good to me. Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2024

📌 Commit 61dcf6c has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 29, 2024

⌛ Testing commit 61dcf6c with merge 834b691...

@bors
Copy link
Contributor

bors commented Jul 29, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 834b691 to master...

@bors bors merged commit 834b691 into rust-lang:master Jul 29, 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.

4 participants