diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index cdac72b6dffb0..d35963737d621 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/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,7 +842,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { -> Result { 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); } @@ -849,7 +850,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } 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,23 +1119,43 @@ 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); + + if base != &borrowed.base_lvalue { + continue; + } + + let bps = &borrowed.projections; + // 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 { - // FIXME: pass in enum describing case we are in? - let ctrl = op(self, i, borrowed, accessed_prefix); - if ctrl == Control::Break { return; } + + // 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; } } // Is `lvalue` a prefix (modulo access type) of the // `borrowed.lvalue`? If so, that's relevant. - let prefix_kind = match access { + let depth = match access { Shallow(Some(ArtificialField::Discriminant)) | Shallow(Some(ArtificialField::ArrayLength)) => { // The discriminant and array length are like @@ -1132,22 +1166,36 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // currently.) continue 'next_borrow; } - Shallow(None) => PrefixSet::Shallow, - Deep => PrefixSet::Supporting, + Shallow(None) => borrowed.shallow_projections_len, + Deep => borrowed.supporting_projections_len, }; - for borrowed_prefix in self.prefixes(&borrowed.lvalue, prefix_kind) { - if borrowed_prefix == lvalue { - // FIXME: pass in enum describing case we are in? - let ctrl = op(self, i, borrowed, borrowed_prefix); - if ctrl == Control::Break { return; } + if bps.len() != ps.len() + && depth.map(|l| ps.len() > (bps.len() - l)).unwrap_or(true) + && 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; } } } } } -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 +1206,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 +1234,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 { let mut cursor = match self.next { @@ -1246,8 +1277,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 +1285,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); } } } diff --git a/src/librustc_mir/dataflow/move_paths/abs_domain.rs b/src/librustc_mir/dataflow/abs_domain.rs similarity index 100% rename from src/librustc_mir/dataflow/move_paths/abs_domain.rs rename to src/librustc_mir/dataflow/abs_domain.rs diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index acfa195d7b09a..14fb9b0288de0 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use rustc::hir; use rustc::mir::{self, Location, Mir}; use rustc::mir::visit::Visitor; use rustc::ty::{self, Region, TyCtxt}; @@ -20,6 +21,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 +46,21 @@ 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>, + /// The base lvalue of projections. + pub(crate) base_lvalue: mir::Lvalue<'tcx>, + /// Number of projections outside the outermost dereference. + pub(crate) shallow_projections_len: Option, + /// Number of projections outside the outermost raw pointer or shared borrow. + pub(crate) supporting_projections_len: Option, } impl<'tcx> fmt::Display for BorrowData<'tcx> { @@ -77,7 +87,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 +106,54 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { region_map: FxHashMap, FxHashSet>, region_span_map: FxHashMap, } - - 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; + let mut supporting_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()); + } + + if supporting_projections_len.is_none() { + 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, + }, + ) => { + supporting_projections_len = Some(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, + supporting_projections_len, }; let idx = self.idx_vec.push(borrow); self.location_map.insert(location, idx); diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index bca9324d5b0aa..c5ae3e4687de1 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.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; diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index a0212de605eee..2bfb7a4b1ba6b 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.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}; diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 73218c9815024..cc0ba618d1057 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.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 occupies only a single word. diff --git a/src/test/compile-fail/borrowck/borrowck-array-element.rs b/src/test/compile-fail/borrowck/borrowck-array-element.rs new file mode 100644 index 0000000000000..7d987b434b9dc --- /dev/null +++ b/src/test/compile-fail/borrowck/borrowck-array-element.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 or the MIT license +// , 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 { 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) +}