Skip to content

Commit

Permalink
Warn about unused pub fields in non-pub structs
Browse files Browse the repository at this point in the history
  • Loading branch information
FabianWolff committed May 15, 2021
1 parent c6dd87a commit 46d55d6
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 22 deletions.
49 changes: 29 additions & 20 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ struct MarkSymbolVisitor<'tcx> {
repr_has_repr_c: bool,
in_pat: bool,
inherited_pub_visibility: bool,
pub_visibility: bool,
ignore_variant_stack: Vec<DefId>,
// maps from tuple struct constructors to tuple struct items
struct_constructors: FxHashMap<hir::HirId, hir::HirId>,
Expand Down Expand Up @@ -188,27 +189,33 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {

fn visit_node(&mut self, node: Node<'tcx>) {
let had_repr_c = self.repr_has_repr_c;
self.repr_has_repr_c = false;
let had_inherited_pub_visibility = self.inherited_pub_visibility;
let had_pub_visibility = self.pub_visibility;
self.repr_has_repr_c = false;
self.inherited_pub_visibility = false;
self.pub_visibility = false;
match node {
Node::Item(item) => match item.kind {
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
let def = self.tcx.adt_def(item.def_id);
self.repr_has_repr_c = def.repr.c();
Node::Item(item) => {
self.pub_visibility = item.vis.node.is_pub();

intravisit::walk_item(self, &item);
}
hir::ItemKind::Enum(..) => {
self.inherited_pub_visibility = item.vis.node.is_pub();
match item.kind {
hir::ItemKind::Struct(..) | hir::ItemKind::Union(..) => {
let def = self.tcx.adt_def(item.def_id);
self.repr_has_repr_c = def.repr.c();

intravisit::walk_item(self, &item);
}
hir::ItemKind::ForeignMod { .. } => {}
_ => {
intravisit::walk_item(self, &item);
intravisit::walk_item(self, &item);
}
hir::ItemKind::Enum(..) => {
self.inherited_pub_visibility = self.pub_visibility;

intravisit::walk_item(self, &item);
}
hir::ItemKind::ForeignMod { .. } => {}
_ => {
intravisit::walk_item(self, &item);
}
}
},
}
Node::TraitItem(trait_item) => {
intravisit::walk_trait_item(self, trait_item);
}
Expand All @@ -220,8 +227,9 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
}
_ => {}
}
self.repr_has_repr_c = had_repr_c;
self.pub_visibility = had_pub_visibility;
self.inherited_pub_visibility = had_inherited_pub_visibility;
self.repr_has_repr_c = had_repr_c;
}

fn mark_as_used_if_union(&mut self, adt: &ty::AdtDef, fields: &[hir::ExprField<'_>]) {
Expand Down Expand Up @@ -259,10 +267,10 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
) {
let has_repr_c = self.repr_has_repr_c;
let inherited_pub_visibility = self.inherited_pub_visibility;
let live_fields = def
.fields()
.iter()
.filter(|f| has_repr_c || inherited_pub_visibility || f.vis.node.is_pub());
let pub_visibility = self.pub_visibility;
let live_fields = def.fields().iter().filter(|f| {
has_repr_c || (pub_visibility && (inherited_pub_visibility || f.vis.node.is_pub()))
});
self.live_symbols.extend(live_fields.map(|f| f.hir_id));

intravisit::walk_struct_def(self, def);
Expand Down Expand Up @@ -500,6 +508,7 @@ fn find_live<'tcx>(
repr_has_repr_c: false,
in_pat: false,
inherited_pub_visibility: false,
pub_visibility: false,
ignore_variant_stack: vec![],
struct_constructors,
};
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/cast/issue-84213.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ struct Something {

fn main() {
let mut something = Something { field: 1337 };
let _ = something.field;

let _pointer_to_something = &something as *const Something;
//~^ ERROR: non-primitive cast
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/cast/issue-84213.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ struct Something {

fn main() {
let mut something = Something { field: 1337 };
let _ = something.field;

let _pointer_to_something = something as *const Something;
//~^ ERROR: non-primitive cast
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/cast/issue-84213.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0605]: non-primitive cast: `Something` as `*const Something`
--> $DIR/issue-84213.rs:10:33
--> $DIR/issue-84213.rs:11:33
|
LL | let _pointer_to_something = something as *const Something;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid cast
Expand All @@ -10,7 +10,7 @@ LL | let _pointer_to_something = &something as *const Something;
| ^

error[E0605]: non-primitive cast: `Something` as `*mut Something`
--> $DIR/issue-84213.rs:13:37
--> $DIR/issue-84213.rs:14:37
|
LL | let _mut_pointer_to_something = something as *mut Something;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ invalid cast
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/lint/dead-code/issue-85255.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Unused `pub` fields in non-`pub` structs should also trigger dead code warnings.
// check-pass

#![warn(dead_code)]

struct Foo {
a: i32, //~ WARNING: field is never read
pub b: i32, //~ WARNING: field is never read
}

struct Bar;

impl Bar {
fn a(&self) -> i32 { 5 } //~ WARNING: associated function is never used
pub fn b(&self) -> i32 { 6 } //~ WARNING: associated function is never used
}


fn main() {
let _ = Foo { a: 1, b: 2 };
let _ = Bar;
}
32 changes: 32 additions & 0 deletions src/test/ui/lint/dead-code/issue-85255.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
warning: field is never read: `a`
--> $DIR/issue-85255.rs:7:5
|
LL | a: i32,
| ^^^^^^
|
note: the lint level is defined here
--> $DIR/issue-85255.rs:4:9
|
LL | #![warn(dead_code)]
| ^^^^^^^^^

warning: field is never read: `b`
--> $DIR/issue-85255.rs:8:5
|
LL | pub b: i32,
| ^^^^^^^^^^

warning: associated function is never used: `a`
--> $DIR/issue-85255.rs:14:8
|
LL | fn a(&self) -> i32 { 5 }
| ^

warning: associated function is never used: `b`
--> $DIR/issue-85255.rs:15:12
|
LL | pub fn b(&self) -> i32 { 6 }
| ^

warning: 4 warnings emitted

0 comments on commit 46d55d6

Please sign in to comment.