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

Some(_) is not equivalent to is_some() #5746

Closed
hayashi-stl opened this issue Jun 24, 2020 · 3 comments · Fixed by #6568
Closed

Some(_) is not equivalent to is_some() #5746

hayashi-stl opened this issue Jun 24, 2020 · 3 comments · Fixed by #6568
Labels
A-documentation Area: Adding or improving documentation C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on good-first-issue These issues are a good way to get started with Clippy

Comments

@hayashi-stl
Copy link

I found a situation in which if let Some(_) = func() and if func().is_some() don't do the same thing. In particular, the latter deadlocks while the former doesn't. This is relevant for the redundant_pattern_matching lint. I replicated this behavior in debug and release mode with rustc 1.44.0.

Here is the situation

@flip1995
Copy link
Member

Playground so it can be run directly.

I think the best thing you can do here is to just allow the lint in this case. This will probably be a really hard fix, but we should definitely document this.

@flip1995 flip1995 added E-hard Call for participation: This a hard problem and requires more experience or effort to work on good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing A-documentation Area: Adding or improving documentation labels Jun 24, 2020
@ghost
Copy link

ghost commented Jun 25, 2020

The way I see this...
if let Some(_) = func() keeps the clone around until the end of the block which the code gets to finish the drop.
if let func.is_some() immediately drops the clone. Now the drop runs for the clone you just created but the cache lock hasn't been released yet.
I'm not 100% sure to be honest....

According to the mutext docs "The exact behavior on locking a mutex in the thread which already holds the lock is left unspecified. However, this function will not return on the second call (it might panic or deadlock, for example)." If you're going to lock in a drop and make clones you need to be really careful.

In any case, maybe we can check if the type of func() implements drop and not lint in this case.

@ghost
Copy link

ghost commented Jun 25, 2020

Thinking a little more. It would have to be a temporary value that implements drop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation C-bug Category: Clippy is not doing the correct thing E-hard Call for participation: This a hard problem and requires more experience or effort to work on good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants