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

False positive on dead_code lint #68408

Closed
martinthomson opened this issue Jan 20, 2020 · 6 comments · Fixed by #96896
Closed

False positive on dead_code lint #68408

martinthomson opened this issue Jan 20, 2020 · 6 comments · Fixed by #96896
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@martinthomson
Copy link

I'm sure that there is a more concise way to generate this error, but this produces a dead_code lint warning:

#![deny(warnings)]

use std::mem;

#[derive(Default)]
struct Y {}
enum X {
    A { y: Y },
    B { y: Y },
}
impl X {
    fn test(&mut self) {
        if let Self::A { y } = self {
            *self = Self::B { y: mem::take(y) };
        }
    }
}
pub fn main() {
    let mut x = X::A { y: Y {} };
    x.test();
}

(Playground)

This code instantiates X::B, but the dead_code lint disagrees:

   Compiling playground v0.0.1 (/playground)
error: variant is never constructed: `B`
 --> src/main.rs:9:5
  |
9 |     B { y: Y },
  |     ^^^^^^^^^^
  |
note: lint level defined here
 --> src/main.rs:1:9
  |
1 | #![deny(warnings)]
  |         ^^^^^^^^
  = note: `#[deny(dead_code)]` implied by `#[deny(warnings)]`
@jonas-schievink jonas-schievink added 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. labels Jan 21, 2020
@hellow554
Copy link
Contributor

Something small-ish-er:

#![deny(dead_code)]

enum X {
    A,
    B { y: () },
}
impl X {
    fn test(&mut self) {
        *self = Self::B { y: () };
    }
}
fn main() {
    let mut x = X::A;
    x.test();
}

If we replace Self by X, then it will change to:

error: field is never used: `y`
 --> src/main.rs:5:9
  |
5 |     B { y: () },
  |         ^^^^^

which is indeed correct

@martinthomson
Copy link
Author

This is probably the most telling example:

#![deny(clippy::use_self)]
#![deny(dead_code)]

#[derive(Debug)]
enum X {
    A { _a: () },
    B { _b: () },
}
impl X {
    fn a() -> X {
        X::A { _a: () }
    }
    fn b() -> Self {
        Self::B { _b: () }
    }
}
pub fn main() {
    println!("{:?}", X::a());
    println!("{:?}", X::b());
}

playground

This doesn't compile (dead_code), and doesn't lint with clippy (clippy::use_self, likely due to rust-lang/rust-clippy#3410). The dead_code problem doesn't manifest if X::A is simple or a newtype (enum X { A(()), B(()) } is OK), it needs to be a struct.

@acfoltzer
Copy link

It looks like the erroneous warning arises only for struct-like enum variants that are created only by referencing via Self. In this example program, the only warning is for E::A:

// struct-like variant, referred to using Self

#[derive(Debug)]
enum E {
    A { x: () },
}

impl E {
    fn mk_e() -> Self {
        Self::A { x: () }
    }
}

// struct-like variant, referred to using type name

#[derive(Debug)]
enum F {
    B { x: () },
}

impl F {
    fn mk_f() -> Self {
        F::B { x: () }
    }
}

// tuple variant, referred to using Self

#[derive(Debug)]
enum G {
    C(()),
}

impl G {
    fn mk_g() -> Self {
        Self::C(())
    }
}

fn main() {
    dbg!(E::mk_e());
    dbg!(F::mk_f());
    dbg!(G::mk_g());
}
   Compiling playground v0.0.1 (/playground)
warning: variant is never constructed: `A`
 --> src/main.rs:5:5
  |
5 |     A { x: () },
  |     ^^^^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.43s
     Running `target/debug/playground`
[src/main.rs:41] E::mk_e() = A {
    x: (),
}
[src/main.rs:42] F::mk_f() = B {
    x: (),
}
[src/main.rs:43] G::mk_g() = C(
    (),
)

(playground)

@molenzwiebel
Copy link

These seem to have been fixed. The above playground link has no warnings.

@hellow554
Copy link
Contributor

searched nightlies: from nightly-2020-01-26 to nightly-2020-08-03
regressed nightly: nightly-2020-05-03
searched commits: from 7f65393 to f05a524
regressed commit: dae90c1

bisected with cargo-bisect-rustc v0.5.2

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --access github --regress success 

How is that related? 😅 But nevertheless: @rustbot modify labels: E-needs-test

@rustbot rustbot added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Aug 4, 2020
@camelid
Copy link
Member

camelid commented Oct 26, 2020

@hellow554 That bisection may be because of a bug in cargo-bisect-rustc that was fixed in the latest release; see rust-lang/cargo-bisect-rustc#98.

JohnTitor added a commit to JohnTitor/rust that referenced this issue May 10, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 10, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue May 11, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#96543 (Remove hacks in `make_token_stream`.)
 - rust-lang#96887 (rustdoc: correct path to type alias methods)
 - rust-lang#96896 (Add regression test for rust-lang#68408)
 - rust-lang#96900 (Fix js error)
 - rust-lang#96903 (Use lifetimes on type-alias-impl-trait used in function signatures to infer output type lifetimes)
 - rust-lang#96916 (simplify length count)
 - rust-lang#96925 (Fix issue rust-lang#95151)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in 7bf795b May 11, 2022
@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants