Skip to content

Commit b42a49e

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

File tree

11 files changed

+103
-20
lines changed

11 files changed

+103
-20
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
@@ -754,6 +754,7 @@ use self::ShallowOrDeep::{Deep, Shallow};
754754
enum ArtificialField {
755755
Discriminant,
756756
ArrayLength,
757+
ShallowBorrow,
757758
}
758759

759760
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -1200,7 +1201,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
12001201
Control::Continue
12011202
}
12021203

1203-
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
1204+
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
1205+
| (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
1206+
Control::Continue
1207+
}
1208+
1209+
(Write(WriteKind::Move), BorrowKind::Shallow) => {
1210+
// Handled by initialization checks.
12041211
Control::Continue
12051212
}
12061213

@@ -1336,6 +1343,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
13361343
match *rvalue {
13371344
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
13381345
let access_kind = match bk {
1346+
BorrowKind::Shallow => {
1347+
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
1348+
},
13391349
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
13401350
BorrowKind::Unique | BorrowKind::Mut { .. } => {
13411351
let wk = WriteKind::MutableBorrow(bk);
@@ -1543,11 +1553,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
15431553
return;
15441554
}
15451555

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

1550-
if places_conflict::places_conflict(self.infcx.tcx, self.mir, place, root_place, sd) {
1558+
if places_conflict::borrow_conflicts_with_place(
1559+
self.infcx.tcx,
1560+
self.mir,
1561+
place,
1562+
borrow.kind,
1563+
root_place,
1564+
sd
1565+
) {
15511566
debug!("check_for_invalidation_at_exit({:?}): INVALID", place);
15521567
// FIXME: should be talking about the region lifetime instead
15531568
// of just a span here.
@@ -1597,7 +1612,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
15971612

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

@@ -1897,7 +1912,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
18971912
let is_local_mutation_allowed = match borrow_kind {
18981913
BorrowKind::Unique => LocalMutationIsAllowed::Yes,
18991914
BorrowKind::Mut { .. } => is_local_mutation_allowed,
1900-
BorrowKind::Shared => unreachable!(),
1915+
BorrowKind::Shared | BorrowKind::Shallow => unreachable!(),
19011916
};
19021917
match self.is_mutable(place, is_local_mutation_allowed) {
19031918
Ok(root_place) => {
@@ -1927,8 +1942,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
19271942
| Write(wk @ WriteKind::Move)
19281943
| Reservation(wk @ WriteKind::StorageDeadOrDrop(_))
19291944
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
1945+
| Reservation(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow))
19301946
| Write(wk @ WriteKind::StorageDeadOrDrop(_))
1931-
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared)) => {
1947+
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shared))
1948+
| Write(wk @ WriteKind::MutableBorrow(BorrowKind::Shallow)) => {
19321949
if let Err(_place_err) = self.is_mutable(place, is_local_mutation_allowed) {
19331950
if self.infcx.tcx.migrate_borrowck() {
19341951
// rust-lang/rust#46908: In pure NLL mode this
@@ -1971,6 +1988,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
19711988
Read(ReadKind::Borrow(BorrowKind::Unique))
19721989
| Read(ReadKind::Borrow(BorrowKind::Mut { .. }))
19731990
| Read(ReadKind::Borrow(BorrowKind::Shared))
1991+
| Read(ReadKind::Borrow(BorrowKind::Shallow))
19741992
| Read(ReadKind::Copy) => {
19751993
// Access authorized
19761994
return false;

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cg, 'cx, 'tcx, 'gcx> {
409409
match *rvalue {
410410
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
411411
let access_kind = match bk {
412+
BorrowKind::Shallow => {
413+
(Shallow(Some(ArtificialField::ShallowBorrow)), Read(ReadKind::Borrow(bk)))
414+
},
412415
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
413416
BorrowKind::Unique | BorrowKind::Mut { .. } => {
414417
let wk = WriteKind::MutableBorrow(bk);
@@ -519,6 +522,10 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cg, 'cx, 'tcx, 'gcx> {
519522
// have already taken the reservation
520523
}
521524

525+
(Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
526+
// Sccess_place be called for BorrowKind::Match.
527+
unreachable!();
528+
}
522529
(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
523530
// Reads/reservations don't invalidate shared borrows
524531
}

‎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, ShallowOrDeep};
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: ShallowOrDeep,
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: ShallowOrDeep,
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,
@@ -215,11 +225,13 @@ fn place_components_conflict<'gcx, 'tcx>(
215225
// If the second example, where we did, then we still know
216226
// that the borrow can access a *part* of our place that
217227
// our access cares about, so we still have a conflict.
218-
//
219-
// FIXME: Differs from AST-borrowck; includes drive-by fix
220-
// to #38899. Will probably need back-compat mode flag.
221-
debug!("places_conflict: full borrow, CONFLICT");
222-
return true;
228+
if borrow_kind == BorrowKind::Shallow && access_components.next().is_some() {
229+
debug!("places_conflict: shallow borrow");
230+
return false;
231+
} else {
232+
debug!("places_conflict: full borrow, CONFLICT");
233+
return true;
234+
}
223235
}
224236
}
225237
}

‎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)