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

[NLL] Be more permissive when checking access due to Match #53438

Merged
merged 9 commits into from
Sep 25, 2018
3 changes: 2 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ for mir::BorrowKind {

match *self {
mir::BorrowKind::Shared |
mir::BorrowKind::Shallow |
mir::BorrowKind::Unique => {}
mir::BorrowKind::Mut { allow_two_phase_borrow } => {
allow_two_phase_borrow.hash_stable(hcx, hasher);
Expand Down Expand Up @@ -272,7 +273,7 @@ for mir::StatementKind<'gcx> {
}
}

impl_stable_hash_for!(enum mir::FakeReadCause { ForMatch, ForLet });
impl_stable_hash_for!(enum mir::FakeReadCause { ForMatchGuard, ForMatchedPlace, ForLet });

impl<'a, 'gcx, T> HashStable<StableHashingContext<'a>>
for mir::ValidationOperand<'gcx, T>
Expand Down
40 changes: 33 additions & 7 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,32 @@ impl From<Mutability> for hir::Mutability {
}
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, RustcEncodable, RustcDecodable)]
pub enum BorrowKind {
/// Data must be immutable and is aliasable.
Shared,

/// The immediately borrowed place must be immutable, but projections from
/// it don't need to be. For example, a shallow borrow of `a.b` doesn't
/// conflict with a mutable borrow of `a.b.c`.
///
/// This is used when lowering matches: when matching on a place we want to
/// ensure that place have the same value from the start of the match until
/// an arm is selected. This prevents this code from compiling:
///
/// let mut x = &Some(0);
/// match *x {
/// None => (),
/// Some(_) if { x = &None; false } => (),
/// Some(_) => (),
/// }
///
/// This can't be a shared borrow because mutably borrowing (*x as Some).0
/// should not prevent `if let None = x { ... }`, for example, becase the
/// mutating `(*x as Some).0` can't affect the discriminant of `x`.
/// We can also report errors with this kind of borrow differently.
Shallow,

/// Data must be immutable but not aliasable. This kind of borrow
/// cannot currently be expressed by the user and is used only in
/// implicit closure bindings. It is needed when the closure is
Expand Down Expand Up @@ -504,7 +525,7 @@ pub enum BorrowKind {
impl BorrowKind {
pub fn allows_two_phase_borrow(&self) -> bool {
match *self {
BorrowKind::Shared | BorrowKind::Unique => false,
BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => false,
BorrowKind::Mut {
allow_two_phase_borrow,
} => allow_two_phase_borrow,
Expand Down Expand Up @@ -1672,7 +1693,11 @@ pub enum FakeReadCause {
///
/// This should ensure that you cannot change the variant for an enum
/// while you are in the midst of matching on it.
ForMatch,
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,

/// Officially, the semantics of
///
Expand Down Expand Up @@ -1773,7 +1798,7 @@ impl<'tcx> Debug for Statement<'tcx> {

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

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

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

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ impl BorrowKind {
// use `&mut`. It gives all the capabilities of an `&uniq`
// and hence is a safe "over approximation".
BorrowKind::Unique => hir::MutMutable,

// We have no type corresponding to a shallow borrow, so use
// `&` as an approximation.
BorrowKind::Shallow => hir::MutImmutable,
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,6 +963,7 @@ impl<'tcx> PlaceContext<'tcx> {

PlaceContext::Inspect |
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
PlaceContext::Projection(Mutability::Not) |
PlaceContext::Copy | PlaceContext::Move |
Expand All @@ -974,7 +975,9 @@ impl<'tcx> PlaceContext<'tcx> {
/// Returns true if this place context represents a use that does not change the value.
pub fn is_nonmutating_use(&self) -> bool {
match *self {
PlaceContext::Inspect | PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Inspect |
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Borrow { kind: BorrowKind::Shallow, .. } |
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
PlaceContext::Projection(Mutability::Not) |
PlaceContext::Copy | PlaceContext::Move => true,
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_mir/borrow_check/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
let kind = match self.kind {
mir::BorrowKind::Shared => "",
mir::BorrowKind::Shallow => "shallow ",
mir::BorrowKind::Unique => "uniq ",
mir::BorrowKind::Mut { .. } => "mut ",
};
Expand Down Expand Up @@ -287,7 +288,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
| PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
TwoPhaseActivation::NotActivated
}
_ => {
Expand Down
54 changes: 47 additions & 7 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Origin::Mir,
),

(BorrowKind::Mut { .. }, _, _, BorrowKind::Shallow, _, _)
| (BorrowKind::Unique, _, _, BorrowKind::Shallow, _, _) => {
let mut err = tcx.cannot_mutate_in_match_guard(
span,
issued_span,
&desc_place,
"mutably borrow",
Origin::Mir,
);
borrow_spans.var_span_label(
&mut err,
format!(
"borrow occurs due to use of `{}` in closure",
desc_place
),
);
err.buffer(&mut self.errors_buffer);

return;
pnkfelix marked this conversation as resolved.
Show resolved Hide resolved
}

(BorrowKind::Unique, _, _, _, _, _) => tcx.cannot_uniquely_borrow_by_one_closure(
span,
&desc_place,
Expand Down Expand Up @@ -368,7 +389,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Origin::Mir,
),

(BorrowKind::Shared, _, _, BorrowKind::Shared, _, _) => unreachable!(),
(BorrowKind::Shallow, _, _, BorrowKind::Unique, _, _)
| (BorrowKind::Shallow, _, _, BorrowKind::Mut { .. }, _, _) => {
// Shallow borrows are uses from the user's point of view.
self.report_use_while_mutably_borrowed(context, (place, span), issued_borrow);
return
}
(BorrowKind::Shared, _, _, BorrowKind::Shared, _, _)
| (BorrowKind::Shared, _, _, BorrowKind::Shallow, _, _)
| (BorrowKind::Shallow, _, _, BorrowKind::Shared, _, _)
| (BorrowKind::Shallow, _, _, BorrowKind::Shallow, _, _) => unreachable!(),
};

if issued_spans == borrow_spans {
Expand Down Expand Up @@ -780,12 +810,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let loan_span = loan_spans.args_or_use();

let tcx = self.infcx.tcx;
let mut err = tcx.cannot_assign_to_borrowed(
span,
loan_span,
&self.describe_place(place).unwrap_or("_".to_owned()),
Origin::Mir,
);
let mut err = if loan.kind == BorrowKind::Shallow {
tcx.cannot_mutate_in_match_guard(
span,
loan_span,
&self.describe_place(place).unwrap_or("_".to_owned()),
"assign",
Origin::Mir,
)
} else {
tcx.cannot_assign_to_borrowed(
span,
loan_span,
&self.describe_place(place).unwrap_or("_".to_owned()),
Origin::Mir,
)
};

loan_spans.var_span_label(&mut err, "borrow occurs due to use in closure");

Expand Down
Loading