about summary refs log tree commit diff
diff options
context:
space:
mode:
authorManish Goregaokar <manishsmail@gmail.com>2020-06-28 08:30:25 -0700
committerGitHub <noreply@github.com>2020-06-28 08:30:25 -0700
commit3f826a89740d6437f9ebd57d70f591a3ba8bbb1b (patch)
tree1d1064d4f66464b0af6be73f14e44dc39fa808ac
parentccc1bf79c8be0b4be549f6f82141104a34efec80 (diff)
parentffcfaa11055a2ec65d326003da41da24d7a7d66c (diff)
downloadrust-3f826a89740d6437f9ebd57d70f591a3ba8bbb1b.tar.gz
rust-3f826a89740d6437f9ebd57d70f591a3ba8bbb1b.zip
Rollup merge of #73774 - ecstatic-morse:liveness-of-projections, r=oli-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.
-rw-r--r--src/librustc_mir/dataflow/impls/liveness.rs27
-rw-r--r--src/test/ui/mir-dataflow/liveness-projection.rs32
-rw-r--r--src/test/ui/mir-dataflow/liveness-projection.stderr16
3 files changed, 73 insertions, 2 deletions
diff --git a/src/librustc_mir/dataflow/impls/liveness.rs b/src/librustc_mir/dataflow/impls/liveness.rs
index d24faacd377..784b0bd9293 100644
--- a/src/librustc_mir/dataflow/impls/liveness.rs
+++ b/src/librustc_mir/dataflow/impls/liveness.rs
@@ -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),
@@ -126,7 +146,6 @@ impl DefUse {
                 | MutatingUseContext::AsmOutput
                 | MutatingUseContext::Borrow
                 | MutatingUseContext::Drop
-                | MutatingUseContext::Projection
                 | MutatingUseContext::Retag,
             )
             | PlaceContext::NonMutatingUse(
@@ -134,11 +153,15 @@ impl DefUse {
                 | 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")
+            }
         }
     }
 }
diff --git a/src/test/ui/mir-dataflow/liveness-projection.rs b/src/test/ui/mir-dataflow/liveness-projection.rs
new file mode 100644
index 00000000000..486f31b635d
--- /dev/null
+++ b/src/test/ui/mir-dataflow/liveness-projection.rs
@@ -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() {}
diff --git a/src/test/ui/mir-dataflow/liveness-projection.stderr b/src/test/ui/mir-dataflow/liveness-projection.stderr
new file mode 100644
index 00000000000..f9480c88090
--- /dev/null
+++ b/src/test/ui/mir-dataflow/liveness-projection.stderr
@@ -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
+