Skip to content

Commit

Permalink
Consider auto derefs before warning about write only fields
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tmiasko committed Feb 19, 2021
1 parent 0148b97 commit 343b673
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 12 deletions.
28 changes: 17 additions & 11 deletions compiler/rustc_passes/src/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<hir::HirId>,
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -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<'_>,
Expand Down Expand Up @@ -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;
}
Expand Down
49 changes: 49 additions & 0 deletions src/test/ui/lint/dead-code/write-only-field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<B>,
}

struct B {
c: Box<C>,
}

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 });
}
20 changes: 19 additions & 1 deletion src/test/ui/lint/dead-code/write-only-field.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 343b673

Please sign in to comment.