Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

False positive for dropping_copy_types #112653

Closed
ia0 opened this issue Jun 15, 2023 · 3 comments
Closed

False positive for dropping_copy_types #112653

ia0 opened this issue Jun 15, 2023 · 3 comments
Assignees
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ia0
Copy link
Contributor

ia0 commented Jun 15, 2023

The false positive detection done in rust-lang/rust-clippy#9491 is actually too restrictive. The test is whether drop is used with the following pattern: pat => drop(fun(args, ...)). In particular, this doesn't match pat => drop(expr) which would be more general and cover things like pat => drop(fun(args, ...)?) (notice the ?).

Here's an example where the false positive triggers:

fn foo() -> Result<u8, ()> {
    println!("doing foo");
    Ok(0) // result is not always useful, the side-effect matters
}
fn bar() {
    println!("doing bar");
}

pub fn stuff() -> Result<(), ()> {
    match 42 {
        0 => drop(foo()?),  // drop is needed because we only care about side-effects
        1 => bar(),
        _ => (),  // doing nothing (no side-effects needed here)
    }
    Ok(())
}
warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
  --> src/lib.rs:11:14
   |
11 |         0 => drop(foo()?),  // drop is needed because we only care about side-effects
   |              ^^^^^------^
   |                   |
   |                   argument has type `u8`
   |
   = note: use `let _ = ...` to ignore the expression or result
   = note: `#[warn(dropping_copy_types)]` on by default
@ia0 ia0 added the C-bug Category: This is a bug. label Jun 15, 2023
@Noratrieb Noratrieb added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 1, 2023
@Noratrieb
Copy link
Member

cc @Urgau

@Urgau
Copy link
Member

Urgau commented Jul 1, 2023

@rustbot claim

workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 2, 2023
…, r=Nilstrieb

Fix `dropping_copy_types` lint from linting in match-arm with side-effects

This PR fixes an issue with the `dropping_copy_types` and `dropping_references` lints when not all patterns that can have side-effects were detected and ignored.

Nearly _fixes_ rust-lang#112653 (will need beta-backport to completely fix the issue)
r? `@Nilstrieb`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 2, 2023
…, r=Nilstrieb

Fix `dropping_copy_types` lint from linting in match-arm with side-effects

This PR fixes an issue with the `dropping_copy_types` and `dropping_references` lints when not all patterns that can have side-effects were detected and ignored.

Nearly _fixes_ rust-lang#112653 (will need beta-backport to completely fix the issue)
r? ``@Nilstrieb``
@Noratrieb
Copy link
Member

It was fixed and backported.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants