Skip to content

Commit be48620

Browse files
committedSep 24, 2018
Auto merge of #53438 - matthewjasper:permissive-match-access, r=pnkfelix
[NLL] Be more permissive when checking access due to Match Partially addresses #53114. notably, we should now have parity with AST borrowck. Matching on uninitialized values is still forbidden. * ~~Give fake borrows for match their own `BorrowKind`~~ * ~~Allow borrows with this kind to happen on values that are already mutably borrowed.~~ * ~~Track borrows with this type even behind shared reference dereferences and consider all accesses to be deep when checking for conflicts with this borrow type. See [src/test/ui/issues/issue-27282-mutate-before-diverging-arm-3.rs](cb5c989#diff-a2126cd3263a1f5342e2ecd5e699fbc6) for an example soundness issue this fixes (a case of #27282 that wasn't handled correctly).~~ * Create a new `BorrowKind`: `Shallow` (name can be bike-shed) * `Shallow` borrows differ from shared borrows in that * When we check for access we treat them as a `Shallow(Some(_))` read * When we check for conflicts with them, if the borrow place is a strict prefix of the access place then we don't consider that a conflict. * For example, a `Shallow` borrow of `x` does not conflict with any access or borrow of `x.0` or `*x` * Remove the current fake borrow in matches. * When building matches, we take a `Shallow` borrow of any `Place` that we switch on or bind in a match, and any prefix of those places. (There are some optimizations where we do fewer borrows, but this shouldn't change semantics) * `match x { &Some(1) => (), _ => (), }` would `Shallow` borrow `x`, `*x` and `(*x as Some).0` (the `*x` borrow is unnecessary, but I'm not sure how easy it would be to remove.) * Replace the fake discriminant read with a `ReadForMatch`. * Change ReadForMatch to only check for initializedness (to prevent `let x: !; match x {}`), but not conflicting borrows. It is still considered a use for liveness and `unsafe` checking. * Give special cased error messages for this kind of borrow. Table from the above issue after this PR | Thing | AST | MIR | Want | Example | | --- | --- | --- | --- |---| | `let _ = <unsafe-field>` | 💚 | 💚 | ❌ | [playground](https://play.rust-lang.org/?gist=bb7843e42fa5318c1043d04bd72abfe4&version=nightly&mode=debug&edition=2015) | | `match <unsafe_field> { _ => () }` | ❌ | ❌ | ❌ | [playground](https://play.rust-lang.org/?gist=3e3af05fbf1fae28fab2aaf9412fb2ea&version=nightly&mode=debug&edition=2015) | | `let _ = <moved>` | 💚 | 💚 | 💚 | [playground](https://play.rust-lang.org/?gist=91a6efde8288558e584aaeee0a50558b&version=nightly&mode=debug&edition=2015) | | `match <moved> { _ => () }` | ❌ | ❌ | 💚 | [playground](https://play.rust-lang.org/?gist=804f8185040b2fe131f2c4a64b3048ca&version=nightly&mode=debug&edition=2015) | | `let _ = <borrowed>` | 💚 | 💚 | 💚 | [playground](https://play.rust-lang.org/?gist=0e487c2893b89cb772ec2f2b7c5da876&version=nightly&mode=debug&edition=2015) | | `match <borrowed> { _ => () }` | 💚 | 💚 | 💚 | [playground](https://play.rust-lang.org/?gist=0e487c2893b89cb772ec2f2b7c5da876&version=nightly&mode=debug&edition=2015) | r? @nikomatsakis
2 parents e5c6575 + 8916946 commit be48620

File tree

53 files changed

+1196
-904
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+1196
-904
lines changed
 

‎src/librustc/ich/impls_mir.rs

+2-1
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);
@@ -272,7 +273,7 @@ for mir::StatementKind<'gcx> {
272273
}
273274
}
274275

275-
impl_stable_hash_for!(enum mir::FakeReadCause { ForMatch, ForLet });
276+
impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet });
276277

277278
impl<'a, 'gcx, T> HashStable<StableHashingContext<'a>>
278279
for mir::ValidationOperand<'gcx, T>

‎src/librustc/mir/mod.rs

+33-7
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,32 @@ impl From<Mutability> for hir::Mutability {
451451
}
452452
}
453453

454-
#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)]
454+
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, RustcEncodable, RustcDecodable)]
455455
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,
@@ -1672,7 +1693,11 @@ pub enum FakeReadCause {
16721693
///
16731694
/// This should ensure that you cannot change the variant for an enum
16741695
/// while you are in the midst of matching on it.
1675-
ForMatch,
1696+
ForMatchGuard,
1697+
1698+
/// `let x: !; match x {}` doesn't generate any read of x so we need to
1699+
/// generate a read of x to check that it is initialized and safe.
1700+
ForMatchedPlace,
16761701

16771702
/// Officially, the semantics of
16781703
///
@@ -1773,7 +1798,7 @@ impl<'tcx> Debug for Statement<'tcx> {
17731798

17741799
/// A path to a value; something that can be evaluated without
17751800
/// changing or disturbing program state.
1776-
#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
1801+
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
17771802
pub enum Place<'tcx> {
17781803
/// local variable
17791804
Local(Local),
@@ -1790,7 +1815,7 @@ pub enum Place<'tcx> {
17901815

17911816
/// The def-id of a static, along with its normalized type (which is
17921817
/// stored to avoid requiring normalization when reading MIR).
1793-
#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
1818+
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
17941819
pub struct Static<'tcx> {
17951820
pub def_id: DefId,
17961821
pub ty: Ty<'tcx>,
@@ -1805,13 +1830,13 @@ impl_stable_hash_for!(struct Static<'tcx> {
18051830
/// or `*B` or `B[index]`. Note that it is parameterized because it is
18061831
/// shared between `Constant` and `Place`. See the aliases
18071832
/// `PlaceProjection` etc below.
1808-
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
1833+
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
18091834
pub struct Projection<'tcx, B, V, T> {
18101835
pub base: B,
18111836
pub elem: ProjectionElem<'tcx, V, T>,
18121837
}
18131838

1814-
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
1839+
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
18151840
pub enum ProjectionElem<'tcx, V, T> {
18161841
Deref,
18171842
Field(Field, T),
@@ -2198,6 +2223,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
21982223
Ref(region, borrow_kind, ref place) => {
21992224
let kind_str = match borrow_kind {
22002225
BorrowKind::Shared => "",
2226+
BorrowKind::Shallow => "shallow ",
22012227
BorrowKind::Mut { .. } | BorrowKind::Unique => "mut ",
22022228
};
22032229

0 commit comments

Comments
 (0)