Remove uniform_array_move_out passes

These passes were buggy, MIR building is now responsible for
canonicalizing `ConstantIndex` projections and `MoveData` is responsible
for splitting `Subslice` projections.

Author: matthewjasper

Diff for: src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -78,10 +78,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             .collect();
 
         if move_out_indices.is_empty() {
-            let root_place = self
-                .prefixes(used_place, PrefixSet::All)
-                .last()
-                .unwrap();
+            let root_place = PlaceRef { projection: &[], ..used_place };
 
             if !self.uninitialized_error_reported.insert(root_place) {
                 debug!(

Diff for: src/librustc_mir/borrow_check/mod.rs

@@ -174,7 +174,7 @@ fn do_mir_borrowck<'a, 'tcx>(
 
     let mut errors_buffer = Vec::new();
     let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<(Place<'tcx>, MoveError<'tcx>)>>) =
-        match MoveData::gather_moves(&body, tcx) {
+        match MoveData::gather_moves(&body, tcx, param_env) {
             Ok(move_data) => (move_data, None),
             Err((move_data, move_errors)) => (move_data, Some(move_errors)),
         };
@@ -1600,7 +1600,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         (prefix, place_span.0, place_span.1),
                         mpi,
                     );
-                    return; // don't bother finding other problems.
                 }
             }
             Err(NoMovePathFound::ReachedStatic) => {
@@ -1614,6 +1613,46 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         }
     }
 
+    /// Subslices correspond to multiple move paths, so we iterate through the
+    /// elements of the base array. For each element we check
+    ///
+    /// * Does this element overlap with our slice.
+    /// * Is any part of it uninitialized.
+    fn check_if_subslice_element_is_moved(
+        &mut self,
+        location: Location,
+        desired_action: InitializationRequiringAction,
+        place_span: (PlaceRef<'cx, 'tcx>, Span),
+        maybe_uninits: &FlowAtLocation<'tcx, MaybeUninitializedPlaces<'cx, 'tcx>>,
+        from: u32,
+        to: u32,
+    ) {
+        if let Some(mpi) = self.move_path_for_place(place_span.0) {
+            let mut child = self.move_data.move_paths[mpi].first_child;
+            while let Some(child_mpi) = child {
+                let child_move_place = &self.move_data.move_paths[child_mpi];
+                let child_place = &child_move_place.place;
+                let last_proj = child_place.projection.last().unwrap();
+                if let ProjectionElem::ConstantIndex { offset, from_end, .. } = last_proj {
+                    debug_assert!(!from_end, "Array constant indexing shouldn't be `from_end`.");
+
+                    if (from..to).contains(offset) {
+                        if let Some(uninit_child) = maybe_uninits.has_any_child_of(child_mpi) {
+                            self.report_use_of_moved_or_uninitialized(
+                                location,
+                                desired_action,
+                                (place_span.0, place_span.0, place_span.1),
+                                uninit_child,
+                            );
+                            return; // don't bother finding other problems.
+                        }
+                    }
+                }
+                child = child_move_place.next_sibling;
+            }
+        }
+    }
+
     fn check_if_path_or_subpath_is_moved(
         &mut self,
         location: Location,
@@ -1640,6 +1679,30 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
 
         self.check_if_full_path_is_moved(location, desired_action, place_span, flow_state);
 
+        if let [
+            base_proj @ ..,
+            ProjectionElem::Subslice { from, to, from_end: false },
+        ] = place_span.0.projection {
+            let place_ty = Place::ty_from(
+                place_span.0.base,
+                base_proj,
+                self.body(),
+                self.infcx.tcx,
+            );
+            if let ty::Array(..) = place_ty.ty.kind {
+                let array_place = PlaceRef { base: place_span.0.base, projection: base_proj };
+                self.check_if_subslice_element_is_moved(
+                    location,
+                    desired_action,
+                    (array_place, place_span.1),
+                    maybe_uninits,
+                    *from,
+                    *to,
+                );
+                return;
+            }
+        }
+
         // A move of any shallow suffix of `place` also interferes
         // with an attempt to use `place`. This is scenario 3 above.
         //

Diff for: src/librustc_mir/borrow_check/places_conflict.rs

@@ -533,8 +533,8 @@ fn place_projection_conflict<'tcx>(
             }
         }
         (ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true },
-         ProjectionElem::Subslice { to, .. })
-        | (ProjectionElem::Subslice { to, .. },
+         ProjectionElem::Subslice { to, from_end: true, .. })
+        | (ProjectionElem::Subslice { to, from_end: true, .. },
             ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true }) => {
             if offset > to {
                 debug!("place_element_conflict: \

Diff for: src/librustc_mir/dataflow/move_paths/builder.rs

@@ -4,7 +4,7 @@ use rustc::ty::{self, TyCtxt};
 use rustc_index::vec::IndexVec;
 use smallvec::{smallvec, SmallVec};
 
-use std::collections::hash_map::Entry;
+use std::convert::TryInto;
 use std::mem;
 
 use super::abs_domain::Lift;
@@ -17,19 +17,21 @@ use super::{
 struct MoveDataBuilder<'a, 'tcx> {
     body: &'a Body<'tcx>,
     tcx: TyCtxt<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
     data: MoveData<'tcx>,
     errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
 }
 
 impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
-    fn new(body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
+    fn new(body: &'a Body<'tcx>, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Self {
         let mut move_paths = IndexVec::new();
         let mut path_map = IndexVec::new();
         let mut init_path_map = IndexVec::new();
 
         MoveDataBuilder {
             body,
             tcx,
+            param_env,
             errors: Vec::new(),
             data: MoveData {
                 moves: IndexVec::new(),
@@ -148,42 +150,47 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
                             InteriorOfSliceOrArray { ty: place_ty, is_index: true },
                         ));
                     }
-                    _ => {
-                        // FIXME: still badly broken
-                    }
+                    _ => {}
                 },
                 _ => {}
             };
 
-            let proj = &place.projection[..i+1];
-            base = match self
-                .builder
-                .data
-                .rev_lookup
-                .projections
-                .entry((base, elem.lift()))
-                {
-                    Entry::Occupied(ent) => *ent.get(),
-                    Entry::Vacant(ent) => {
-                        let path = MoveDataBuilder::new_move_path(
-                            &mut self.builder.data.move_paths,
-                            &mut self.builder.data.path_map,
-                            &mut self.builder.data.init_path_map,
-                            Some(base),
-                            Place {
-                                base: place.base.clone(),
-                                projection: tcx.intern_place_elems(proj),
-                            },
-                        );
-                        ent.insert(path);
-                        path
-                    }
-                };
+            base = self.add_move_path(base, elem, |tcx| {
+                Place {
+                    base: place.base.clone(),
+                    projection: tcx.intern_place_elems(&place.projection[..i+1]),
+                }
+            });
         }
 
         Ok(base)
     }
 
+    fn add_move_path(
+        &mut self,
+        base: MovePathIndex,
+        elem: &PlaceElem<'tcx>,
+        mk_place: impl FnOnce(TyCtxt<'tcx>) -> Place<'tcx>,
+    ) -> MovePathIndex {
+        let MoveDataBuilder {
+            data: MoveData { rev_lookup, move_paths, path_map, init_path_map, .. },
+            tcx,
+            ..
+        } = self.builder;
+        *rev_lookup.projections
+            .entry((base, elem.lift()))
+            .or_insert_with(move || {
+                let path = MoveDataBuilder::new_move_path(
+                    move_paths,
+                    path_map,
+                    init_path_map,
+                    Some(base),
+                    mk_place(*tcx),
+                );
+                path
+            })
+    }
+
     fn create_move_path(&mut self, place: &Place<'tcx>) {
         // This is an non-moving access (such as an overwrite or
         // drop), so this not being a valid move path is OK.
@@ -214,8 +221,9 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
 pub(super) fn gather_moves<'tcx>(
     body: &Body<'tcx>,
     tcx: TyCtxt<'tcx>,
+    param_env: ty::ParamEnv<'tcx>,
 ) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
-    let mut builder = MoveDataBuilder::new(body, tcx);
+    let mut builder = MoveDataBuilder::new(body, tcx, param_env);
 
     builder.gather_args();
 
@@ -411,20 +419,67 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
     fn gather_move(&mut self, place: &Place<'tcx>) {
         debug!("gather_move({:?}, {:?})", self.loc, place);
 
-        let path = match self.move_path_for(place) {
-            Ok(path) | Err(MoveError::UnionMove { path }) => path,
-            Err(error @ MoveError::IllegalMove { .. }) => {
-                self.builder.errors.push((place.clone(), error));
-                return;
+        if let [
+            ref base @ ..,
+            ProjectionElem::Subslice { from, to, from_end: false },
+        ] = **place.projection {
+            // Split `Subslice` patterns into the corresponding list of
+            // `ConstIndex` patterns. This is done to ensure that all move paths
+            // are disjoint, which is expected by drop elaboration.
+            let base_place = Place {
+                base: place.base.clone(),
+                projection: self.builder.tcx.intern_place_elems(base),
+            };
+            let base_path = match self.move_path_for(&base_place) {
+                Ok(path) => path,
+                Err(MoveError::UnionMove { path }) => {
+                    self.record_move(place, path);
+                    return;
+                }
+                Err(error @ MoveError::IllegalMove { .. }) => {
+                    self.builder.errors.push((base_place, error));
+                    return;
+                }
+            };
+            let base_ty = base_place.ty(self.builder.body, self.builder.tcx).ty;
+            let len: u32 = match base_ty.kind {
+                ty::Array(_, size) => {
+                    let length = size.eval_usize(self.builder.tcx, self.builder.param_env);
+                    length.try_into().expect(
+                        "slice pattern of array with more than u32::MAX elements"
+                    )
+                }
+                _ => bug!("from_end: false slice pattern of non-array type"),
+            };
+            for offset in from..to {
+                let elem = ProjectionElem::ConstantIndex {
+                    offset,
+                    min_length: len,
+                    from_end: false,
+                };
+                let path = self.add_move_path(
+                    base_path,
+                    &elem,
+                    |tcx| tcx.mk_place_elem(base_place.clone(), elem),
+                );
+                self.record_move(place, path);
             }
-        };
-        let move_out = self.builder.data.moves.push(MoveOut { path: path, source: self.loc });
+        } else {
+            match self.move_path_for(place) {
+                Ok(path) | Err(MoveError::UnionMove { path }) => self.record_move(place, path),
+                Err(error @ MoveError::IllegalMove { .. }) => {
+                    self.builder.errors.push((place.clone(), error));
+                }
+            };
+        }
+    }
 
+    fn record_move(&mut self, place: &Place<'tcx>, path: MovePathIndex) {
+        let move_out = self.builder.data.moves.push(MoveOut { path: path, source: self.loc });
         debug!(
             "gather_move({:?}, {:?}): adding move {:?} of {:?}",
             self.loc, place, move_out, path
         );
-
         self.builder.data.path_map[path].push(move_out);
         self.builder.data.loc_map[self.loc].push(move_out);
     }

Diff for: src/librustc_mir/transform/elaborate_drops.rs

@@ -26,7 +26,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
 
         let def_id = src.def_id();
         let param_env = tcx.param_env(src.def_id()).with_reveal_all();
-        let move_data = match MoveData::gather_moves(body, tcx) {
+        let move_data = match MoveData::gather_moves(body, tcx, param_env) {
             Ok(move_data) => move_data,
             Err(_) => bug!("No `move_errors` should be allowed in MIR borrowck"),
         };

Diff for: src/librustc_mir/transform/mod.rs

@@ -35,7 +35,6 @@ pub mod copy_prop;
 pub mod const_prop;
 pub mod generator;
 pub mod inline;
-pub mod uniform_array_move_out;
 pub mod uninhabited_enum_branching;
 
 pub(crate) fn provide(providers: &mut Providers<'_>) {
@@ -229,7 +228,6 @@ fn mir_const(tcx: TyCtxt<'_>, def_id: DefId) -> &Steal<BodyAndCache<'_>> {
         // What we need to do constant evaluation.
         &simplify::SimplifyCfg::new("initial"),
         &rustc_peek::SanityCheck,
-        &uniform_array_move_out::UniformArrayMoveOut,
     ]);
     body.ensure_predecessors();
     tcx.alloc_steal_mir(body)
@@ -294,7 +292,6 @@ fn run_optimization_passes<'tcx>(
         // Optimizations begin.
         &uninhabited_enum_branching::UninhabitedEnumBranching,
         &simplify::SimplifyCfg::new("after-uninhabited-enum-branching"),
-        &uniform_array_move_out::RestoreSubsliceArrayMoveOut::new(tcx),
         &inline::Inline,
 
         // Lowering generator control-flow and variables

Diff for: src/librustc_mir/transform/rustc_peek.rs

@@ -37,7 +37,7 @@ impl<'tcx> MirPass<'tcx> for SanityCheck {
 
         let attributes = tcx.get_attrs(def_id);
         let param_env = tcx.param_env(def_id);
-        let move_data = MoveData::gather_moves(body, tcx).unwrap();
+        let move_data = MoveData::gather_moves(body, tcx, param_env).unwrap();
         let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env };
         let dead_unwinds = BitSet::new_empty(body.basic_blocks().len());
         let flow_inits =