Handle borrows of array elements

Author: sinkuu

src/librustc_mir/borrow_check.rs

@@ -35,6 +35,7 @@ use dataflow::{MovingOutStatements};
 use dataflow::{Borrows, BorrowData, BorrowIndex};
 use dataflow::move_paths::{MoveError, IllegalMoveOriginKind};
 use dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex, LookupResult, MoveOutIndex};
+use dataflow::abs_domain::Lift;
 use util::borrowck_errors::{BorrowckErrors, Origin};
 
 use self::MutateMode::{JustWrite, WriteAndRead};
@@ -841,15 +842,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                             -> Result<MovePathIndex, NoMovePathFound>
     {
         let mut last_prefix = lvalue;
-        for prefix in self.prefixes(lvalue, PrefixSet::All) {
+        for prefix in self.prefixes(lvalue) {
             if let Some(mpi) = self.move_path_for_lvalue(prefix) {
                 return Ok(mpi);
             }
             last_prefix = prefix;
         }
         match *last_prefix {
             Lvalue::Local(_) => panic!("should have move path for every Local"),
-            Lvalue::Projection(_) => panic!("PrefixSet::All meant dont stop for Projection"),
+            Lvalue::Projection(_) => panic!("`prefixes` meant dont stop for Projection"),
             Lvalue::Static(_) => return Err(NoMovePathFound::ReachedStatic),
         }
     }
@@ -1093,6 +1094,19 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         where F: FnMut(&mut Self, BorrowIndex, &BorrowData<'tcx>, &Lvalue<'tcx>) -> Control
     {
         let (access, lvalue) = access_lvalue;
+        debug!(
+            "each_borrow_involving_path({:?}, {:?})",
+            access,
+            self.describe_lvalue(lvalue)
+        );
+
+        let mut ps = vec![];
+        let mut base = lvalue;
+        while let Lvalue::Projection(ref proj) = *base {
+            ps.push(proj.elem.lift());
+            base = &proj.base;
+        }
+        debug!("base = {:?}, projections = {:?}", base, ps);
 
         // FIXME: analogous code in check_loans first maps `lvalue` to
         // its base_path.
@@ -1105,49 +1119,84 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         'next_borrow: for i in flow_state.borrows.elems_incoming() {
             let borrowed = &data[i];
 
-            // Is `lvalue` (or a prefix of it) already borrowed? If
-            // so, that's relevant.
-            //
+            debug!("borrowed = {:?}", borrowed);
+
             // FIXME: Differs from AST-borrowck; includes drive-by fix
             // to #38899. Will probably need back-compat mode flag.
-            for accessed_prefix in self.prefixes(lvalue, PrefixSet::All) {
-                if *accessed_prefix == borrowed.lvalue {
+            if base == &borrowed.base_lvalue {
+                let bps = &borrowed.projections;
+
+                // Is `lvalue` (or a prefix of it) already borrowed? If
+                // so, that's relevant.
+                if ps.ends_with(bps) {
+                    let accessed_prefix = {
+                        let mut lv = lvalue;
+                        for _ in 0..ps.len() - bps.len() {
+                            if let Lvalue::Projection(ref proj) = *lv {
+                                lv = &proj.base;
+                            } else {
+                                unreachable!()
+                            }
+                        }
+                        lv
+                    };
+                    debug!("accessed_prefix = {:?}", accessed_prefix);
                     // FIXME: pass in enum describing case we are in?
                     let ctrl = op(self, i, borrowed, accessed_prefix);
-                    if ctrl == Control::Break { return; }
+                    if ctrl == Control::Break {
+                        return;
+                    }
                 }
-            }
 
-            // Is `lvalue` a prefix (modulo access type) of the
-            // `borrowed.lvalue`? If so, that's relevant.
-
-            let prefix_kind = match access {
-                Shallow(Some(ArtificialField::Discriminant)) |
-                Shallow(Some(ArtificialField::ArrayLength)) => {
-                    // The discriminant and array length are like
-                    // additional fields on the type; they do not
-                    // overlap any existing data there. Furthermore,
-                    // they cannot actually be a prefix of any
-                    // borrowed lvalue (at least in MIR as it is
-                    // currently.)
-                    continue 'next_borrow;
-                }
-                Shallow(None) => PrefixSet::Shallow,
-                Deep => PrefixSet::Supporting,
-            };
+                // Is `lvalue` a prefix (modulo access type) of the
+                // `borrowed.lvalue`? If so, that's relevant.
+
+                let shallow = match access {
+                    Shallow(Some(ArtificialField::Discriminant)) |
+                    Shallow(Some(ArtificialField::ArrayLength)) => {
+                        // The discriminant and array length are like
+                        // additional fields on the type; they do not
+                        // overlap any existing data there. Furthermore,
+                        // they cannot actually be a prefix of any
+                        // borrowed lvalue (at least in MIR as it is
+                        // currently.)
+                        continue 'next_borrow;
+                    }
+                    Shallow(None) => true,
+                    Deep => false,
+                };
+                let bps = if shallow {
+                    &bps[borrowed.shallow_projections_len..]
+                } else {
+                    bps
+                };
 
-            for borrowed_prefix in self.prefixes(&borrowed.lvalue, prefix_kind) {
-                if borrowed_prefix == lvalue {
+                if bps.len() != ps.len() &&
+                    bps.ends_with(&ps)
+                {
+                    let borrowed_prefix = {
+                        let mut lv = &borrowed.lvalue;
+                        for _ in 0..bps.len() - ps.len() {
+                            if let Lvalue::Projection(ref proj) = *lv {
+                                lv = &proj.base;
+                            } else {
+                                unreachable!()
+                            }
+                        }
+                        lv
+                    };
+                    debug!("borrowed_prefix = {:?}", borrowed_prefix);
                     // FIXME: pass in enum describing case we are in?
                     let ctrl = op(self, i, borrowed, borrowed_prefix);
-                    if ctrl == Control::Break { return; }
+                    if ctrl == Control::Break {
+                        return;
+                    }
                 }
             }
         }
     }
 }
 
-use self::prefixes::PrefixSet;
 
 /// From the NLL RFC: "The deep [aka 'supporting'] prefixes for an
 /// lvalue are formed by stripping away fields and derefs, except that
@@ -1158,11 +1207,9 @@ use self::prefixes::PrefixSet;
 /// is borrowed. But: writing `a` is legal if `*a` is borrowed,
 /// whether or not `a` is a shared or mutable reference. [...] "
 mod prefixes {
-    use super::{MirBorrowckCtxt};
+    use super::MirBorrowckCtxt;
 
-    use rustc::hir;
-    use rustc::ty::{self, TyCtxt};
-    use rustc::mir::{Lvalue, Mir, ProjectionElem};
+    use rustc::mir::{Lvalue, ProjectionElem};
 
     pub trait IsPrefixOf<'tcx> {
         fn is_prefix_of(&self, other: &Lvalue<'tcx>) -> bool;
@@ -1188,38 +1235,23 @@ mod prefixes {
     }
 
 
-    pub(super) struct Prefixes<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
-        mir: &'cx Mir<'tcx>,
-        tcx: TyCtxt<'cx, 'gcx, 'tcx>,
-        kind: PrefixSet,
+    pub(super) struct Prefixes<'cx, 'tcx: 'cx> {
         next: Option<&'cx Lvalue<'tcx>>,
     }
 
-    #[derive(Copy, Clone, PartialEq, Eq, Debug)]
-    pub(super) enum PrefixSet {
-        /// Doesn't stop until it returns the base case (a Local or
-        /// Static prefix).
-        All,
-        /// Stops at any dereference.
-        Shallow,
-        /// Stops at the deref of a shared reference.
-        Supporting,
-    }
-
     impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         /// Returns an iterator over the prefixes of `lvalue`
         /// (inclusive) from longest to smallest, potentially
         /// terminating the iteration early based on `kind`.
         pub(super) fn prefixes(&self,
-                               lvalue: &'cx Lvalue<'tcx>,
-                               kind: PrefixSet)
-                               -> Prefixes<'cx, 'gcx, 'tcx>
+                               lvalue: &'cx Lvalue<'tcx>)
+                               -> Prefixes<'cx, 'tcx>
         {
-            Prefixes { next: Some(lvalue), kind, mir: self.mir, tcx: self.tcx }
+            Prefixes { next: Some(lvalue) }
         }
     }
 
-    impl<'cx, 'gcx, 'tcx> Iterator for Prefixes<'cx, 'gcx, 'tcx> {
+    impl<'cx, 'tcx: 'cx> Iterator for Prefixes<'cx, 'tcx> {
         type Item = &'cx Lvalue<'tcx>;
         fn next(&mut self) -> Option<Self::Item> {
             let mut cursor = match self.next {
@@ -1246,8 +1278,6 @@ mod prefixes {
                 match proj.elem {
                     ProjectionElem::Field(_/*field*/, _/*ty*/) => {
                         // FIXME: add union handling
-                        self.next = Some(&proj.base);
-                        return Some(cursor);
                     }
                     ProjectionElem::Downcast(..) |
                     ProjectionElem::Subslice { .. } |
@@ -1256,58 +1286,10 @@ mod prefixes {
                         cursor = &proj.base;
                         continue 'cursor;
                     }
-                    ProjectionElem::Deref => {
-                        // (handled below)
-                    }
-                }
-
-                assert_eq!(proj.elem, ProjectionElem::Deref);
-
-                match self.kind {
-                    PrefixSet::Shallow => {
-                        // shallow prefixes are found by stripping away
-                        // fields, but stop at *any* dereference.
-                        // So we can just stop the traversal now.
-                        self.next = None;
-                        return Some(cursor);
-                    }
-                    PrefixSet::All => {
-                        // all prefixes: just blindly enqueue the base
-                        // of the projection
-                        self.next = Some(&proj.base);
-                        return Some(cursor);
-                    }
-                    PrefixSet::Supporting => {
-                        // fall through!
-                    }
-                }
-
-                assert_eq!(self.kind, PrefixSet::Supporting);
-                // supporting prefixes: strip away fields and
-                // derefs, except we stop at the deref of a shared
-                // reference.
-
-                let ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
-                match ty.sty {
-                    ty::TyRawPtr(_) |
-                    ty::TyRef(_/*rgn*/, ty::TypeAndMut { ty: _, mutbl: hir::MutImmutable }) => {
-                        // don't continue traversing over derefs of raw pointers or shared borrows.
-                        self.next = None;
-                        return Some(cursor);
-                    }
-
-                    ty::TyRef(_/*rgn*/, ty::TypeAndMut { ty: _, mutbl: hir::MutMutable }) => {
-                        self.next = Some(&proj.base);
-                        return Some(cursor);
-                    }
-
-                    ty::TyAdt(..) if ty.is_box() => {
-                        self.next = Some(&proj.base);
-                        return Some(cursor);
-                    }
-
-                    _ => panic!("unknown type fed to Projection Deref."),
+                    _ => {}
                 }
+                self.next = Some(&proj.base);
+                return Some(cursor);
             }
         }
     }

src/librustc_mir/dataflow/move_paths/abs_domain.rs renamed to src/librustc_mir/dataflow/abs_domain.rs

@@ -20,6 +20,7 @@ use rustc_data_structures::indexed_set::{IdxSet};
 use rustc_data_structures::indexed_vec::{IndexVec};
 
 use dataflow::{BitDenotation, BlockSets, DataflowOperator};
+use dataflow::abs_domain::{AbstractElem, Lift};
 pub use dataflow::indexes::BorrowIndex;
 use transform::nll::region_infer::RegionInferenceContext;
 use transform::nll::ToRegionVid;
@@ -44,13 +45,19 @@ pub struct Borrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
 // temporarily allow some dead fields: `kind` and `region` will be
 // needed by borrowck; `lvalue` will probably be a MovePathIndex when
 // that is extended to include borrowed data paths.
-#[allow(dead_code)]
 #[derive(Debug)]
 pub struct BorrowData<'tcx> {
     pub(crate) location: Location,
     pub(crate) kind: mir::BorrowKind,
     pub(crate) region: Region<'tcx>,
     pub(crate) lvalue: mir::Lvalue<'tcx>,
+
+    /// Projections in outermost-first order (e.g. `a[1].foo` => `[Field, Index]`).
+    pub(crate) projections: Vec<AbstractElem<'tcx>>,
+    /// The base lvalue of projections.
+    pub(crate) base_lvalue: mir::Lvalue<'tcx>,
+    /// Number of projections outside the outermost dereference.
+    pub(crate) shallow_projections_len: usize,
 }
 
 impl<'tcx> fmt::Display for BorrowData<'tcx> {
@@ -77,7 +84,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
             idx_vec: IndexVec::new(),
             location_map: FxHashMap(),
             region_map: FxHashMap(),
-            region_span_map: FxHashMap()
+            region_span_map: FxHashMap(),
         };
         visitor.visit_mir(mir);
         return Borrows { tcx: tcx,
@@ -96,16 +103,38 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
             region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
             region_span_map: FxHashMap<RegionKind, Span>,
         }
-
-        impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
+        impl<'a, 'gcx: 'tcx, 'tcx: 'a> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
             fn visit_rvalue(&mut self,
                             rvalue: &mir::Rvalue<'tcx>,
                             location: mir::Location) {
                 if let mir::Rvalue::Ref(region, kind, ref lvalue) = *rvalue {
                     if is_unsafe_lvalue(self.tcx, self.mir, lvalue) { return; }
+                    let mut base_lvalue = lvalue;
+                    let mut projections = vec![];
+                    let mut shallow_projections_len = None;
+
+                    while let mir::Lvalue::Projection(ref proj) = *base_lvalue {
+                        projections.push(proj.elem.lift());
+                        base_lvalue = &proj.base;
+
+                        if let mir::ProjectionElem::Deref = proj.elem {
+                            if shallow_projections_len.is_none() {
+                                shallow_projections_len = Some(projections.len());
+                            }
+                        }
+                    }
+
+                    let shallow_projections_len =
+                        shallow_projections_len.unwrap_or(projections.len());
 
                     let borrow = BorrowData {
-                        location: location, kind: kind, region: region, lvalue: lvalue.clone(),
+                        location,
+                        kind,
+                        region,
+                        lvalue: lvalue.clone(),
+                        projections,
+                        base_lvalue: base_lvalue.clone(),
+                        shallow_projections_len,
                     };
                     let idx = self.idx_vec.push(borrow);
                     self.location_map.insert(location, idx);

src/librustc_mir/dataflow/impls/borrows.rs

@@ -36,6 +36,7 @@ mod drop_flag_effects;
 mod graphviz;
 mod impls;
 pub mod move_paths;
+pub mod abs_domain;
 
 pub(crate) use self::move_paths::indexes;
 

src/librustc_mir/dataflow/mod.rs

@@ -19,7 +19,7 @@ use syntax::codemap::DUMMY_SP;
 use std::collections::hash_map::Entry;
 use std::mem;
 
-use super::abs_domain::Lift;
+use dataflow::abs_domain::Lift;
 
 use super::{LocationMap, MoveData, MovePath, MovePathLookup, MovePathIndex, MoveOut, MoveOutIndex};
 use super::{MoveError};

src/librustc_mir/dataflow/move_paths/builder.rs

@@ -18,9 +18,7 @@ use syntax_pos::{Span};
 use std::fmt;
 use std::ops::{Index, IndexMut};
 
-use self::abs_domain::{AbstractElem, Lift};
-
-mod abs_domain;
+use super::abs_domain::{AbstractElem, Lift};
 
 // This submodule holds some newtype'd Index wrappers that are using
 // NonZero to ensure that Option<Index> occupies only a single word.

src/librustc_mir/dataflow/move_paths/mod.rs

@@ -0,0 +1,59 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// revisions: ast mir
+//[mir]compile-flags: -Z borrowck-mir
+
+fn use_x(_: usize) -> bool { true }
+
+fn main() {
+}
+
+fn foo() {
+    let mut v = [1, 2, 3];
+    let p = &v[0];
+    if true {
+        use_x(*p);
+    } else {
+        use_x(22);
+    }
+    v[0] += 1; //[ast]~ ERROR cannot assign to `v[..]` because it is borrowed
+    //[mir]~^ cannot assign to `v[..]` because it is borrowed (Ast)
+    //[mir]~| cannot assign to `v[..]` because it is borrowed (Mir)
+}
+
+fn bar() {
+    let mut v = [[1]];
+    let p = &v[0][0];
+    if true {
+        use_x(*p);
+    } else {
+        use_x(22);
+    }
+    v[0][0] += 1; //[ast]~ ERROR cannot assign to `v[..][..]` because it is borrowed
+    //[mir]~^ cannot assign to `v[..][..]` because it is borrowed (Ast)
+    //[mir]~| cannot assign to `v[..][..]` because it is borrowed (Mir)
+}
+
+fn baz() {
+    struct S<T> { x: T, y: T, }
+    let mut v = [S { x: 1, y: 2 },
+                 S { x: 3, y: 4 },
+                 S { x: 5, y: 6 }];
+    let p = &v[0].x;
+    if true {
+        use_x(*p);
+    } else {
+        use_x(22);
+    }
+    v[0].x += 1; //[ast]~ ERROR cannot assign to `v[..].x` because it is borrowed
+    //[mir]~^ cannot assign to `v[..].x` because it is borrowed (Ast)
+    //[mir]~| cannot assign to `v[..].x` because it is borrowed (Mir)
+}

src/test/compile-fail/borrowck/borrowck-array-element.rs

@@ -31,6 +31,8 @@ fn causes_ice(mut l: &mut Sexpression) {
                              //[mir]~| ERROR (Mir) [E0499]
         }
     }}
+    //[mir]~^ ERROR (Mir) [E0597]
+    // FIXME
 }
 
 fn main() {