Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use normal mutable borrows in matches #57609

Merged
merged 10 commits into from
Feb 25, 2019
7 changes: 6 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,12 @@ impl_stable_hash_for!(impl<'gcx> for enum mir::StatementKind<'gcx> [ mir::Statem
});

impl_stable_hash_for!(enum mir::RetagKind { FnEntry, TwoPhase, Raw, Default });
impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet });
impl_stable_hash_for!(enum mir::FakeReadCause {
ForMatchGuard,
ForMatchedPlace,
ForGuardBinding,
ForLet
});

impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::Place<'gcx> {
fn hash_stable<W: StableHasherResult>(&self,
Expand Down
13 changes: 9 additions & 4 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,17 +1823,22 @@ pub enum RetagKind {
/// The `FakeReadCause` describes the type of pattern why a `FakeRead` statement exists.
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
pub enum FakeReadCause {
/// Inject a fake read of the borrowed input at the start of each arm's
/// pattern testing code.
/// Inject a fake read of the borrowed input at the end of each guards
/// code.
///
/// This should ensure that you cannot change the variant for an enum
/// while you are in the midst of matching on it.
/// This should ensure that you cannot change the variant for an enum while
/// you are in the midst of matching on it.
ForMatchGuard,

/// `let x: !; match x {}` doesn't generate any read of x so we need to
/// generate a read of x to check that it is initialized and safe.
ForMatchedPlace,

/// A fake read of the RefWithinGuard version of a bind-by-value variable
/// in a match guard to ensure that it's value hasn't change by the time
/// we create the OutsideGuard version.
ForGuardBinding,

/// Officially, the semantics of
///
/// `let pattern = <expr>;`
Expand Down
44 changes: 16 additions & 28 deletions src/librustc_mir/borrow_check/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ use crate::borrow_check::nll::ToRegionVid;
use crate::dataflow::indexes::BorrowIndex;
use crate::dataflow::move_paths::MoveData;
use rustc::mir::traversal;
use rustc::mir::visit::{
PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext
};
use rustc::mir::visit::{PlaceContext, Visitor, NonUseContext, MutatingUseContext};
use rustc::mir::{self, Location, Mir, Local};
use rustc::ty::{RegionVid, TyCtxt};
use rustc::util::nodemap::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -257,31 +255,21 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
);
}

// Otherwise, this is the unique later use
// that we expect.
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
TwoPhaseActivation::NotActivated,
_ => {
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
// we've not found any other activations (checked above).
assert_eq!(
borrow_data.activation_location,
TwoPhaseActivation::NotActivated,
"never found an activation for this borrow!",
);

self.activation_map
.entry(location)
.or_default()
.push(borrow_index);
TwoPhaseActivation::ActivatedAt(location)
}
};
// Otherwise, this is the unique later use that we expect.
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
// we've not found any other activations (checked above).
assert_eq!(
borrow_data.activation_location,
TwoPhaseActivation::NotActivated,
"never found an activation for this borrow!",
);
self.activation_map
.entry(location)
.or_default()
.push(borrow_index);

borrow_data.activation_location = TwoPhaseActivation::ActivatedAt(location);
}
}

Expand Down
30 changes: 19 additions & 11 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1330,22 +1330,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let loan_span = loan_spans.args_or_use();

let tcx = self.infcx.tcx;
let mut err = if loan.kind == BorrowKind::Shallow {
tcx.cannot_mutate_in_match_guard(
if loan.kind == BorrowKind::Shallow {
let mut err = tcx.cannot_mutate_in_match_guard(
span,
loan_span,
&self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
"assign",
Origin::Mir,
)
} else {
tcx.cannot_assign_to_borrowed(
span,
loan_span,
&self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
Origin::Mir,
)
};
);
loan_spans.var_span_label(
&mut err,
format!("borrow occurs due to use{}", loan_spans.describe()),
);

err.buffer(&mut self.errors_buffer);

return;
}

let mut err = tcx.cannot_assign_to_borrowed(
span,
loan_span,
&self.describe_place(place).unwrap_or_else(|| "_".to_owned()),
Origin::Mir,
);

loan_spans.var_span_label(
&mut err,
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

(Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
| (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow) => {
| (Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
Control::Continue
}

Expand Down
13 changes: 5 additions & 8 deletions src/librustc_mir/borrow_check/nll/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,8 @@ impl<'cx, 'tcx, 'gcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx, 'gcx> {
JustWrite
);
}
StatementKind::FakeRead(_, ref place) => {
self.access_place(
ContextKind::FakeRead.new(location),
place,
(Deep, Read(ReadKind::Borrow(BorrowKind::Shared))),
LocalMutationIsAllowed::No,
);
StatementKind::FakeRead(_, _) => {
// Only relavent for initialized/liveness/safety checks.
}
StatementKind::SetDiscriminant {
ref place,
Expand Down Expand Up @@ -438,7 +433,9 @@ impl<'cg, 'cx, 'tcx, 'gcx> InvalidationGenerator<'cx, 'tcx, 'gcx> {
}

(Read(_), BorrowKind::Shallow) | (Reservation(..), BorrowKind::Shallow)
| (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared) => {
| (Read(_), BorrowKind::Shared) | (Reservation(..), BorrowKind::Shared)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Unique)
| (Read(ReadKind::Borrow(BorrowKind::Shallow)), BorrowKind::Mut { .. }) => {
// Reads/reservations don't invalidate shared or shallow borrows
}

Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ use rustc::mir::*;
use rustc::hir;
use syntax_pos::Span;

use std::slice;

impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
pub fn ast_block(&mut self,
destination: &Place<'tcx>,
Expand Down Expand Up @@ -125,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
None,
remainder_span,
lint_level,
slice::from_ref(&pattern),
&pattern,
ArmHasGuard(false),
Some((None, initializer_span)),
);
Expand All @@ -138,7 +136,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
}));
} else {
scope = this.declare_bindings(
None, remainder_span, lint_level, slice::from_ref(&pattern),
None, remainder_span, lint_level, &pattern,
ArmHasGuard(false), None);

debug!("ast_block_stmts: pattern={:?}", pattern);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ExprKind::Block { body: ast_block } => {
this.ast_block(destination, block, ast_block, source_info)
}
ExprKind::Match { discriminant, arms } => {
this.match_expr(destination, expr_span, block, discriminant, arms)
ExprKind::Match { scrutinee, arms } => {
this.match_expr(destination, expr_span, block, scrutinee, arms)
}
ExprKind::NeverToAny { source } => {
let source = this.hir.mirror(source);
Expand Down
Loading