Skip to content

Commit

Permalink
Rollup merge of #73774 - ecstatic-morse:liveness-of-projections, r=ol…
Browse files Browse the repository at this point in the history
…i-obk

Make liveness more precise for assignments to fields

Previously, we were too conservative and `x.field = 4` was treated as a "use" of `x`. Now it neither kills `x` (since other fields of `x` may still be live) nor marks it as live.

cc @jonas-schievink, who ran into this problem.
  • Loading branch information
Manishearth authored Jun 28, 2020
2 parents ccc1bf7 + ffcfaa1 commit 3f826a8
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 2 deletions.
27 changes: 25 additions & 2 deletions src/librustc_mir/dataflow/impls/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,27 @@ impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T>
where
T: GenKill<Local>,
{
fn visit_place(&mut self, place: &mir::Place<'tcx>, context: PlaceContext, location: Location) {
let mir::Place { projection, local } = *place;

// We purposefully do not call `super_place` here to avoid calling `visit_local` for this
// place with one of the `Projection` variants of `PlaceContext`.
self.visit_projection(local, projection, context, location);

match DefUse::for_place(context) {
// Treat derefs as a use of the base local. `*p = 4` is not a def of `p` but a use.
Some(_) if place.is_indirect() => self.0.gen(local),

Some(DefUse::Def) if projection.is_empty() => self.0.kill(local),
Some(DefUse::Use) => self.0.gen(local),
_ => {}
}
}

fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) {
// Because we do not call `super_place` above, `visit_local` is only called for locals that
// do not appear as part of a `Place` in the MIR. This handles cases like the implicit use
// of the return place in a `Return` terminator or the index in an `Index` projection.
match DefUse::for_place(context) {
Some(DefUse::Def) => self.0.kill(local),
Some(DefUse::Use) => self.0.gen(local),
Expand Down Expand Up @@ -126,19 +146,22 @@ impl DefUse {
| MutatingUseContext::AsmOutput
| MutatingUseContext::Borrow
| MutatingUseContext::Drop
| MutatingUseContext::Projection
| MutatingUseContext::Retag,
)
| PlaceContext::NonMutatingUse(
NonMutatingUseContext::AddressOf
| NonMutatingUseContext::Copy
| NonMutatingUseContext::Inspect
| NonMutatingUseContext::Move
| NonMutatingUseContext::Projection
| NonMutatingUseContext::ShallowBorrow
| NonMutatingUseContext::SharedBorrow
| NonMutatingUseContext::UniqueBorrow,
) => Some(DefUse::Use),

PlaceContext::MutatingUse(MutatingUseContext::Projection)
| PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => {
unreachable!("A projection could be a def or a use and must be handled separately")
}
}
}
}
32 changes: 32 additions & 0 deletions src/test/ui/mir-dataflow/liveness-projection.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#![feature(core_intrinsics, rustc_attrs)]

use std::intrinsics::rustc_peek;

#[rustc_mir(rustc_peek_liveness, stop_after_dataflow)]
fn foo() {
{
let mut x: (i32, i32) = (42, 0);

// Assignment to a projection does not cause `x` to become live
unsafe { rustc_peek(x); } //~ ERROR bit not set
x.1 = 42;

x = (0, 42);

// ...but a read from a projection does.
unsafe { rustc_peek(x); }
println!("{}", x.1);
}

{
let mut x = 42;

// Derefs are treated like a read of a local even if they are on the LHS of an assignment.
let p = &mut x;
unsafe { rustc_peek(&p); }
*p = 24;
unsafe { rustc_peek(&p); } //~ ERROR bit not set
}
}

fn main() {}
16 changes: 16 additions & 0 deletions src/test/ui/mir-dataflow/liveness-projection.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: rustc_peek: bit not set
--> $DIR/liveness-projection.rs:11:18
|
LL | unsafe { rustc_peek(x); }
| ^^^^^^^^^^^^^

error: rustc_peek: bit not set
--> $DIR/liveness-projection.rs:28:18
|
LL | unsafe { rustc_peek(&p); }
| ^^^^^^^^^^^^^^

error: stop_after_dataflow ended compilation

error: aborting due to 3 previous errors

0 comments on commit 3f826a8

Please sign in to comment.