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

dead_code lint does not trigger for tuple structs with unread fields #92790

Closed
shepmaster opened this issue Jan 11, 2022 · 12 comments · Fixed by #95977
Closed

dead_code lint does not trigger for tuple structs with unread fields #92790

shepmaster opened this issue Jan 11, 2022 · 12 comments · Fixed by #95977
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@shepmaster
Copy link
Member

shepmaster commented Jan 11, 2022

This code:

fn main() {
    let _ = Example { u: 0 };
}

struct Example {
    u: u8,
}

Produces this warning:

warning: field is never read: `u`
 --> src/main.rs:6:5
  |
6 |     u: u8,
  |     ^^^^^
  |

However, switching to a tuple struct:

fn main() {
    let _ = Example(0);
}

struct Example(u8);

Does not report a warning.

Original report misleadingly focusing on Debug
fn main() {
    dbg!(Example { a: 0 });
}

#[derive(Debug)]
struct Example {
    a: u8,
}

produces

warning: field is never read: `a`
 --> src/main.rs:7:5
  |
7 |     a: u8,
  |     ^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

However, switching to a tuple struct:

fn main() {
    dbg!(Example(0));
}

#[derive(Debug)]
struct Example(u8);

does not produce the warning.

/cc @FabianWolff
/cc #85200; #84647; #88900

Meta

Rustc 1.57.0 and 1.60.0-nightly (2022-01-10 89b9f7b)

@shepmaster shepmaster added the C-bug Category: This is a bug. label Jan 11, 2022
@FabianWolff
Copy link
Contributor

This has nothing to do with the #[derive(Debug)]:

fn main() {
    let _ = Example { u: 0 };
}

struct Example {
    u: u8,
}
warning: field is never read: `u`
 --> src/main.rs:6:5
  |
6 |     u: u8,
  |     ^^^^^
  |

but

fn main() {
    let _ = Example(0);
}

struct Example(u8);

gives no warning.

@shepmaster shepmaster changed the title dead_code lint does not ignore Debug for tuple structs dead_code lint does not trigger for tuple structs with unread fields Jan 11, 2022
@shepmaster
Copy link
Member Author

@FabianWolff oh dear. Thanks! I've updated the title and original post; sorry for the false ping.

@camelid camelid added A-lint 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. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 12, 2022
@camelid
Copy link
Member

camelid commented Jan 12, 2022

Not a regression AFAIK, but seems like worth prioritizing.

@shepmaster
Copy link
Member Author

Not a regression AFAIK

I agree. I did a quick test across Rust 1.{0,10,20,30,40,50}.0 and none of them reported the lint.

@camelid
Copy link
Member

camelid commented Jan 12, 2022

Thanks for checking!

@FabianWolff
Copy link
Contributor

This behavior does indeed date back to the very beginning of the detection of dead struct fields (commit 0271224 in 2014). The culprit is line 621 in dead.rs:

fn should_warn_about_field(&mut self, field: &hir::FieldDef<'_>) -> bool {
let def_id = self.tcx.hir().local_def_id(field.hir_id);
let field_type = self.tcx.type_of(def_id);
!field.is_positional()
&& !self.symbol_is_live(def_id)
&& !field_type.is_phantom_data()
&& !has_allow_dead_code_or_lang_attr(self.tcx, field.hir_id)
}

Commenting out this line with the above tuple struct example gives:

warning: field is never read: `0`
 --> s2.rs:6:16
  |
6 | struct Example(u8);
  |                ^^
  |

I can open a PR for this, or is there a reason anyone here can think of why dead tuple struct fields shouldn't get a warning?

@rustbot claim

@camelid
Copy link
Member

camelid commented Jan 12, 2022

I can't think of one, so opening a PR seems good.

@danielhenrymantilla
Copy link
Contributor

FWIW, I find the example with dbg! to be surprising: dbg! is debug-printing a struct, and thus reading its fields

@shepmaster
Copy link
Member Author

@danielhenrymantilla see #85200; #84647; #88900

@apiraino
Copy link
Contributor

Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.

@rustbot label -I-prioritize +P-medium

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 13, 2022
@rcls
Copy link

rcls commented Feb 21, 2022

is there a reason anyone here can think of why dead tuple struct fields shouldn't get a warning?

One thought is that deleting a dead tuple field may be non-trivial, if it is not the last field. E.g., imagine if someone got:

struct DeadLive(u8,u8);
...
warning: field is never read: `0`

You can't simply delete the dead field. You have to also renumber all the uses of .1, which might be a non-trivial and error prone task. So the cost v. benefit tradeoff of having a warning is different to the named-field case.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Aug 2, 2022
… r=estebank

Warn about dead tuple struct fields

Continuation of rust-lang#92972. Fixes rust-lang#92790.

The language team has already commented on this in rust-lang#92972 (comment); I have incorporated their requests here. Specifically, there is now a new allow-by-default `unused_tuple_struct_fields` lint (name bikesheddable), and fields of unit type are ignored (rust-lang#92972 (comment)), so error messages look like this:
```
error: field is never read: `1`
  --> $DIR/tuple-struct-field.rs:6:21
   |
LL | struct Wrapper(i32, [u8; LEN], String);
   |                     ^^^^^^^^^
   |
help: change the field to unit type to suppress this warning while preserving the field numbering
   |
LL | struct Wrapper(i32, (), String);
   |                     ~~
```
r? `@joshtriplett`
@bors bors closed this as completed in 9bbbf60 Aug 5, 2022
@shepmaster
Copy link
Member Author

Thank you @FabianWolff !

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