Skip to content

Commit

Permalink
Rollup merge of rust-lang#56790 - rust-lang:borrowck-niche-discrimina…
Browse files Browse the repository at this point in the history
…nts, r=nikomatsakis

Make RValue::Discriminant a normal Shallow read

Enum layout optimizations mean that the discriminant of an enum may not be stored in a tag disjoint from the rest of the fields of the enum. Stop borrow checking as though they are.

Run with MIRI to see why this is needed: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=09a3236685a06b6096e2e2e3968b852c.

This issue exists with the lexical borrow checker as well (see rust-lang#45045) so migrate mode should prevent this from being immediately breaking.

r? @nikomatsakis

Fixes rust-lang#56797
  • Loading branch information
Centril authored Dec 16, 2018
2 parents f1fa9d4 + cdd5373 commit d91032a
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 24 deletions.
9 changes: 4 additions & 5 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
self.mutate_place(
ContextKind::SetDiscrim.new(location),
(place, span),
Shallow(Some(ArtificialField::Discriminant)),
Shallow(None),
JustWrite,
flow_state,
);
Expand Down Expand Up @@ -782,7 +782,6 @@ use self::AccessDepth::{Deep, Shallow};

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum ArtificialField {
Discriminant,
ArrayLength,
ShallowBorrow,
}
Expand Down Expand Up @@ -1191,14 +1190,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

Rvalue::Len(ref place) | Rvalue::Discriminant(ref place) => {
let af = match *rvalue {
Rvalue::Len(..) => ArtificialField::ArrayLength,
Rvalue::Discriminant(..) => ArtificialField::Discriminant,
Rvalue::Len(..) => Some(ArtificialField::ArrayLength),
Rvalue::Discriminant(..) => None,
_ => unreachable!(),
};
self.access_place(
context,
(place, span),
(Shallow(Some(af)), Read(ReadKind::Copy)),
(Shallow(af), Read(ReadKind::Copy)),
LocalMutationIsAllowed::No,
flow_state,
);
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/borrow_check/nll/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx, 'gcx> {
self.mutate_place(
ContextKind::SetDiscrim.new(location),
place,
Shallow(Some(ArtificialField::Discriminant)),
Shallow(None),
JustWrite,
);
}
Expand Down Expand Up @@ -360,14 +360,14 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {

Rvalue::Len(ref place) | Rvalue::Discriminant(ref place) => {
let af = match *rvalue {
Rvalue::Len(..) => ArtificialField::ArrayLength,
Rvalue::Discriminant(..) => ArtificialField::Discriminant,
Rvalue::Len(..) => Some(ArtificialField::ArrayLength),
Rvalue::Discriminant(..) => None,
_ => unreachable!(),
};
self.access_place(
context,
place,
(Shallow(Some(af)), Read(ReadKind::Copy)),
(Shallow(af), Read(ReadKind::Copy)),
LocalMutationIsAllowed::No,
);
}
Expand Down
13 changes: 5 additions & 8 deletions src/librustc_mir/borrow_check/places_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,12 @@ fn place_components_conflict<'gcx, 'tcx>(
let base_ty = base.ty(mir, tcx).to_ty(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,
// they cannot actually be a prefix of any
// borrowed place (at least in MIR as it is
// currently.)
// The array length is like additional fields on the
// type; it does not overlap any existing data there.
// Furthermore, if cannot actually be a prefix of any
// borrowed place (at least in MIR as it is currently.)
//
// e.g., a (mutable) borrow of `a[5]` while we read the
// array length of `a`.
Expand Down
33 changes: 31 additions & 2 deletions src/test/ui/borrowck/borrowck-anon-fields-variant.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
warning[E0503]: cannot use `y` because it was mutably borrowed
--> $DIR/borrowck-anon-fields-variant.rs:27:7
|
LL | Foo::Y(ref mut a, _) => a,
| --------- borrow of `y.0` occurs here
...
LL | Foo::Y(_, ref mut b) => b,
| ^^^^^^^^^^^^^^^^^^^^ use of borrowed `y.0`
...
LL | *a += 1;
| ------- borrow later used here
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

error[E0503]: cannot use `y` because it was mutably borrowed
--> $DIR/borrowck-anon-fields-variant.rs:44:7
|
LL | Foo::Y(ref mut a, _) => a,
| --------- borrow of `y.0` occurs here
...
LL | Foo::Y(ref mut b, _) => b, //~ ERROR cannot borrow
| ^^^^^^^^^^^^^^^^^^^^ use of borrowed `y.0`
...
LL | *a += 1;
| ------- borrow later used here

error[E0499]: cannot borrow `y.0` as mutable more than once at a time
--> $DIR/borrowck-anon-fields-variant.rs:44:14
|
Expand All @@ -10,6 +38,7 @@ LL | Foo::Y(ref mut b, _) => b, //~ ERROR cannot borrow
LL | *a += 1;
| ------- first borrow later used here

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0499`.
Some errors occurred: E0499, E0503.
For more information about an error, try `rustc --explain E0499`.
8 changes: 4 additions & 4 deletions src/test/ui/nll/match-on-borrowed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ fn enum_example(mut e: E) {
E::V(ref mut x, _) => x,
E::W => panic!(),
};
match e { // OK, no access of borrowed data
match e { // Don't know that E uses a tag for its discriminant
_ if false => (),
E::V(_, r) => (),
E::V(_, r) => (), //~ ERROR
E::W => (),
}
x;
Expand All @@ -59,9 +59,9 @@ fn indirect_enum_example(mut f: &mut E) {
E::V(ref mut x, _) => x,
E::W => panic!(),
};
match f { // OK, no access of borrowed data
match f { // Don't know that E uses a tag for its discriminant
_ if false => (),
E::V(_, r) => (),
E::V(_, r) => (), //~ ERROR
E::W => (),
}
x;
Expand Down
26 changes: 25 additions & 1 deletion src/test/ui/nll/match-on-borrowed.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,27 @@
error[E0503]: cannot use `e` because it was mutably borrowed
--> $DIR/match-on-borrowed.rs:51:9
|
LL | E::V(ref mut x, _) => x,
| --------- borrow of `e.0` occurs here
...
LL | E::V(_, r) => (), //~ ERROR
| ^^^^^^^^^^ use of borrowed `e.0`
...
LL | x;
| - borrow later used here

error[E0503]: cannot use `*f` because it was mutably borrowed
--> $DIR/match-on-borrowed.rs:64:9
|
LL | E::V(ref mut x, _) => x,
| --------- borrow of `f.0` occurs here
...
LL | E::V(_, r) => (), //~ ERROR
| ^^^^^^^^^^ use of borrowed `f.0`
...
LL | x;
| - borrow later used here

error[E0503]: cannot use `t` because it was mutably borrowed
--> $DIR/match-on-borrowed.rs:82:9
|
Expand All @@ -16,7 +40,7 @@ error[E0381]: use of possibly uninitialized variable: `n`
LL | match n {} //~ ERROR
| ^ use of possibly uninitialized `n`

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

Some errors occurred: E0381, E0503.
For more information about an error, try `rustc --explain E0381`.

0 comments on commit d91032a

Please sign in to comment.