From ced5c2d08af52de40d104687c2b6066d9daedecf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 10 Sep 2018 22:33:45 +0100 Subject: [PATCH] Add "Shallow" borrow kind This allows treating the "fake" match borrows differently from shared borrows. --- src/librustc/ich/impls_mir.rs | 1 + src/librustc/mir/mod.rs | 24 +++++++++++++- src/librustc/mir/tcx.rs | 4 +++ src/librustc/mir/visit.rs | 5 ++- src/librustc_mir/borrow_check/borrow_set.rs | 4 ++- .../borrow_check/error_reporting.rs | 5 +++ src/librustc_mir/borrow_check/mod.rs | 32 +++++++++++++++---- .../borrow_check/nll/invalidation.rs | 8 +++-- src/librustc_mir/borrow_check/path_utils.rs | 9 +++++- .../borrow_check/places_conflict.rs | 28 +++++++++++----- src/librustc_mir/build/matches/mod.rs | 4 ++- 11 files changed, 102 insertions(+), 22 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 313ef054829..e145e87a089 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -46,6 +46,7 @@ fn hash_stable(&self, 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); diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 98d9a0a7c6f..21c2299eac0 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -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 @@ -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, @@ -2198,6 +2219,7 @@ fn fmt(&self, fmt: &mut Formatter) -> fmt::Result { Ref(region, borrow_kind, ref place) => { let kind_str = match borrow_kind { BorrowKind::Shared => "", + BorrowKind::Shallow => "shallow ", BorrowKind::Mut { .. } | BorrowKind::Unique => "mut ", }; diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index c928be4f9df..2a25e057a71 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -287,6 +287,10 @@ pub fn to_mutbl_lossy(self) -> hir::Mutability { // 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, } } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 91c83ecb2e2..6de7e2215bf 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -963,6 +963,7 @@ pub fn is_mutating_use(&self) -> bool { 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 | @@ -974,7 +975,9 @@ pub fn is_mutating_use(&self) -> bool { /// 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, diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index bb70b4b76c2..bcf37722130 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -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 ", }; @@ -287,7 +288,8 @@ fn visit_place( 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 } _ => { diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 1d91fa365d5..f0dd8a9feb8 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -333,6 +333,11 @@ pub(super) fn report_conflicting_borrow( Origin::Mir, ), + (BorrowKind::Mut { .. }, _, _, BorrowKind::Shallow, _, _) + | (BorrowKind::Unique, _, _, BorrowKind::Shallow, _, _) => { + return; + } + (BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure( span, &desc_place, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 769e1097bff..34fe9408eaf 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -755,6 +755,7 @@ enum MutateMode { enum ArtificialField { Discriminant, ArrayLength, + ShallowBorrow, } #[derive(Copy, Clone, PartialEq, Eq, Debug)] @@ -972,7 +973,13 @@ fn check_access_for_conflict( 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 } @@ -1108,6 +1115,9 @@ fn consume_rvalue( 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); @@ -1315,11 +1325,16 @@ fn check_for_invalidation_at_exit( 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. @@ -1369,7 +1384,7 @@ fn check_activations( // only mutable borrows should be 2-phase assert!(match borrow.kind { - BorrowKind::Shared => false, + BorrowKind::Shared | BorrowKind::Shallow => false, BorrowKind::Unique | BorrowKind::Mut { .. } => true, }); @@ -1669,7 +1684,7 @@ fn check_access_permissions( 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) => { @@ -1699,8 +1714,10 @@ fn check_access_permissions( | 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 @@ -1743,6 +1760,7 @@ fn check_access_permissions( 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; diff --git a/src/librustc_mir/borrow_check/nll/invalidation.rs b/src/librustc_mir/borrow_check/nll/invalidation.rs index 3d387963da9..1246f7120c4 100644 --- a/src/librustc_mir/borrow_check/nll/invalidation.rs +++ b/src/librustc_mir/borrow_check/nll/invalidation.rs @@ -329,6 +329,9 @@ fn consume_rvalue( 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); @@ -439,8 +442,9 @@ fn check_access_for_conflict( // 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 { .. }) => { diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs index 67787d23db6..ccb7a405626 100644 --- a/src/librustc_mir/borrow_check/path_utils.rs +++ b/src/librustc_mir/borrow_check/path_utils.rs @@ -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 diff --git a/src/librustc_mir/borrow_check/places_conflict.rs b/src/librustc_mir/borrow_check/places_conflict.rs index e371b34c480..13ac1d60c95 100644 --- a/src/librustc_mir/borrow_check/places_conflict.rs +++ b/src/librustc_mir/borrow_check/places_conflict.rs @@ -12,7 +12,7 @@ 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; @@ -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 { @@ -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 + ) }) }) } @@ -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 { @@ -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, @@ -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; + } } } } diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index c30dcdafdb4..235440e2841 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -1363,7 +1363,9 @@ fn bind_matched_candidate_for_guard( // 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, }, -- 2.44.0