Skip to content

Commit

Permalink
Rollup merge of #82297 - tmiasko:write-only, r=oli-obk
Browse files Browse the repository at this point in the history
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.

Closes #81626.
  • Loading branch information
Dylan-DPC authored Feb 23, 2021
2 parents e2561c5 + 343b673 commit 9d378b3
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 9d378b3

Please sign in to comment.