Skip to content

Commit ced5c2d

Browse files
matthewjaspermatthewjasper
committed
Add "Shallow" borrow kind
This allows treating the "fake" match borrows differently from shared borrows.
1 parent a072d1b commit ced5c2d

File tree

11 files changed

+102
-22
lines changed

11 files changed

+102
-22
lines changed

‎src/librustc/ich/impls_mir.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ for mir::BorrowKind {
4646

4747
match *self {
4848
mir::BorrowKind::Shared |
49+
mir::BorrowKind::Shallow |
4950
mir::BorrowKind::Unique => {}
5051
mir::BorrowKind::Mut { allow_two_phase_borrow } => {
5152
allow_two_phase_borrow.hash_stable(hcx, hasher);

‎src/librustc/mir/mod.rs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,27 @@ pub enum BorrowKind {
456456
/// Data must be immutable and is aliasable.
457457
Shared,
458458

459+
/// The immediately borrowed place must be immutable, but projections from
460+
/// it don't need to be. For example, a shallow borrow of `a.b` doesn't
461+
/// conflict with a mutable borrow of `a.b.c`.
462+
///
463+
/// This is used when lowering matches: when matching on a place we want to
464+
/// ensure that place have the same value from the start of the match until
465+
/// an arm is selected. This prevents this code from compiling:
466+
///
467+
/// let mut x = &Some(0);
468+
/// match *x {
469+
/// None => (),
470+
/// Some(_) if { x = &None; false } => (),
471+
/// Some(_) => (),
472+
/// }
473+
///
474+
/// This can't be a shared borrow because mutably borrowing (*x as Some).0
475+
/// should not prevent `if let None = x { ... }`, for example, becase the
476+
/// mutating `(*x as Some).0` can't affect the discriminant of `x`.
477+
/// We can also report errors with this kind of borrow differently.
478+
Shallow,
479+
459480
/// Data must be immutable but not aliasable. This kind of borrow
460481
/// cannot currently be expressed by the user and is used only in
461482
/// implicit closure bindings. It is needed when the closure is
@@ -504,7 +525,7 @@ pub enum BorrowKind {
504525
impl BorrowKind {
505526
pub fn allows_two_phase_borrow(&self) -> bool {
506527
match *self {
507-
BorrowKind::Shared | BorrowKind::Unique => false,
528+
BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => false,
508529
BorrowKind::Mut {
509530
allow_two_phase_borrow,
510531
} => allow_two_phase_borrow,
@@ -2198,6 +2219,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
21982219
Ref(region, borrow_kind, ref place) => {
21992220
let kind_str = match borrow_kind {
22002221
BorrowKind::Shared => "",
2222+
BorrowKind::Shallow => "shallow ",
22012223
BorrowKind::Mut { .. } | BorrowKind::Unique => "mut ",
22022224
};
22032225

‎src/librustc/mir/tcx.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,10 @@ impl BorrowKind {
287287
// use `&mut`. It gives all the capabilities of an `&uniq`
288288
// and hence is a safe "over approximation".
289289
BorrowKind::Unique => hir::MutMutable,
290+
291+
// We have no type corresponding to a shallow borrow, so use
292+
// `&` as an approximation.
293+
BorrowKind::Shallow => hir::MutImmutable,
290294
}
291295
}
292296
}

‎src/librustc/mir/visit.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -963,6 +963,7 @@ impl<'tcx> PlaceContext<'tcx> {
963963

964964
PlaceContext::Inspect |
965965
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
966+
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
966967
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
967968
PlaceContext::Projection(Mutability::Not) |
968969
PlaceContext::Copy | PlaceContext::Move |
@@ -974,7 +975,9 @@ impl<'tcx> PlaceContext<'tcx> {
974975
/// Returns true if this place context represents a use that does not change the value.
975976
pub fn is_nonmutating_use(&self) -> bool {
976977
match *self {
977-
PlaceContext::Inspect | PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
978+
PlaceContext::Inspect |
979+
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
980+
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
978981
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
979982
PlaceContext::Projection(Mutability::Not) |
980983
PlaceContext::Copy | PlaceContext::Move => true,

‎src/librustc_mir/borrow_check/borrow_set.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
8787
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
8888
let kind = match self.kind {
8989
mir::BorrowKind::Shared => "",
90+
mir::BorrowKind::Shallow => "shallow ",
9091
mir::BorrowKind::Unique => "uniq ",
9192
mir::BorrowKind::Mut { .. } => "mut ",
9293
};
@@ -287,7 +288,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
287288
borrow_data.activation_location = match context {
288289
// The use of TMP in a shared borrow does not
289290
// count as an actual activation.
290-
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
291+
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
292+
| PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
291293
TwoPhaseActivation::NotActivated
292294
}
293295
_ => {

‎src/librustc_mir/borrow_check/error_reporting.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
333333
Origin::Mir,
334334
),
335335

336+
(BorrowKind::Mut { .. }, _, _, BorrowKind::Shallow, _, _)
337+
| (BorrowKind::Unique, _, _, BorrowKind::Shallow, _, _) => {
338+
return;
339+
}
340+
336341
(BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure(
337342
span,
338343
&desc_place,

‎src/librustc_mir/borrow_check/mod.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,7 @@ use self::AccessDepth::{Deep, Shallow};
755755
enum ArtificialField {
756756
Discriminant,
757757
ArrayLength,
758+
ShallowBorrow,
758759
}
759760

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

975-
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
976+
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
977+
| (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
978+
Control::Continue
979+
}
980+
981+
(Write(WriteKind::Move), BorrowKind::Shallow) => {
982+
// Handled by initialization checks.
976983
Control::Continue
977984
}
978985

@@ -1108,6 +1115,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
11081115
match *rvalue {
11091116
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
11101117
let access_kind = match bk {
1118+
BorrowKind::Shallow => {
1119+
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
1120+
},
11111121
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
11121122
BorrowKind::Unique | BorrowKind::Mut { .. } => {
11131123
let wk = WriteKind::MutableBorrow(bk);
@@ -1315,11 +1325,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
13151325
return;
13161326
}
13171327

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

1322-
if places_conflict::places_conflict(self.infcx.tcx, self.mir, place, root_place, sd) {
1330+
if places_conflict::borrow_conflicts_with_place(
1331+
self.infcx.tcx,
1332+
self.mir,
1333+
place,
1334+
borrow.kind,
1335+
root_place,
1336+
sd
1337+
) {
13231338
debug!("check_for_invalidation_at_exit({:?}): INVALID", place);
13241339
// FIXME: should be talking about the region lifetime instead
13251340
// of just a span here.
@@ -1369,7 +1384,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
13691384

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

@@ -1669,7 +1684,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
16691684
let is_local_mutation_allowed = match borrow_kind {
16701685
BorrowKind::Unique => LocalMutationIsAllowed::Yes,
16711686
BorrowKind::Mut { .. } => is_local_mutation_allowed,
1672-
BorrowKind::Shared => unreachable!(),
1687+
BorrowKind::Shared | BorrowKind::Shallow => unreachable!(),
16731688
};
16741689
match self.is_mutable(place, is_local_mutation_allowed) {
16751690
Ok(root_place) => {
@@ -1699,8 +1714,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
16991714
| Write(wk @ WriteKind::Move)
17001715
| Reservation(wk @ WriteKind::StorageDeadOrDrop)
17011716
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
1717+
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow))
17021718
| Write(wk @ WriteKind::StorageDeadOrDrop)
1703-
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared)) => {
1719+
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
1720+
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow)) => {
17041721
if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) {
17051722
if self.infcx.tcx.migrate_borrowck() {
17061723
// rust-lang/rust#46908: In pure NLL mode this
@@ -1743,6 +1760,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
17431760
Read(ReadKind::Borrow(BorrowKind::Unique))
17441761
| Read(ReadKind::Borrow(BorrowKind::Mut { .. }))
17451762
| Read(ReadKind::Borrow(BorrowKind::Shared))
1763+
| Read(ReadKind::Borrow(BorrowKind::Shallow))
17461764
| Read(ReadKind::Copy) => {
17471765
// Access authorized
17481766
return false;

‎src/librustc_mir/borrow_check/nll/invalidation.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
329329
match *rvalue {
330330
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
331331
let access_kind = match bk {
332+
BorrowKind::Shallow => {
333+
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
334+
},
332335
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
333336
BorrowKind::Unique | BorrowKind::Mut { .. } => {
334337
let wk = WriteKind::MutableBorrow(bk);
@@ -439,8 +442,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
439442
// have already taken the reservation
440443
}
441444

442-
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
443-
// Reads/reservations don't invalidate shared borrows
445+
(Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
446+
| (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
447+
// Reads/reservations don't invalidate shared or shallow borrows
444448
}
445449

446450
(Read(_), BorrowKind::Unique) | (Read(_), BorrowKind::Mut { .. }) => {

‎src/librustc_mir/borrow_check/path_utils.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,14 @@ pub(super) fn each_borrow_involving_path<'a, 'tcx, 'gcx: 'tcx, F, I, S> (
6161
for i in candidates {
6262
let borrowed = &borrow_set[i];
6363

64-
if places_conflict::places_conflict(tcx, mir, &borrowed.borrowed_place, place, access) {
64+
if places_conflict::places_conflict(
65+
tcx,
66+
mir,
67+
&borrowed.borrowed_place,
68+
borrowed.kind,
69+
place,
70+
access,
71+
) {
6572
debug!(
6673
"each_borrow_involving_path: {:?} @ {:?} vs. {:?}/{:?}",
6774
i, borrowed, place, access

‎src/librustc_mir/borrow_check/places_conflict.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use borrow_check::ArtificialField;
1212
use borrow_check::Overlap;
1313
use borrow_check::{Deep, Shallow, AccessDepth};
1414
use rustc::hir;
15-
use rustc::mir::{Mir, Place};
15+
use rustc::mir::{BorrowKind, Mir, Place};
1616
use rustc::mir::{Projection, ProjectionElem};
1717
use rustc::ty::{self, TyCtxt};
1818
use std::cmp::max;
@@ -21,6 +21,7 @@ pub(super) fn places_conflict<'gcx, 'tcx>(
2121
tcx: TyCtxt<'_, 'gcx, 'tcx>,
2222
mir: &Mir<'tcx>,
2323
borrow_place: &Place<'tcx>,
24+
borrow_kind: BorrowKind,
2425
access_place: &Place<'tcx>,
2526
access: AccessDepth,
2627
) -> bool {
@@ -39,7 +40,14 @@ pub(super) fn places_conflict<'gcx, 'tcx>(
3940

4041
unroll_place(borrow_place, None, |borrow_components| {
4142
unroll_place(access_place, None, |access_components| {
42-
place_components_conflict(tcx, mir, borrow_components, access_components, access)
43+
place_components_conflict(
44+
tcx,
45+
mir,
46+
borrow_components,
47+
borrow_kind,
48+
access_components,
49+
access
50+
)
4351
})
4452
})
4553
}
@@ -48,6 +56,7 @@ fn place_components_conflict<'gcx, 'tcx>(
4856
tcx: TyCtxt<'_, 'gcx, 'tcx>,
4957
mir: &Mir<'tcx>,
5058
mut borrow_components: PlaceComponentsIter<'_, 'tcx>,
59+
borrow_kind: BorrowKind,
5160
mut access_components: PlaceComponentsIter<'_, 'tcx>,
5261
access: AccessDepth,
5362
) -> bool {
@@ -157,7 +166,8 @@ fn place_components_conflict<'gcx, 'tcx>(
157166

158167
match (elem, &base_ty.sty, access) {
159168
(_, _, Shallow(Some(ArtificialField::Discriminant)))
160-
| (_, _, Shallow(Some(ArtificialField::ArrayLength))) => {
169+
| (_, _, Shallow(Some(ArtificialField::ArrayLength)))
170+
| (_, _, Shallow(Some(ArtificialField::ShallowBorrow))) => {
161171
// The discriminant and array length are like
162172
// additional fields on the type; they do not
163173
// overlap any existing data there. Furthermore,
@@ -225,11 +235,13 @@ fn place_components_conflict<'gcx, 'tcx>(
225235
// If the second example, where we did, then we still know
226236
// that the borrow can access a *part* of our place that
227237
// our access cares about, so we still have a conflict.
228-
//
229-
// FIXME: Differs from AST-borrowck; includes drive-by fix
230-
// to #38899. Will probably need back-compat mode flag.
231-
debug!("places_conflict: full borrow, CONFLICT");
232-
return true;
238+
if borrow_kind == BorrowKind::Shallow && access_components.next().is_some() {
239+
debug!("places_conflict: shallow borrow");
240+
return false;
241+
} else {
242+
debug!("places_conflict: full borrow, CONFLICT");
243+
return true;
244+
}
233245
}
234246
}
235247
}

‎src/librustc_mir/build/matches/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1363,7 +1363,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
13631363
// borrow of the whole match input. See additional
13641364
// discussion on rust-lang/rust#49870.
13651365
let borrow_kind = match borrow_kind {
1366-
BorrowKind::Shared | BorrowKind::Unique => borrow_kind,
1366+
BorrowKind::Shared
1367+
| BorrowKind::Shallow
1368+
| BorrowKind::Unique => borrow_kind,
13671369
BorrowKind::Mut { .. } => BorrowKind::Mut {
13681370
allow_two_phase_borrow: true,
13691371
},

0 commit comments

Comments
 (0)