Skip to content

Commit

Permalink
[drop_ref]: don't lint idiomatic in match arm
Browse files Browse the repository at this point in the history
  • Loading branch information
ericwu17 committed Jan 2, 2023
1 parent a85e480 commit 61a4389
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
let is_copy = is_copy(cx, arg_ty);
let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr);
let (lint, msg) = match fn_name {
sym::mem_drop if arg_ty.is_ref() => (DROP_REF, DROP_REF_SUMMARY),
sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => (DROP_REF, DROP_REF_SUMMARY),
sym::mem_forget if arg_ty.is_ref() => (FORGET_REF, FORGET_REF_SUMMARY),
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => (DROP_COPY, DROP_COPY_SUMMARY),
sym::mem_forget if is_copy => (FORGET_COPY, FORGET_COPY_SUMMARY),
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/drop_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,26 @@ fn test_owl_result_2() -> Result<u8, ()> {
produce_half_owl_ok().map(drop)?;
Ok(1)
}

#[allow(unused)]
#[allow(clippy::unit_cmp)]
fn issue10122(x: u8) {
// This is a function which returns a reference and has a side-effect, which means
// that calling drop() on the function is considered an idiomatic way of achieving the side-effect
// in a match arm.
fn println_and<T>(t: &T) -> &T {
println!("foo");
t
}

match x {
0 => drop(println_and(&12)), // Don't lint (copy type), we only care about side-effects
1 => drop(println_and(&String::new())), // Don't lint (no copy type), we only care about side-effects
2 => {
drop(println_and(&13)); // Lint, even if we only care about the side-effect, it's already in a block
},
3 if drop(println_and(&14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
4 => drop(2), // Lint, not a fn/method call
_ => (),
}
}
39 changes: 38 additions & 1 deletion tests/ui/drop_ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -107,5 +107,42 @@ note: argument has type `&SomeStruct`
LL | std::mem::drop(&SomeStruct);
| ^^^^^^^^^^^

error: aborting due to 9 previous errors
error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing
--> $DIR/drop_ref.rs:89:13
|
LL | drop(println_and(&13)); // Lint, even if we only care about the side-effect, it's already in a block
| ^^^^^^^^^^^^^^^^^^^^^^
|
note: argument has type `&i32`
--> $DIR/drop_ref.rs:89:18
|
LL | drop(println_and(&13)); // Lint, even if we only care about the side-effect, it's already in a block
| ^^^^^^^^^^^^^^^^

error: calls to `std::mem::drop` with a reference instead of an owned value. Dropping a reference does nothing
--> $DIR/drop_ref.rs:91:14
|
LL | 3 if drop(println_and(&14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
| ^^^^^^^^^^^^^^^^^^^^^^
|
note: argument has type `&i32`
--> $DIR/drop_ref.rs:91:19
|
LL | 3 if drop(println_and(&14)) == () => (), // Lint, idiomatic use is only in body of `Arm`
| ^^^^^^^^^^^^^^^^

error: calls to `std::mem::drop` with a value that implements `Copy`. Dropping a copy leaves the original intact
--> $DIR/drop_ref.rs:92:14
|
LL | 4 => drop(2), // Lint, not a fn/method call
| ^^^^^^^
|
note: argument has type `i32`
--> $DIR/drop_ref.rs:92:19
|
LL | 4 => drop(2), // Lint, not a fn/method call
| ^
= note: `#[deny(clippy::drop_copy)]` on by default

error: aborting due to 12 previous errors

0 comments on commit 61a4389

Please sign in to comment.