From 343b67387772dbd068d06a76267288579d3eaed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 19 Feb 2021 00:00:00 +0000 Subject: [PATCH] Consider auto derefs before warning about write only fields Changes from 81473 extended the dead code lint with an ability to detect fields that are written to but never read from. The implementation skips over fields on the left hand side of an assignment, without marking them as live. A field access might involve an automatic dereference and de-facto read the field. Conservatively mark expressions with deref adjustments as live to avoid generating false positive warnings. --- compiler/rustc_passes/src/dead.rs | 28 ++++++----- .../ui/lint/dead-code/write-only-field.rs | 49 +++++++++++++++++++ .../ui/lint/dead-code/write-only-field.stderr | 20 +++++++- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index d51b501f7ae3d..62a95aa57c29f 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -37,15 +37,6 @@ fn should_explore(tcx: TyCtxt<'_>, hir_id: hir::HirId) -> bool { ) } -fn base_expr<'a>(mut expr: &'a hir::Expr<'a>) -> &'a hir::Expr<'a> { - loop { - match expr.kind { - hir::ExprKind::Field(base, ..) => expr = base, - _ => return expr, - } - } -} - struct MarkSymbolVisitor<'tcx> { worklist: Vec, tcx: TyCtxt<'tcx>, @@ -143,6 +134,22 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { } } + fn handle_assign(&mut self, expr: &'tcx hir::Expr<'tcx>) { + if self + .typeck_results() + .expr_adjustments(expr) + .iter() + .any(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_))) + { + self.visit_expr(expr); + } else if let hir::ExprKind::Field(base, ..) = expr.kind { + // Ignore write to field + self.handle_assign(base); + } else { + self.visit_expr(expr); + } + } + fn handle_field_pattern_match( &mut self, lhs: &hir::Pat<'_>, @@ -272,8 +279,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> { self.lookup_and_handle_method(expr.hir_id); } hir::ExprKind::Assign(ref left, ref right, ..) => { - // Ignore write to field - self.visit_expr(base_expr(left)); + self.handle_assign(left); self.visit_expr(right); return; } diff --git a/src/test/ui/lint/dead-code/write-only-field.rs b/src/test/ui/lint/dead-code/write-only-field.rs index 78cfcfda8f971..7b3f1e9f5b6cb 100644 --- a/src/test/ui/lint/dead-code/write-only-field.rs +++ b/src/test/ui/lint/dead-code/write-only-field.rs @@ -17,4 +17,53 @@ fn field_write(s: &mut S) { fn main() { let mut s = S { f: 0, sub: Sub { f: 0 } }; field_write(&mut s); + + auto_deref(); + nested_boxes(); +} + +fn auto_deref() { + struct E { + x: bool, + y: bool, //~ ERROR: field is never read + } + + struct P<'a> { + e: &'a mut E + } + + impl P<'_> { + fn f(&mut self) { + self.e.x = true; + self.e.y = true; + } + } + + let mut e = E { x: false, y: false }; + let mut p = P { e: &mut e }; + p.f(); + assert!(e.x); +} + +fn nested_boxes() { + struct A { + b: Box, + } + + struct B { + c: Box, + } + + struct C { + u: u32, //~ ERROR: field is never read + v: u32, //~ ERROR: field is never read + } + + let mut a = A { + b: Box::new(B { + c: Box::new(C { u: 0, v: 0 }), + }), + }; + a.b.c.v = 10; + a.b.c = Box::new(C { u: 1, v: 2 }); } diff --git a/src/test/ui/lint/dead-code/write-only-field.stderr b/src/test/ui/lint/dead-code/write-only-field.stderr index 70d2149665b20..a191d22c8b94c 100644 --- a/src/test/ui/lint/dead-code/write-only-field.stderr +++ b/src/test/ui/lint/dead-code/write-only-field.stderr @@ -22,5 +22,23 @@ error: field is never read: `f` LL | f: i32, | ^^^^^^ -error: aborting due to 3 previous errors +error: field is never read: `y` + --> $DIR/write-only-field.rs:28:9 + | +LL | y: bool, + | ^^^^^^^ + +error: field is never read: `u` + --> $DIR/write-only-field.rs:58:9 + | +LL | u: u32, + | ^^^^^^ + +error: field is never read: `v` + --> $DIR/write-only-field.rs:59:9 + | +LL | v: u32, + | ^^^^^^ + +error: aborting due to 6 previous errors