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: Limit two-phase borrows to autoref-introduced borrows #47489

Merged
merged 5 commits into from
Feb 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use std::mem;
impl_stable_hash_for!(struct mir::GeneratorLayout<'tcx> { fields });
impl_stable_hash_for!(struct mir::SourceInfo { span, scope });
impl_stable_hash_for!(enum mir::Mutability { Mut, Not });
impl_stable_hash_for!(enum mir::BorrowKind { Shared, Unique, Mut });
impl_stable_hash_for!(enum mir::LocalKind { Var, Temp, Arg, ReturnPointer });
impl_stable_hash_for!(struct mir::LocalDecl<'tcx> {
mutability,
Expand All @@ -36,6 +35,25 @@ impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator,
impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, kind });
impl_stable_hash_for!(struct mir::UnsafetyCheckResult { violations, unsafe_blocks });

impl<'gcx> HashStable<StableHashingContext<'gcx>>
for mir::BorrowKind {
#[inline]
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'gcx>,
hasher: &mut StableHasher<W>) {
mem::discriminant(self).hash_stable(hcx, hasher);

match *self {
mir::BorrowKind::Shared |
mir::BorrowKind::Unique => {}
mir::BorrowKind::Mut { allow_two_phase_borrow } => {
allow_two_phase_borrow.hash_stable(hcx, hasher);
}
}
}
}


impl<'gcx> HashStable<StableHashingContext<'gcx>>
for mir::UnsafetyViolationKind {
#[inline]
Expand Down
14 changes: 14 additions & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,20 @@ impl_stable_hash_for!(struct ty::adjustment::Adjustment<'tcx> { kind, target });
impl_stable_hash_for!(struct ty::adjustment::OverloadedDeref<'tcx> { region, mutbl });
impl_stable_hash_for!(struct ty::UpvarBorrow<'tcx> { kind, region });

impl<'gcx> HashStable<StableHashingContext<'gcx>> for ty::adjustment::AutoBorrowMutability {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'gcx>,
hasher: &mut StableHasher<W>) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
ty::adjustment::AutoBorrowMutability::Mutable { ref allow_two_phase_borrow } => {
allow_two_phase_borrow.hash_stable(hcx, hasher);
}
ty::adjustment::AutoBorrowMutability::Immutable => {}
}
}
}

impl_stable_hash_for!(struct ty::UpvarId { var_id, closure_expr_id });

impl_stable_hash_for!(enum ty::BorrowKind {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
expr.span,
cmt_base,
r,
ty::BorrowKind::from_mutbl(m),
ty::BorrowKind::from_mutbl(m.into()),
AutoRef);
}

Expand Down
17 changes: 15 additions & 2 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,20 @@ pub enum BorrowKind {
Unique,

/// Data is mutable and not aliasable.
Mut,
Mut {
/// True if this borrow arose from method-call auto-ref
/// (i.e. `adjustment::Adjust::Borrow`)
allow_two_phase_borrow: bool
}
}

impl BorrowKind {
pub fn allows_two_phase_borrow(&self) -> bool {
match *self {
BorrowKind::Shared | BorrowKind::Unique => false,
BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
}
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1611,7 +1624,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
Ref(region, borrow_kind, ref place) => {
let kind_str = match borrow_kind {
BorrowKind::Shared => "",
BorrowKind::Mut | BorrowKind::Unique => "mut ",
BorrowKind::Mut { .. } | BorrowKind::Unique => "mut ",
};

// When printing regions, add trailing space if necessary.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl<'tcx> BinOp {
impl BorrowKind {
pub fn to_mutbl_lossy(self) -> hir::Mutability {
match self {
BorrowKind::Mut => hir::MutMutable,
BorrowKind::Mut { .. } => hir::MutMutable,
BorrowKind::Shared => hir::MutImmutable,

// We have no type corresponding to a unique imm borrow, so
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,9 +951,10 @@ impl<'tcx> PlaceContext<'tcx> {
pub fn is_mutating_use(&self) -> bool {
match *self {
PlaceContext::Store | PlaceContext::AsmOutput | PlaceContext::Call |
PlaceContext::Borrow { kind: BorrowKind::Mut, .. } |
PlaceContext::Borrow { kind: BorrowKind::Mut { .. }, .. } |
PlaceContext::Projection(Mutability::Mut) |
PlaceContext::Drop => true,

PlaceContext::Inspect |
PlaceContext::Borrow { kind: BorrowKind::Shared, .. } |
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
Expand All @@ -971,7 +972,8 @@ impl<'tcx> PlaceContext<'tcx> {
PlaceContext::Borrow { kind: BorrowKind::Unique, .. } |
PlaceContext::Projection(Mutability::Not) |
PlaceContext::Copy | PlaceContext::Move => true,
PlaceContext::Borrow { kind: BorrowKind::Mut, .. } | PlaceContext::Store |

PlaceContext::Borrow { kind: BorrowKind::Mut { .. }, .. } | PlaceContext::Store |
PlaceContext::AsmOutput |
PlaceContext::Call | PlaceContext::Projection(Mutability::Mut) |
PlaceContext::Drop | PlaceContext::StorageLive | PlaceContext::StorageDead |
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"select which borrowck is used (`ast`, `mir`, or `compare`)"),
two_phase_borrows: bool = (false, parse_bool, [UNTRACKED],
"use two-phase reserved/active distinction for `&mut` borrows in MIR borrowck"),
two_phase_beyond_autoref: bool = (false, parse_bool, [UNTRACKED],
"when using two-phase-borrows, allow two phases even for non-autoref `&mut` borrows"),
time_passes: bool = (false, parse_bool, [UNTRACKED],
"measure time of each rustc pass"),
count_llvm_insns: bool = (false, parse_bool,
Expand Down
17 changes: 16 additions & 1 deletion src/librustc/ty/adjustment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,25 @@ impl<'a, 'gcx, 'tcx> OverloadedDeref<'tcx> {
}
}

#[derive(Copy, Clone, PartialEq, Debug, RustcEncodable, RustcDecodable)]
pub enum AutoBorrowMutability {
Mutable { allow_two_phase_borrow: bool },
Immutable,
}

impl From<AutoBorrowMutability> for hir::Mutability {
fn from(m: AutoBorrowMutability) -> Self {
match m {
AutoBorrowMutability::Mutable { .. } => hir::MutMutable,
AutoBorrowMutability::Immutable => hir::MutImmutable,
}
}
}

#[derive(Copy, Clone, PartialEq, Debug, RustcEncodable, RustcDecodable)]
pub enum AutoBorrow<'tcx> {
/// Convert from T to &T.
Ref(ty::Region<'tcx>, hir::Mutability),
Ref(ty::Region<'tcx>, AutoBorrowMutability),

/// Convert from T to *T.
RawPtr(hir::Mutability),
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_const_eval/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl<'tcx> fmt::Display for Pattern<'tcx> {
BindingMode::ByValue => mutability == Mutability::Mut,
BindingMode::ByRef(_, bk) => {
write!(f, "ref ")?;
bk == BorrowKind::Mut
match bk { BorrowKind::Mut { .. } => true, _ => false }
}
};
if is_mut {
Expand Down Expand Up @@ -429,7 +429,7 @@ impl<'a, 'tcx> PatternContext<'a, 'tcx> {
(Mutability::Not, BindingMode::ByValue),
ty::BindByReference(hir::MutMutable) =>
(Mutability::Not, BindingMode::ByRef(
region.unwrap(), BorrowKind::Mut)),
region.unwrap(), BorrowKind::Mut { allow_two_phase_borrow: false })),
ty::BindByReference(hir::MutImmutable) =>
(Mutability::Not, BindingMode::ByRef(
region.unwrap(), BorrowKind::Shared)),
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAllocation {
for adj in cx.tables.expr_adjustments(e) {
if let adjustment::Adjust::Borrow(adjustment::AutoBorrow::Ref(_, m)) = adj.kind {
let msg = match m {
hir::MutImmutable => "unnecessary allocation, use & instead",
hir::MutMutable => "unnecessary allocation, use &mut instead"
adjustment::AutoBorrowMutability::Immutable =>
"unnecessary allocation, use & instead",
adjustment::AutoBorrowMutability::Mutable { .. }=>
"unnecessary allocation, use &mut instead"
};
cx.span_lint(UNUSED_ALLOCATION, e.span, msg);
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
"immutable",
"mutable",
) {
(BorrowKind::Shared, lft, _, BorrowKind::Mut, _, rgt) |
(BorrowKind::Mut, _, lft, BorrowKind::Shared, rgt, _) => self.tcx
(BorrowKind::Shared, lft, _, BorrowKind::Mut { .. }, _, rgt) |
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Shared, rgt, _) => self.tcx
.cannot_reborrow_already_borrowed(
span,
&desc_place,
Expand All @@ -271,7 +271,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Origin::Mir,
),

(BorrowKind::Mut, _, _, BorrowKind::Mut, _, _) => self.tcx
(BorrowKind::Mut { .. }, _, _, BorrowKind::Mut { .. }, _, _) => self.tcx
.cannot_mutably_borrow_multiply(
span,
&desc_place,
Expand Down Expand Up @@ -314,7 +314,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Origin::Mir,
),

(BorrowKind::Mut, _, lft, BorrowKind::Unique, _, _) => self.tcx
(BorrowKind::Mut { .. }, _, lft, BorrowKind::Unique, _, _) => self.tcx
.cannot_reborrow_already_uniquely_borrowed(
span,
&desc_place,
Expand Down
27 changes: 18 additions & 9 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,15 @@ impl InitializationRequiringAction {
}

impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
/// Returns true if the borrow represented by `kind` is
/// allowed to be split into separate Reservation and
/// Activation phases.
fn allow_two_phase_borrow(&self, kind: BorrowKind) -> bool {
self.tcx.sess.two_phase_borrows() &&
(kind.allows_two_phase_borrow() ||
self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref)
}

/// Checks an access to the given place to see if it is allowed. Examines the set of borrows
/// that are in scope, as well as which paths have been initialized, to ensure that (a) the
/// place is initialized and (b) it is not borrowed in some way that would prevent this
Expand Down Expand Up @@ -797,9 +806,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Control::Continue
}

(Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut) => {
(Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut { .. }) => {
// Reading from mere reservations of mutable-borrows is OK.
if this.tcx.sess.two_phase_borrows() && index.is_reservation()
if this.allow_two_phase_borrow(borrow.kind) && index.is_reservation()
{
return Control::Continue;
}
Expand Down Expand Up @@ -828,7 +837,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
}

(Reservation(kind), BorrowKind::Unique)
| (Reservation(kind), BorrowKind::Mut)
| (Reservation(kind), BorrowKind::Mut { .. })
| (Activation(kind, _), _)
| (Write(kind), _) => {
match rw {
Expand Down Expand Up @@ -945,9 +954,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Rvalue::Ref(_ /*rgn*/, bk, ref place) => {
let access_kind = match bk {
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
BorrowKind::Unique | BorrowKind::Mut => {
BorrowKind::Unique | BorrowKind::Mut { .. } => {
let wk = WriteKind::MutableBorrow(bk);
if self.tcx.sess.two_phase_borrows() {
if self.allow_two_phase_borrow(bk) {
(Deep, Reservation(wk))
} else {
(Deep, Write(wk))
Expand Down Expand Up @@ -1196,7 +1205,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// mutable borrow before we check it.
match borrow.kind {
BorrowKind::Shared => return,
BorrowKind::Unique | BorrowKind::Mut => {}
BorrowKind::Unique | BorrowKind::Mut { .. } => {}
}

self.access_place(
Expand Down Expand Up @@ -1467,8 +1476,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
span_bug!(span, "&unique borrow for {:?} should not fail", place);
}
}
Reservation(WriteKind::MutableBorrow(BorrowKind::Mut))
| Write(WriteKind::MutableBorrow(BorrowKind::Mut)) => if let Err(place_err) =
Reservation(WriteKind::MutableBorrow(BorrowKind::Mut { .. }))
| Write(WriteKind::MutableBorrow(BorrowKind::Mut { .. })) => if let Err(place_err) =
self.is_mutable(place, is_local_mutation_allowed)
{
error_reported = true;
Expand Down Expand Up @@ -1532,7 +1541,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
Activation(..) => {} // permission checks are done at Reservation point.

Read(ReadKind::Borrow(BorrowKind::Unique))
| Read(ReadKind::Borrow(BorrowKind::Mut))
| Read(ReadKind::Borrow(BorrowKind::Mut { .. }))
| Read(ReadKind::Borrow(BorrowKind::Shared))
| Read(ReadKind::Copy) => {} // Access authorized
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
let kind = match self.kind {
mir::BorrowKind::Shared => "",
mir::BorrowKind::Unique => "uniq ",
mir::BorrowKind::Mut => "mut ",
mir::BorrowKind::Mut { .. } => "mut ",
};
let region = format!("{}", self.region);
let region = if region.len() > 0 { format!("{} ", region) } else { region };
Expand Down
36 changes: 26 additions & 10 deletions src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ use hair::cx::to_ref::ToRef;
use rustc::hir::def::{Def, CtorKind};
use rustc::middle::const_val::ConstVal;
use rustc::ty::{self, AdtKind, VariantDef, Ty};
use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow};
use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability};
use rustc::ty::cast::CastKind as TyCastKind;
use rustc::hir;
use rustc::hir::def_id::LocalDefId;
use rustc::mir::{BorrowKind};

impl<'tcx> Mirror<'tcx> for &'tcx hir::Expr {
type Output = Expr<'tcx>;
Expand Down Expand Up @@ -111,7 +112,7 @@ fn apply_adjustment<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
span,
kind: ExprKind::Borrow {
region: deref.region,
borrow_kind: to_borrow_kind(deref.mutbl),
borrow_kind: deref.mutbl.to_borrow_kind(),
arg: expr.to_ref(),
},
};
Expand All @@ -121,7 +122,7 @@ fn apply_adjustment<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
Adjust::Borrow(AutoBorrow::Ref(r, m)) => {
ExprKind::Borrow {
region: r,
borrow_kind: to_borrow_kind(m),
borrow_kind: m.to_borrow_kind(),
arg: expr.to_ref(),
}
}
Expand All @@ -141,7 +142,7 @@ fn apply_adjustment<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
span,
kind: ExprKind::Borrow {
region,
borrow_kind: to_borrow_kind(m),
borrow_kind: m.to_borrow_kind(),
arg: expr.to_ref(),
},
};
Expand Down Expand Up @@ -287,7 +288,7 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
};
ExprKind::Borrow {
region,
borrow_kind: to_borrow_kind(mutbl),
borrow_kind: mutbl.to_borrow_kind(),
arg: expr.to_ref(),
}
}
Expand Down Expand Up @@ -642,10 +643,25 @@ fn method_callee<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
}
}

fn to_borrow_kind(m: hir::Mutability) -> BorrowKind {
match m {
hir::MutMutable => BorrowKind::Mut,
hir::MutImmutable => BorrowKind::Shared,
trait ToBorrowKind { fn to_borrow_kind(&self) -> BorrowKind; }

impl ToBorrowKind for AutoBorrowMutability {
fn to_borrow_kind(&self) -> BorrowKind {
match *self {
AutoBorrowMutability::Mutable { allow_two_phase_borrow } =>
BorrowKind::Mut { allow_two_phase_borrow },
AutoBorrowMutability::Immutable =>
BorrowKind::Shared,
}
}
}

impl ToBorrowKind for hir::Mutability {
fn to_borrow_kind(&self) -> BorrowKind {
match *self {
hir::MutMutable => BorrowKind::Mut { allow_two_phase_borrow: false },
hir::MutImmutable => BorrowKind::Shared,
}
}
}

Expand Down Expand Up @@ -947,7 +963,7 @@ fn capture_freevar<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>,
let borrow_kind = match upvar_borrow.kind {
ty::BorrowKind::ImmBorrow => BorrowKind::Shared,
ty::BorrowKind::UniqueImmBorrow => BorrowKind::Unique,
ty::BorrowKind::MutBorrow => BorrowKind::Mut,
ty::BorrowKind::MutBorrow => BorrowKind::Mut { allow_two_phase_borrow: false }
};
Expr {
temp_lifetime,
Expand Down
Loading