Skip to content

[MIR Borrowck] Handle borrows of array elements #46014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 77 additions & 96 deletions src/librustc_mir/borrow_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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<Self::Item> {
let mut cursor = match self.next {
Expand All @@ -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 { .. } |
Expand All @@ -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);
}
}
}
Expand Down
58 changes: 53 additions & 5 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand All @@ -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<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: Option<usize>,
/// Number of projections outside the outermost raw pointer or shared borrow.
pub(crate) supporting_projections_len: Option<usize>,
}

impl<'tcx> fmt::Display for BorrowData<'tcx> {
Expand All @@ -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,
Expand All @@ -96,16 +106,54 @@ 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;
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);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/dataflow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading