rust-lang / rust-lang/rust

dead_code suggestion for unused fields does not account for changes to auto trait implementations

Open
#119,645 14 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-lints C-discussion T-compiler T-lang
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

I tried this code:

use std::rc::Rc;

struct Pending(Rc<()>);

fn pending() -> Pending {
    Pending(Rc::new(()))
}

fn main() {
    let _ = pending();
}

(The original code: https://github.com/rust-lang/futures-rs/blob/72e7e397cdfe7574e9b5d8845cafefc6e5dda70a/futures-executor/tests/local_pool.rs#L13)

I expected to see this happen: no warning or a warning suggests Pending(PhantomData<Rc<()>>).

Instead, this happened:

A warning suggests Pending(()).

error: field `0` is never read
  --> futures-executor/tests/local_pool.rs:13:16
   |
13 | struct Pending(Rc<()>);
   |        ------- ^^^^^^
   |        |
   |        field in this struct
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`
help: consider changing the field to be of unit type to suppress this warning while preserving the field numbering, or remove the field
   |
13 | struct Pending(());
   |                ~~

Changing Rc<()> to () changes auto trait implementations of Pending, so dead_code should not suggest such a code or should suggest both code (PhantomData<Rc<()>> and ()) with explanations.

Also, if it suggests PhantomData, it should also be noted that PhantomData is not able to propagate all auto-traits (e.g., Unpin). (although this is not a problem in the above case) EDIT: see https://github.com/rust-lang/rust/issues/119645#issuecomment-1879509605

https://github.com/rust-lang/futures-rs/actions/runs/7428509435/job/20215931270

Meta

rustc --version --verbose:

rustc 1.77.0-nightly (595bc6f00 2024-01-05)
binary: rustc
commit-hash: 595bc6f00369475047538fdae1ff8cea692ac385
commit-date: 2024-01-05
host: aarch64-apple-darwin
release: 1.77.0-nightly
LLVM version: 17.0.6

This has been shown in the latest nightly (nightly-2024-01-06) since https://github.com/rust-lang/rust/pull/118297 (cc @shepmaster). IMO, it should not be changed to be warned by default with such a lint with a wrong suggestion.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Rust reproducer and the diagnostic shown for futures-executor/tests/local_pool.rs:13, then investigate the dead_code suggestion for an unused field. Compare the suggested unit-field change with the field's auto-trait behavior; done means the compiler avoids an incorrect suggestion or presents an alternative that preserves the relevant behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.