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 dead_code "never constructed" warning on 1.80.0 for a struct where the constructor takes &'static self #128272

Closed
chrisnc opened this issue Jul 27, 2024 · 11 comments · Fixed by #128404
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@chrisnc
Copy link
Contributor

chrisnc commented Jul 27, 2024

Code

/* XXX: changing Foo::init to take self: &'static Foo instead of
 * &'static self silences the dead_code warning. */
pub struct Foo {
    x: i32,
}

impl Foo {
    #[must_use]
    //pub const fn init(self: &'static Foo, x: i32) -> Foo {
    pub const fn init(&'static self, x: i32) -> Foo {
        Foo { x }
    }

    pub fn foo(&self) -> i32 {
        self.x
    }
}

#[cfg(test)]
mod test {
    use crate::Foo;

    #[test]
    fn test() {
        static FOO: Foo = FOO.init(7);
        assert_eq!(FOO.foo(), FOO.x);
    }
}

Current output

warning: struct `Foo` is never constructed
 --> src/lib.rs:3:12
  |
3 | pub struct Foo {
  |            ^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: `playground` (lib) generated 1 warning

Desired output

No output.

Rationale and extra context

This problem does not occur with 1.79.0. https://play.rust-lang.org/?version=beta&mode=debug&edition=2021&gist=ad761b02ac5d86f28629e7fa2d6a1936

This is a reduced example from real code where objects are constructed in such a way that they need static references to fields within the same object (and those objects themselves will be static). It's clearly possible to construct such an object, so the dead_code warning is not accurate.

Other cases

Changing the parameter to be self: &'static Foo instead silences the warning.

Rust Version

$ rustc --version --verbose
rustc 1.80.0 (051478957 2024-07-21)
binary: rustc
commit-hash: 051478957371ee0084a7c0913941d2a8c4757bb9
commit-date: 2024-07-21
host: aarch64-apple-darwin
release: 1.80.0
LLVM version: 18.1.7
@chrisnc chrisnc added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 27, 2024
@chrisnc chrisnc changed the title false-positive dead_code "never constructed" warning on 1.80.0 for a struct where the constructor takes &'static self false positive dead_code "never constructed" warning on 1.80.0 for a struct where the constructor takes &'static self Jul 27, 2024
@tgross35
Copy link
Contributor

tgross35 commented Jul 27, 2024

How do you wind up with the &'static self in the original code, is it via FFI? If so, #126169 / #127104 may be relevant. (If this was 1.80 then it happend before that, similar code though).

@chrisnc
Copy link
Contributor Author

chrisnc commented Jul 27, 2024

The example I pasted shows how the &'static self is obtained: static FOO: Foo = FOO.init(7);
This part matches exactly what the original code was doing. I'm not using any repr attribute in this example so I'm not sure it's related to those issues (I did notice those while searching for dups). The warning should fire for Foo if the init function didn't exist, but init constructs Foo.

@tgross35
Copy link
Contributor

Completely missed the example, thanks. This is a pretty weird case considering self is the parameter that doesn't get used.

@tgross35 tgross35 added the C-bug Category: This is a bug. label Jul 27, 2024
@chrisnc
Copy link
Contributor Author

chrisnc commented Jul 27, 2024

In the real code the &'static self does get used of course, but as I was minimizing it I found that it wasn't necessary to see the warning. The 'static itself is not necessary to see the warning either, but in this case I think the only way to call init in safe rust is with a static, so I included it for clarity.

@chrisnc
Copy link
Contributor Author

chrisnc commented Jul 27, 2024

Some bisecting shows that this is caused by #125572 just like the others, but this is a distinct case. I see there's some discussion about whether the FFI-constructed cases should be using some other annotation to avoid the warning which is otherwise accurate for safe Rust code, but in this instance, the liveness analysis for safe Rust is not accurate.

@theemathas
Copy link
Contributor

Minimal reproduction: a library crate containing the following code

pub struct Foo(&'static Foo);  // The field is included for demonstration of how this could be useful

impl Foo {
    pub const fn chain(&'static self) -> Foo {
        Foo(self)
    }
}

// An external crate could construct Foo by writing:
// static X: Foo = X.chain();
Compiler output on nightly 2024-07-12
   Compiling playground v0.0.1 (/playground)
warning: struct `Foo` is never constructed
 --> src/lib.rs:1:12
  |
1 | pub struct Foo(&'static Foo);  // The field is included for demonstration of how this could be useful
  |            ^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: `playground` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.47s

@gurry
Copy link
Contributor

gurry commented Jul 28, 2024

A self-contained reproducer:

pub struct Foo(&'static Foo);  // The field is included for demonstration of how this could be useful
    
impl Foo {
    pub const fn chain(&'static self) -> Foo {
        Foo(self)
    }
}

fn main() {
    static _X: Foo = _X.chain();
}

@gurry
Copy link
Contributor

gurry commented Jul 28, 2024

@rustbot claim

@mu001999
Copy link
Contributor

In the real code the &'static self does get used of course

@chrisnc Hi, I'm curious about what do you need an uninitialized self (&'static self) for?

@chrisnc
Copy link
Contributor Author

chrisnc commented Jul 29, 2024

@mu001999 the original comment gives some explanation:

This is a reduced example from real code where objects are constructed in such a way that they need static references to fields within the same object (and those objects themselves will be static).

The reference is not uninitialized (nor is the object, it's statically initialized, self-referentially).

@mu001999
Copy link
Contributor

@chrisnc oh sorry, I missed it. i think i understand ;)

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 31, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc `@mu001999`
r? `@pnkfelix`

Fixes rust-lang#128272
Fixes rust-lang#126169
tgross35 added a commit to tgross35/rust that referenced this issue Aug 1, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc `@mu001999`
r? `@pnkfelix`

Fixes rust-lang#128272
Fixes rust-lang#126169
tgross35 added a commit to tgross35/rust that referenced this issue Aug 1, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc `@NobodyXu` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting `@mu001999's` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc `@mu001999`
r? `@pnkfelix`

Fixes rust-lang#128272
Fixes rust-lang#126169
jieyouxu added a commit to jieyouxu/rust that referenced this issue Aug 2, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ``@NobodyXu`` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ``@mu001999's`` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc ``@mu001999``
r? ``@pnkfelix``

Fixes rust-lang#128272
Fixes rust-lang#126169
jieyouxu added a commit to jieyouxu/rust that referenced this issue Aug 2, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ```@NobodyXu``` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ```@mu001999's``` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc ```@mu001999```
r? ```@pnkfelix```

Fixes rust-lang#128272
Fixes rust-lang#126169
jieyouxu added a commit to jieyouxu/rust that referenced this issue Aug 3, 2024
…hanges, r=pnkfelix

Revert recent changes to dead code analysis

This is a revert to recent changes to dead code analysis, namely:
* efdf219 Rollup merge of rust-lang#128104 - mu001999-contrib:fix/128053, r=petrochenkov
* a70dc29 Rollup merge of rust-lang#127017 - mu001999-contrib:dead/enhance, r=pnkfelix
* 31fe962 Rollup merge of rust-lang#127107 - mu001999-contrib:dead/enhance-2, r=pnkfelix
* 2724aea Rollup merge of rust-lang#126618 - mu001999-contrib:dead/enhance, r=pnkfelix
* 977c5fd Rollup merge of rust-lang#126315 - mu001999-contrib:fix/126289, r=petrochenkov
* 13314df Rollup merge of rust-lang#125572 - mu001999-contrib:dead/enhance, r=pnkfelix

There is an additional change stacked on top, which suppresses false-negatives that were masked by this work. I believe the functions that are touched in that code are legitimately unused functions and the types are not reachable since this `AnonPipe` type is not publically reachable -- please correct me if I'm wrong cc ````@NobodyXu```` who added these in #rust-lang#127153.

Some of these reverts (rust-lang#126315 and rust-lang#126618) are only included because it makes the revert apply cleanly, and I think these changes were only done to fix follow-ups from the other PRs?

I apologize for the size of the PR and the churn that it has on the codebase (and for reverting ````@mu001999's```` work here), but I'm putting this PR up because I am concerned that we're making ad-hoc changes to fix bugs that are fallout of these PRs, and I'd like to see these changes reimplemented in a way that's more separable from the existing dead code pass. I am happy to review any code to reapply these changes in a more separable way.

cc ````@mu001999````
r? ````@pnkfelix````

Fixes rust-lang#128272
Fixes rust-lang#126169
@bors bors closed this as completed in 1f47624 Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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
5 participants