Skip to content

Commit

Permalink
Add "Shallow" borrow kind
Browse files Browse the repository at this point in the history
This allows treating the "fake" match borrows differently from shared
borrows.
  • Loading branch information
matthewjasper committed Sep 24, 2018
1 parent a072d1b commit ced5c2d
Show file tree
Hide file tree
Showing 11 changed files with 102 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/librustc/ich/impls_mir.rs
Expand Up @@ -46,6 +46,7 @@ for mir::BorrowKind {

match *self {
mir::BorrowKind::Shared |
mir::BorrowKind::Shallow |
mir::BorrowKind::Unique => {}
mir::BorrowKind::Mut { allow_two_phase_borrow } => {
allow_two_phase_borrow.hash_stable(hcx, hasher);
Expand Down
24 changes: 23 additions & 1 deletion src/librustc/mir/mod.rs
Expand Up @@ -456,6 +456,27 @@ pub enum BorrowKind {
/// Data must be immutable and is aliasable.
Shared,

/// The immediately borrowed place must be immutable, but projections from
/// it don't need to be. For example, a shallow borrow of `a.b` doesn't
/// conflict with a mutable borrow of `a.b.c`.
///
/// This is used when lowering matches: when matching on a place we want to
/// ensure that place have the same value from the start of the match until
/// an arm is selected. This prevents this code from compiling:
///
/// let mut x = &Some(0);
/// match *x {
/// None => (),
/// Some(_) if { x = &None; false } => (),
/// Some(_) => (),
/// }
///
/// This can't be a shared borrow because mutably borrowing (*x as Some).0
/// should not prevent `if let None = x { ... }`, for example, becase the
/// mutating `(*x as Some).0` can't affect the discriminant of `x`.
/// We can also report errors with this kind of borrow differently.
Shallow,

/// Data must be immutable but not aliasable. This kind of borrow
/// cannot currently be expressed by the user and is used only in
/// implicit closure bindings. It is needed when the closure is
Expand Down Expand Up @@ -504,7 +525,7 @@ pub enum BorrowKind {
impl BorrowKind {
pub fn allows_two_phase_borrow(&self) -> bool {
match *self {
BorrowKind::Shared | BorrowKind::Unique => false,
BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => false,
BorrowKind::Mut {
allow_two_phase_borrow,
} => allow_two_phase_borrow,
Expand Down Expand Up @@ -2198,6 +2219,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
Ref(region, borrow_kind, ref place) => {
let kind_str = match borrow_kind {
BorrowKind::Shared => "",
BorrowKind::Shallow => "shallow ",
BorrowKind::Mut { .. } | BorrowKind::Unique => "mut ",
};

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/mir/tcx.rs
Expand Up @@ -287,6 +287,10 @@ impl BorrowKind {
// use `&mut`. It gives all the capabilities of an `&uniq`
// and hence is a safe "over approximation".
BorrowKind::Unique => hir::MutMutable,

// We have no type corresponding to a shallow borrow, so use
// `&` as an approximation.
BorrowKind::Shallow => hir::MutImmutable,
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/mir/visit.rs
Expand Up @@ -963,6 +963,7 @@ impl<'tcx> PlaceContext<'tcx> {

PlaceContext::Inspect |
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
PlaceContext::Projection(Mutability::Not) |
PlaceContext::Copy | PlaceContext::Move |
Expand All @@ -974,7 +975,9 @@ impl<'tcx> PlaceContext<'tcx> {
/// Returns true if this place context represents a use that does not change the value.
pub fn is_nonmutating_use(&self) -> bool {
match *self {
PlaceContext::Inspect | PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Inspect |
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
PlaceContext::Projection(Mutability::Not) |
PlaceContext::Copy | PlaceContext::Move => true,
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/borrow_check/borrow_set.rs
Expand Up @@ -87,6 +87,7 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
let kind = match self.kind {
mir::BorrowKind::Shared => "",
mir::BorrowKind::Shallow => "shallow ",
mir::BorrowKind::Unique => "uniq ",
mir::BorrowKind::Mut { .. } => "mut ",
};
Expand Down Expand Up @@ -287,7 +288,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
| PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
TwoPhaseActivation::NotActivated
}
_ => {
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_mir/borrow_check/error_reporting.rs
Expand Up @@ -333,6 +333,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Origin::Mir,
),

(BorrowKind::Mut { .. }, _, _, BorrowKind::Shallow, _, _)
| (BorrowKind::Unique, _, _, BorrowKind::Shallow, _, _) => {
return;
}

(BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure(
span,
&desc_place,
Expand Down
32 changes: 25 additions & 7 deletions src/librustc_mir/borrow_check/mod.rs
Expand Up @@ -755,6 +755,7 @@ use self::AccessDepth::{Deep, Shallow};
enum ArtificialField {
Discriminant,
ArrayLength,
ShallowBorrow,
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -972,7 +973,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Control::Continue
}

(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
| (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
Control::Continue
}

(Write(WriteKind::Move), BorrowKind::Shallow) => {
// Handled by initialization checks.
Control::Continue
}

Expand Down Expand Up @@ -1108,6 +1115,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
match *rvalue {
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
let access_kind = match bk {
BorrowKind::Shallow => {
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
},
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
BorrowKind::Unique | BorrowKind::Mut { .. } => {
let wk = WriteKind::MutableBorrow(bk);
Expand Down Expand Up @@ -1315,11 +1325,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
return;
}

// FIXME: replace this with a proper borrow_conflicts_with_place when
// that is merged.
let sd = if might_be_alive { Deep } else { Shallow(None) };

if places_conflict::places_conflict(self.infcx.tcx, self.mir, place, root_place, sd) {
if places_conflict::borrow_conflicts_with_place(
self.infcx.tcx,
self.mir,
place,
borrow.kind,
root_place,
sd
) {
debug!("check_for_invalidation_at_exit({:?}): INVALID", place);
// FIXME: should be talking about the region lifetime instead
// of just a span here.
Expand Down Expand Up @@ -1369,7 +1384,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

// only mutable borrows should be 2-phase
assert!(match borrow.kind {
BorrowKind::Shared => false,
BorrowKind::Shared | BorrowKind::Shallow => false,
BorrowKind::Unique | BorrowKind::Mut { .. } => true,
});

Expand Down Expand Up @@ -1669,7 +1684,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let is_local_mutation_allowed = match borrow_kind {
BorrowKind::Unique => LocalMutationIsAllowed::Yes,
BorrowKind::Mut { .. } => is_local_mutation_allowed,
BorrowKind::Shared => unreachable!(),
BorrowKind::Shared | BorrowKind::Shallow => unreachable!(),
};
match self.is_mutable(place, is_local_mutation_allowed) {
Ok(root_place) => {
Expand Down Expand Up @@ -1699,8 +1714,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
| Write(wk @ WriteKind::Move)
| Reservation(wk @ WriteKind::StorageDeadOrDrop)
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow))
| Write(wk @ WriteKind::StorageDeadOrDrop)
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared)) => {
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow)) => {
if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) {
if self.infcx.tcx.migrate_borrowck() {
// rust-lang/rust#46908: In pure NLL mode this
Expand Down Expand Up @@ -1743,6 +1760,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Read(ReadKind::Borrow(BorrowKind::Unique))
| Read(ReadKind::Borrow(BorrowKind::Mut { .. }))
| Read(ReadKind::Borrow(BorrowKind::Shared))
| Read(ReadKind::Borrow(BorrowKind::Shallow))
| Read(ReadKind::Copy) => {
// Access authorized
return false;
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_mir/borrow_check/nll/invalidation.rs
Expand Up @@ -329,6 +329,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
match *rvalue {
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
let access_kind = match bk {
BorrowKind::Shallow => {
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
},
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
BorrowKind::Unique | BorrowKind::Mut { .. } => {
let wk = WriteKind::MutableBorrow(bk);
Expand Down Expand Up @@ -439,8 +442,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
// have already taken the reservation
}

(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
// Reads/reservations don't invalidate shared borrows
(Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
| (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
// Reads/reservations don't invalidate shared or shallow borrows
}

(Read(_), BorrowKind::Unique) | (Read(_), BorrowKind::Mut { .. }) => {
Expand Down
9 changes: 8 additions & 1 deletion src/librustc_mir/borrow_check/path_utils.rs
Expand Up @@ -61,7 +61,14 @@ pub(super) fn each_borrow_involving_path<'a, 'tcx, 'gcx: 'tcx, F, I, S> (
for i in candidates {
let borrowed = &borrow_set[i];

if places_conflict::places_conflict(tcx, mir, &borrowed.borrowed_place, place, access) {
if places_conflict::places_conflict(
tcx,
mir,
&borrowed.borrowed_place,
borrowed.kind,
place,
access,
) {
debug!(
"each_borrow_involving_path: {:?} @ {:?} vs. {:?}/{:?}",
i, borrowed, place, access
Expand Down
28 changes: 20 additions & 8 deletions src/librustc_mir/borrow_check/places_conflict.rs
Expand Up @@ -12,7 +12,7 @@ use borrow_check::ArtificialField;
use borrow_check::Overlap;
use borrow_check::{Deep, Shallow, AccessDepth};
use rustc::hir;
use rustc::mir::{Mir, Place};
use rustc::mir::{BorrowKind, Mir, Place};
use rustc::mir::{Projection, ProjectionElem};
use rustc::ty::{self, TyCtxt};
use std::cmp::max;
Expand All @@ -21,6 +21,7 @@ pub(super) fn places_conflict<'gcx, 'tcx>(
tcx: TyCtxt<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
borrow_place: &Place<'tcx>,
borrow_kind: BorrowKind,
access_place: &Place<'tcx>,
access: AccessDepth,
) -> bool {
Expand All @@ -39,7 +40,14 @@ pub(super) fn places_conflict<'gcx, 'tcx>(

unroll_place(borrow_place, None, |borrow_components| {
unroll_place(access_place, None, |access_components| {
place_components_conflict(tcx, mir, borrow_components, access_components, access)
place_components_conflict(
tcx,
mir,
borrow_components,
borrow_kind,
access_components,
access
)
})
})
}
Expand All @@ -48,6 +56,7 @@ fn place_components_conflict<'gcx, 'tcx>(
tcx: TyCtxt<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
mut borrow_components: PlaceComponentsIter<'_, 'tcx>,
borrow_kind: BorrowKind,
mut access_components: PlaceComponentsIter<'_, 'tcx>,
access: AccessDepth,
) -> bool {
Expand Down Expand Up @@ -157,7 +166,8 @@ fn place_components_conflict<'gcx, 'tcx>(

match (elem, &base_ty.sty, access) {
(_, _, Shallow(Some(ArtificialField::Discriminant)))
| (_, _, Shallow(Some(ArtificialField::ArrayLength))) => {
| (_, _, Shallow(Some(ArtificialField::ArrayLength)))
| (_, _, Shallow(Some(ArtificialField::ShallowBorrow))) => {
// The discriminant and array length are like
// additional fields on the type; they do not
// overlap any existing data there. Furthermore,
Expand Down Expand Up @@ -225,11 +235,13 @@ fn place_components_conflict<'gcx, 'tcx>(
// If the second example, where we did, then we still know
// that the borrow can access a *part* of our place that
// our access cares about, so we still have a conflict.
//
// FIXME: Differs from AST-borrowck; includes drive-by fix
// to #38899. Will probably need back-compat mode flag.
debug!("places_conflict: full borrow, CONFLICT");
return true;
if borrow_kind == BorrowKind::Shallow && access_components.next().is_some() {
debug!("places_conflict: shallow borrow");
return false;
} else {
debug!("places_conflict: full borrow, CONFLICT");
return true;
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/build/matches/mod.rs
Expand Up @@ -1363,7 +1363,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// borrow of the whole match input. See additional
// discussion on rust-lang/rust#49870.
let borrow_kind = match borrow_kind {
BorrowKind::Shared | BorrowKind::Unique => borrow_kind,
BorrowKind::Shared
| BorrowKind::Shallow
| BorrowKind::Unique => borrow_kind,
BorrowKind::Mut { .. } => BorrowKind::Mut {
allow_two_phase_borrow: true,
},
Expand Down

0 comments on commit ced5c2d

Please sign in to comment.