Use RequiresStorage to determine which locals can overlap

Author: tmandry

src/librustc_mir/dataflow/impls/storage_liveness.rs

@@ -72,10 +72,6 @@ impl<'a, 'tcx> BottomValue for MaybeStorageLive<'a, 'tcx> {
 
 /// Dataflow analysis that determines whether each local requires storage at a
 /// given location; i.e. whether its storage can go away without being observed.
-///
-/// In the case of a movable generator, borrowed_locals can be `None` and we
-/// will not consider borrows in this pass. This relies on the fact that we only
-/// use this pass at yield points for these generators.
 pub struct RequiresStorage<'mir, 'tcx, 'b> {
     body: &'mir Body<'tcx>,
     borrowed_locals:

src/librustc_mir/transform/generator.rs

@@ -62,6 +62,7 @@ use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use rustc_data_structures::bit_set::{BitSet, BitMatrix};
 use std::borrow::Cow;
 use std::iter;
+use std::marker::PhantomData;
 use std::mem;
 use crate::transform::{MirPass, MirSource};
 use crate::transform::simplify;
@@ -539,8 +540,8 @@ fn locals_live_across_suspend_points(
         body,
         &live_locals,
         &ignored,
-        storage_live,
-        storage_live_analysis);
+        requires_storage,
+        requires_storage_analysis);
 
     LivenessInfo {
         live_locals,
@@ -577,8 +578,8 @@ fn compute_storage_conflicts(
     body: &'mir Body<'tcx>,
     stored_locals: &liveness::LiveVarSet,
     ignored: &StorageIgnored,
-    storage_live: DataflowResults<'tcx, MaybeStorageLive<'mir, 'tcx>>,
-    _storage_live_analysis: MaybeStorageLive<'mir, 'tcx>,
+    requires_storage: DataflowResults<'tcx, RequiresStorage<'mir, 'tcx, '_>>,
+    _requires_storage_analysis: RequiresStorage<'mir, 'tcx, '_>,
 ) -> BitMatrix<GeneratorSavedLocal, GeneratorSavedLocal> {
     assert_eq!(body.local_decls.len(), ignored.0.domain_size());
     assert_eq!(body.local_decls.len(), stored_locals.domain_size());
@@ -594,9 +595,10 @@ fn compute_storage_conflicts(
     let mut visitor = StorageConflictVisitor {
         body,
         stored_locals: &stored_locals,
-        local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len())
+        local_conflicts: BitMatrix::from_row_n(&ineligible_locals, body.local_decls.len()),
+        _phantom: PhantomData::default(),
     };
-    let mut state = FlowAtLocation::new(storage_live);
+    let mut state = FlowAtLocation::new(requires_storage);
     visitor.analyze_results(&mut state);
     let local_conflicts = visitor.local_conflicts;
 
@@ -626,18 +628,19 @@ fn compute_storage_conflicts(
     storage_conflicts
 }
 
-struct StorageConflictVisitor<'body, 'tcx, 's> {
+struct StorageConflictVisitor<'body: 'b, 'tcx, 's, 'b> {
     body: &'body Body<'tcx>,
     stored_locals: &'s liveness::LiveVarSet,
     // FIXME(tmandry): Consider using sparse bitsets here once we have good
     // benchmarks for generators.
     local_conflicts: BitMatrix<Local, Local>,
+    _phantom: PhantomData<&'b ()>,
 }
 
-impl<'body, 'tcx, 's> DataflowResultsConsumer<'body, 'tcx>
-    for StorageConflictVisitor<'body, 'tcx, 's>
+impl<'body, 'tcx, 's, 'b> DataflowResultsConsumer<'body, 'tcx>
+    for StorageConflictVisitor<'body, 'tcx, 's, 'b>
 {
-    type FlowState = FlowAtLocation<'tcx, MaybeStorageLive<'body, 'tcx>>;
+    type FlowState = FlowAtLocation<'tcx, RequiresStorage<'body, 'tcx, 'b>>;
 
     fn body(&self) -> &'body Body<'tcx> {
         self.body
@@ -665,9 +668,9 @@ impl<'body, 'tcx, 's> DataflowResultsConsumer<'body, 'tcx>
     }
 }
 
-impl<'body, 'tcx, 's> StorageConflictVisitor<'body, 'tcx, 's> {
+impl<'body, 'tcx, 's, 'b> StorageConflictVisitor<'body, 'tcx, 's, 'b> {
     fn apply_state(&mut self,
-                   flow_state: &FlowAtLocation<'tcx, MaybeStorageLive<'body, 'tcx>>,
+                   flow_state: &FlowAtLocation<'tcx, RequiresStorage<'body, 'tcx, 'b>>,
                    loc: Location) {
         // Ignore unreachable blocks.
         match self.body.basic_blocks()[loc.block].terminator().kind {

src/test/run-pass/generator/size-moved-locals.rs

@@ -21,7 +21,7 @@ impl Drop for Foo {
     fn drop(&mut self) {}
 }
 
-fn simple() -> impl Generator<Yield = (), Return = ()> {
+fn move_before_yield() -> impl Generator<Yield = (), Return = ()> {
     static || {
         let first = Foo([0; FOO_SIZE]);
         let _second = first;
@@ -32,7 +32,7 @@ fn simple() -> impl Generator<Yield = (), Return = ()> {
 
 fn noop() {}
 
-fn complex() -> impl Generator<Yield = (), Return = ()> {
+fn move_before_yield_with_noop() -> impl Generator<Yield = (), Return = ()> {
     static || {
         let first = Foo([0; FOO_SIZE]);
         noop();
@@ -42,7 +42,21 @@ fn complex() -> impl Generator<Yield = (), Return = ()> {
     }
 }
 
+// Today we don't have NRVO (we allocate space for both `first` and `second`,)
+// but we can overlap `first` with `_third`.
+fn overlap_move_points() -> impl Generator<Yield = (), Return = ()> {
+    static || {
+        let first = Foo([0; FOO_SIZE]);
+        yield;
+        let second = first;
+        yield;
+        let _third = second;
+        yield;
+    }
+}
+
 fn main() {
-    assert_eq!(1028, std::mem::size_of_val(&simple()));
-    assert_eq!(1032, std::mem::size_of_val(&complex()));
+    assert_eq!(1028, std::mem::size_of_val(&move_before_yield()));
+    assert_eq!(1032, std::mem::size_of_val(&move_before_yield_with_noop()));
+    assert_eq!(2056, std::mem::size_of_val(&overlap_move_points()));
 }