Rollup merge of rust-lang#85324 - FabianWolff:issue-85255, r=varkor

Warn about unused `pub` fields in non-`pub` structs

This pull request fixes rust-lang#85255. The current implementation of dead code analysis is too prudent because it marks all `pub` fields of structs as live, even though they cannot be accessed from outside of the current crate if the struct itself only has restricted or private visibility.

I have changed this behavior to take the containing struct's visibility into account when looking at field visibility and liveness. This also makes dead code warnings more consistent; consider the example given in rust-lang#85255:
```rust
struct Foo {
    a: i32,
    pub b: i32,
}

struct Bar;

impl Bar {
    fn a(&self) -> i32 { 5 }
    pub fn b(&self) -> i32 { 6 }
}

fn main() {
    let _ = Foo { a: 1, b: 2 };
    let _ = Bar;
}
```
Current nightly already warns about `Bar::b()`, even though it is `pub` (but `Bar` is not). It should therefore also warn about `Foo::b`, which it does with the changes in this PR.

Author: GuillaumeGomez

compiler/rustc_passes/src/dead.rs

@@ -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>,
@@ -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);
             }
@@ -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<'_>]) {
@@ -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);
@@ -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,
     };

src/test/ui/cast/issue-84213.fixed

@@ -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

src/test/ui/cast/issue-84213.rs

@@ -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

src/test/ui/cast/issue-84213.stderr

@@ -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
@@ -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

src/test/ui/lint/dead-code/issue-85255.rs

@@ -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;
+}

src/test/ui/lint/dead-code/issue-85255.stderr

@@ -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
+