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

drop_ref should not trigger when drop is used in a match branch #10122

Closed
ia0 opened this issue Dec 28, 2022 · 1 comment · Fixed by #10142
Closed

drop_ref should not trigger when drop is used in a match branch #10122

ia0 opened this issue Dec 28, 2022 · 1 comment · Fixed by #10142
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@ia0
Copy link

ia0 commented Dec 28, 2022

Summary

This bug is similar to #9482 but for drop_ref instead of drop_copy.

I would similarly argue that if drop() is called on the result of a function (or other expression with side-effects) as a match branch (i.e. the code contains something like => drop(expr_with_side_effects),) then the drop_ref lint should not trigger because drop() is not used to drop the value but to ignore the value.

The fix may be similar to #9491.

Lint Name

drop_ref

Reproducer

I tried this code:

fn foo(xs: &mut [u8; 3]) -> &mut u8 {
    println!("doing foo");
    &mut xs[2] // result is not always useful, the side-effect matters
}
fn bar() {
    println!("doing bar");
}

fn main() {
    let mut xs = *b"abc";
    match 42 {
        0 => drop(foo(&mut xs)),  // drop is needed because we only care about side-effects
        1 => bar(),
        _ => (),  // doing nothing (no side-effects needed here)
    }
}

I saw this happen:

error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing
  --> src/main.rs:12:14
   |
12 |         0 => drop(foo(&mut xs)),  // drop is needed because we only care about side-effects
   |              ^^^^^^^^^^^^^^^^^^
   |
note: argument has type `&mut u8`
  --> src/main.rs:12:19
   |
12 |         0 => drop(foo(&mut xs)),  // drop is needed because we only care about side-effects
   |                   ^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref

I expected to see this happen:

No error

Version

rustc 1.67.0-nightly (c97b539e4 2022-11-30)
binary: rustc
commit-hash: c97b539e408ea353f4fde2f9251d598291fec421
commit-date: 2022-11-30
host: x86_64-unknown-linux-gnu
release: 1.67.0-nightly
LLVM version: 15.0.4

Additional Labels

No response

@ia0 ia0 added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Dec 28, 2022
@bors bors closed this as completed in 4f4c961 Jan 5, 2023
@ia0
Copy link
Author

ia0 commented Jan 6, 2023

Thanks @ericwu2003 for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant