diff options
| author | bors <bors@rust-lang.org> | 2018-02-09 02:26:43 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2018-02-09 02:26:43 +0000 |
| commit | afa8acce251cda7ab1548640fdb769139a45f839 (patch) | |
| tree | bc933826b368226427af8894bc4de82291c7ea9a | |
| parent | 932c736479f43dc8893a924946e4335d8e308c2e (diff) | |
| parent | b55cd8cc7c55a50941cc55cd399f6d5aebdf77f8 (diff) | |
Auto merge of #47489 - pnkfelix:limit-2pb-issue-46747, r=nikomatsakis
NLL: Limit two-phase borrows to autoref-introduced borrows This imposes a restriction on two-phase borrows so that it only applies to autoref-introduced borrows. The goal is to ensure that our initial deployment of two-phase borrows is very conservative. We want it to still cover the `v.push(v.len());` example, but we do not want it to cover cases like `let imm = &v; let mu = &mut v; mu.push(imm.len());` (Why do we want it to be conservative? Because when you are not conservative, then the results you get, at least with the current analysis, are tightly coupled to details of the MIR construction that we would rather remain invisible to the end user.) Fix #46747 I decided, for this PR, to add a debug-flag `-Z two-phase-beyond-autoref`, to re-enable the more general approach. But my intention here is *not* that we would eventually turn on that debugflag by default; the main reason I added it was that I thought it was useful for writing tests to be able to write source that looks like desugared MIR.
29 files changed, 564 insertions, 77 deletions
diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index f46b590d2dc..03a369577a3 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -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, @@ -37,6 +36,25 @@ impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, 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] fn hash_stable<W: StableHasherResult>(&self, diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 107779ec3fa..d1e431597e7 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -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 { diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index c69005101c6..7db75a51668 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -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); } diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index d82691f882c..c035d02a612 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -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, + } + } } /////////////////////////////////////////////////////////////////////////// @@ -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. diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index 53607764b39..5433c54fb94 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -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 diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 57ed41f2f06..afaf7d41e92 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -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, .. } | @@ -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 | diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 8b9a08dda87..5b8092e86da 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1121,6 +1121,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, diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index 96d69b4fba2..7579d95a8fe 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -120,9 +120,24 @@ 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), diff --git a/src/librustc_const_eval/pattern.rs b/src/librustc_const_eval/pattern.rs index e0b3929e32a..bdb1001124d 100644 --- a/src/librustc_const_eval/pattern.rs +++ b/src/librustc_const_eval/pattern.rs @@ -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 { @@ -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)), diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 56f863ab3aa..439533fae49 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -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); } diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 1ea897bf27c..34551e8e76f 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -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, @@ -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, @@ -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, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index d90209993aa..217dfdf8d41 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -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 @@ -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; } @@ -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 { @@ -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)) @@ -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( @@ -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; @@ -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 } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 80990bcc080..fe9b0b86bef 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -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 }; diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 317b038c482..00ab2e45995 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -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>; @@ -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(), }, }; @@ -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(), } } @@ -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(), }, }; @@ -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(), } } @@ -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, + } } } @@ -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, diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 42ffcc194ca..e6ebdd3d6c1 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -716,11 +716,14 @@ fn build_call_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }), span )); + let borrow_kind = BorrowKind::Mut { + allow_two_phase_borrow: false, + }; statements.push(Statement { source_info, kind: StatementKind::Assign( Place::Local(ref_rcvr), - Rvalue::Ref(tcx.types.re_erased, BorrowKind::Mut, rcvr_l) + Rvalue::Ref(tcx.types.re_erased, borrow_kind, rcvr_l) ) }); Operand::Move(Place::Local(ref_rcvr)) diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index ceea97e3ed3..15bbcad7325 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -426,7 +426,7 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> { debug!("Creating temp for return destination"); let dest = Rvalue::Ref( self.tcx.types.re_erased, - BorrowKind::Mut, + BorrowKind::Mut { allow_two_phase_borrow: false }, destination.0); let ty = dest.ty(caller_mir, self.tcx); @@ -511,7 +511,7 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> { callsite: &CallSite<'tcx>, caller_mir: &mut Mir<'tcx>) -> Local { let arg = Rvalue::Ref( self.tcx.types.re_erased, - BorrowKind::Mut, + BorrowKind::Mut { allow_two_phase_borrow: false }, arg.deref()); let ty = arg.ty(caller_mir, self.tcx); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 297e0e491f6..e7bb03eefc4 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -602,7 +602,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { } let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx); - if kind == BorrowKind::Mut { + if let BorrowKind::Mut { .. } = kind { // In theory, any zero-sized value could be borrowed // mutably without consequences. However, only &mut [] // is allowed right now, and only in functions. diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs index 65771068014..e2feb0ed390 100644 --- a/src/librustc_mir/util/elaborate_drops.rs +++ b/src/librustc_mir/util/elaborate_drops.rs @@ -531,7 +531,9 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> let result = BasicBlockData { statements: vec![self.assign( &Place::Local(ref_place), - Rvalue::Ref(tcx.types.re_erased, BorrowKind::Mut, self.place.clone()) + Rvalue::Ref(tcx.types.re_erased, + BorrowKind::Mut { allow_two_phase_borrow: false }, + self.place.clone()) )], terminator: Some(Terminator { kind: TerminatorKind::Call { @@ -591,7 +593,7 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> } else { (Rvalue::Ref( tcx.types.re_erased, - BorrowKind::Mut, + BorrowKind::Mut { allow_two_phase_borrow: false }, self.place.clone().index(cur)), Rvalue::BinaryOp(BinOp::Add, copy(&Place::Local(cur)), one)) }; @@ -735,7 +737,9 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> // cur = tmp as *mut T; // end = Offset(cur, len); drop_block_stmts.push(self.assign(&tmp, Rvalue::Ref( - tcx.types.re_erased, BorrowKind::Mut, self.place.clone() + tcx.types.re_erased, + BorrowKind::Mut { allow_two_phase_borrow: false }, + self.place.clone() ))); drop_block_stmts.push(self.assign(&cur, Rvalue::Cast( CastKind::Misc, Operand::Move(tmp.clone()), iter_ty diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index cd1975488a2..49b4ef0d385 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -870,7 +870,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> { } else { self.cx.tcx.data_layout.pointer_align }; - if bk == mir::BorrowKind::Mut { + if let mir::BorrowKind::Mut { .. } = bk { consts::addr_of_mut(self.cx, llval, align, "ref_mut") } else { consts::addr_of(self.cx, llval, align, "ref") diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 76df9be4838..3d61ffe3933 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -16,7 +16,7 @@ use hir::def::Def; use hir::def_id::{DefId, LOCAL_CRATE}; use rustc::{infer, traits}; use rustc::ty::{self, TyCtxt, TypeFoldable, Ty}; -use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow}; +use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability}; use syntax::abi; use syntax::symbol::Symbol; use syntax_pos::Span; @@ -176,8 +176,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mut autoref = None; if borrow { if let ty::TyRef(region, mt) = method.sig.inputs()[0].sty { + let mutbl = match mt.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // For initial two-phase borrow + // deployment, conservatively omit + // overloaded function call ops. + allow_two_phase_borrow: false, + } + }; autoref = Some(Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(region, mt.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target: method.sig.inputs()[0] }); } diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index d0280bf0b30..47e4b0272be 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -68,7 +68,7 @@ use rustc::infer::{Coercion, InferResult, InferOk}; use rustc::infer::type_variable::TypeVariableOrigin; use rustc::lint; use rustc::traits::{self, ObligationCause, ObligationCauseCode}; -use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow}; +use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability}; use rustc::ty::{self, TypeAndMut, Ty, ClosureSubsts}; use rustc::ty::fold::TypeFoldable; use rustc::ty::error::TypeError; @@ -421,8 +421,17 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { ty::TyRef(r_borrow, _) => r_borrow, _ => span_bug!(span, "expected a ref type, got {:?}", ty), }; + let mutbl = match mt_b.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // Deref-coercion is a case where we deliberately + // disallow two-phase borrows in its initial + // deployment; see discussion on PR #47489. + allow_two_phase_borrow: false, + } + }; adjustments.push(Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(r_borrow, mt_b.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(r_borrow, mutbl)), target: ty }); @@ -461,11 +470,17 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { let coercion = Coercion(self.cause.span); let r_borrow = self.next_region_var(coercion); + let mutbl = match mt_b.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + allow_two_phase_borrow: false, + } + }; Some((Adjustment { kind: Adjust::Deref(None), target: mt_a.ty }, Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(r_borrow, mt_b.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(r_borrow, mutbl)), target: self.tcx.mk_ref(r_borrow, ty::TypeAndMut { mutbl: mt_b.mutbl, ty: mt_a.ty @@ -871,6 +886,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ] => { match self.node_ty(expr.hir_id).sty { ty::TyRef(_, mt_orig) => { + let mutbl_adj: hir::Mutability = mutbl_adj.into(); // Reborrow that we can safely ignore, because // the next adjustment can only be a Deref // which will be merged into it. diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index 7f5b353f79e..20d58991496 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -17,7 +17,7 @@ use rustc::ty::subst::Substs; use rustc::traits; use rustc::ty::{self, Ty}; use rustc::ty::subst::Subst; -use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, OverloadedDeref}; +use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, OverloadedDeref}; use rustc::ty::fold::TypeFoldable; use rustc::infer::{self, InferOk}; use syntax_pos::Span; @@ -165,6 +165,14 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { mutbl, ty: target }); + let mutbl = match mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // Method call receivers are the primary use case + // for two-phase borrows. + allow_two_phase_borrow: true, + } + }; adjustments.push(Adjustment { kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target @@ -172,7 +180,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { if let Some(unsize_target) = pick.unsize { target = self.tcx.mk_ref(region, ty::TypeAndMut { - mutbl, + mutbl: mutbl.into(), ty: unsize_target }); adjustments.push(Adjustment { @@ -530,10 +538,19 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { for adjustment in &mut adjustments[..] { if let Adjust::Borrow(AutoBorrow::Ref(..)) = adjustment.kind { debug!("convert_place_op_to_mutable: converting autoref {:?}", adjustment); + let mutbl = match mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // For initial two-phase borrow + // deployment, conservatively omit + // overloaded operators. + allow_two_phase_borrow: false, + } + }; adjustment.kind = Adjust::Borrow(AutoBorrow::Ref(region, mutbl)); adjustment.target = self.tcx.mk_ref(region, ty::TypeAndMut { ty: source, - mutbl + mutbl: mutbl.into(), }); } source = adjustment.target; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f044b2c711e..5fd4e0cb109 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -96,7 +96,7 @@ use rustc::middle::region; use rustc::ty::subst::{Kind, Subst, Substs}; use rustc::traits::{self, FulfillmentContext, ObligationCause, ObligationCauseCode}; use rustc::ty::{self, Ty, TyCtxt, Visibility, ToPredicate}; -use rustc::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; +use rustc::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc::ty::fold::TypeFoldable; use rustc::ty::maps::Providers; use rustc::ty::util::{Representability, IntTypeExt}; @@ -2357,8 +2357,19 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let mut adjustments = autoderef.adjust_steps(needs); if let ty::TyRef(region, mt) = method.sig.inputs()[0].sty { + let mutbl = match mt.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // FIXME (#46747): arguably indexing is + // "just another kind of call"; perhaps it + // would be more consistent to allow + // two-phase borrows for .index() + // receivers here. + allow_two_phase_borrow: false, + } + }; adjustments.push(Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(region, mt.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target: self.tcx.mk_ref(region, ty::TypeAndMut { mutbl: mt.mutbl, ty: adjusted_ty @@ -3646,8 +3657,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expr.span, oprnd_t, needs) { let method = self.register_infer_ok_obligations(ok); if let ty::TyRef(region, mt) = method.sig.inputs()[0].sty { + let mutbl = match mt.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // (It shouldn't actually matter for unary ops whether + // we enable two-phase borrows or not, since a unary + // op has no additional operands.) + allow_two_phase_borrow: false, + } + }; self.apply_adjustments(oprnd, vec![Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(region, mt.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target: method.sig.inputs()[0] }]); } diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 0698e3ecb6e..a6776a0fe86 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -14,7 +14,7 @@ use super::{FnCtxt, Needs}; use super::method::MethodCallee; use rustc::ty::{self, Ty, TypeFoldable, TypeVariants}; use rustc::ty::TypeVariants::{TyStr, TyRef}; -use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow}; +use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability}; use rustc::infer::type_variable::TypeVariableOrigin; use errors; use syntax_pos::Span; @@ -198,8 +198,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let by_ref_binop = !op.node.is_by_value(); if is_assign == IsAssign::Yes || by_ref_binop { if let ty::TyRef(region, mt) = method.sig.inputs()[0].sty { + let mutbl = match mt.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // For initial two-phase borrow + // deployment, conservatively omit + // overloaded binary ops. + allow_two_phase_borrow: false, + } + }; let autoref = Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(region, mt.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target: method.sig.inputs()[0] }; self.apply_adjustments(lhs_expr, vec![autoref]); @@ -207,8 +216,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } if by_ref_binop { if let ty::TyRef(region, mt) = method.sig.inputs()[1].sty { + let mutbl = match mt.mutbl { + hir::MutImmutable => AutoBorrowMutability::Immutable, + hir::MutMutable => AutoBorrowMutability::Mutable { + // For initial two-phase borrow + // deployment, conservatively omit + // overloaded binary ops. + allow_two_phase_borrow: false, + } + }; let autoref = Adjustment { - kind: Adjust::Borrow(AutoBorrow::Ref(region, mt.mutbl)), + kind: Adjust::Borrow(AutoBorrow::Ref(region, mutbl)), target: method.sig.inputs()[1] }; // HACK(eddyb) Bypass checks due to reborrows being in diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 64063ec5bed..b5bf59fef9a 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -1063,7 +1063,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { match *autoref { adjustment::AutoBorrow::Ref(r, m) => { self.link_region(expr.span, r, - ty::BorrowKind::from_mutbl(m), expr_cmt); + ty::BorrowKind::from_mutbl(m.into()), expr_cmt); } adjustment::AutoBorrow::RawPtr(m) => { diff --git a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs index a9797e4d215..709c00ba846 100644 --- a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs +++ b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs @@ -8,9 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// revisions: lxl nll -//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows -//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll +// ignore-tidy-linelength + +// revisions: lxl_beyond nll_beyond nll_target + +//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref +//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll +//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll // This is an important corner case pointed out by Niko: one is // allowed to initiate a shared borrow during a reservation, but it @@ -18,6 +22,14 @@ // // FIXME: for clarity, diagnostics for these cases might be better off // if they specifically said "cannot activate mutable borrow of `x`" +// +// The convention for the listed revisions: "lxl" means lexical +// lifetimes (which can be easier to reason about). "nll" means +// non-lexical lifetimes. "nll_target" means the initial conservative +// two-phase borrows that only applies to autoref-introduced borrows. +// "nll_beyond" means the generalization of two-phase borrows to all +// `&mut`-borrows (doing so makes it easier to write code for specific +// corner cases). #![allow(dead_code)] @@ -27,6 +39,7 @@ fn ok() { let mut x = 3; let y = &mut x; { let z = &x; read(z); } + //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable *y += 1; } @@ -34,9 +47,11 @@ fn not_ok() { let mut x = 3; let y = &mut x; let z = &x; + //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable *y += 1; - //[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable - //[nll]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + //[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + //[nll_beyond]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + //[nll_target]~^^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable read(z); } @@ -44,18 +59,21 @@ fn should_be_ok_with_nll() { let mut x = 3; let y = &mut x; let z = &x; + //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable read(z); *y += 1; - //[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable - // (okay with nll today) + //[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + // (okay with (generalized) nll today) } fn should_also_eventually_be_ok_with_nll() { let mut x = 3; let y = &mut x; let _z = &x; + //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable *y += 1; - //[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + //[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable + // (okay with (generalized) nll today) } fn main() { } diff --git a/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs b/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs index 7695bd3e465..dd174981fb1 100644 --- a/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs +++ b/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs @@ -8,9 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// revisions: lxl nll -//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows -//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll +// ignore-tidy-linelength + +// revisions: lxl_beyond nll_beyond nll_target +//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref +//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref -Z nll +//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll // This is the second counter-example from Niko's blog post // smallcultfollowing.com/babysteps/blog/2017/03/01/nested-method-calls-via-two-phase-borrowing/ @@ -18,6 +21,14 @@ // It is "artificial". It is meant to illustrate directly that we // should allow an aliasing access during reservation, but *not* while // the mutable borrow is active. +// +// The convention for the listed revisions: "lxl" means lexical +// lifetimes (which can be easier to reason about). "nll" means +// non-lexical lifetimes. "nll_target" means the initial conservative +// two-phase borrows that only applies to autoref-introduced borrows. +// "nll_beyond" means the generalization of two-phase borrows to all +// `&mut`-borrows (doing so makes it easier to write code for specific +// corner cases). fn main() { /*0*/ let mut i = 0; @@ -25,11 +36,13 @@ fn main() { /*1*/ let p = &mut i; // (reservation of `i` starts here) /*2*/ let j = i; // OK: `i` is only reserved here + //[nll_target]~^ ERROR cannot use `i` because it was mutably borrowed [E0503] /*3*/ *p += 1; // (mutable borrow of `i` starts here, since `p` is used) - /*4*/ let k = i; //[lxl]~ ERROR cannot use `i` because it was mutably borrowed [E0503] - //[nll]~^ ERROR cannot use `i` because it was mutably borrowed [E0503] + /*4*/ let k = i; //[lxl_beyond]~ ERROR cannot use `i` because it was mutably borrowed [E0503] + //[nll_beyond]~^ ERROR cannot use `i` because it was mutably borrowed [E0503] + //[nll_target]~^^ ERROR cannot use `i` because it was mutably borrowed [E0503] /*5*/ *p += 1; diff --git a/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs b/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs new file mode 100644 index 00000000000..795d45a776d --- /dev/null +++ b/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs @@ -0,0 +1,260 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// revisions: lxl nll g2p +//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows +//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll +//[g2p]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll -Z two-phase-beyond-autoref + +// This is a test checking that when we limit two-phase borrows to +// method receivers, we do not let other kinds of auto-ref to leak +// through. +// +// The g2p revision illustrates the "undesirable" behavior you would +// otherwise observe without limiting the phasing to autoref on method +// receivers (namely, in many cases demonstrated below, the error +// would not arise). + +// (If we revise the compiler or this test so that the g2p revision +// passes, turn the `rustc_attrs` feature back on and tag the `fn +// main` with `#[rustc_error]` so that this remains a valid +// compile-fail test.) +// +// #![feature(rustc_attrs)] + +use std::ops::{Index, IndexMut}; +use std::ops::{AddAssign, SubAssign, MulAssign, DivAssign, RemAssign}; +use std::ops::{BitAndAssign, BitOrAssign, BitXorAssign, ShlAssign, ShrAssign}; + +// This is case outlined by Niko that we want to ensure we reject +// (at least initially). + +fn foo(x: &mut u32, y: u32) { + *x += y; +} + +fn deref_coercion(x: &mut u32) { + foo(x, *x); + //[lxl]~^ ERROR cannot use `*x` because it was mutably borrowed [E0503] + //[nll]~^^ ERROR cannot use `*x` because it was mutably borrowed [E0503] +} + +// While adding a flag to adjustments (indicating whether they +// should support two-phase borrows, here are the cases I +// encountered: +// +// - [x] Resolving overloaded_call_traits (call, call_mut, call_once) +// - [x] deref_coercion (shown above) +// - [x] coerce_unsized e.g. `&[T; n]`, `&mut [T; n] -> &[T]`, +// `&mut [T; n] -> &mut [T]`, `&Concrete -> &Trait` +// - [x] Method Call Receivers (the case we want to support!) +// - [x] ExprIndex and ExprUnary Deref; only need to handle coerce_index_op +// - [x] overloaded_binops + +fn overloaded_call_traits() { + // Regarding overloaded call traits, note that there is no + // scenario where adding two-phase borrows should "fix" these + // cases, because either we will resolve both invocations to + // `call_mut` (in which case the inner call requires a mutable + // borrow which will conflict with the outer reservation), or we + // will resolve both to `call` (which will just work, regardless + // of two-phase borrow support), or we will resolve both to + // `call_once` (in which case the inner call requires moving the + // receiver, invalidating the outer call). + + fn twice_ten_sm<F: FnMut(i32) -> i32>(f: &mut F) { + f(f(10)); + //[lxl]~^ ERROR cannot borrow `*f` as mutable more than once at a time + //[lxl]~| ERROR cannot borrow `*f` as mutable more than once at a time + //[nll]~^^^ ERROR cannot borrow `*f` as mutable more than once at a time + //[nll]~| ERROR cannot borrow `*f` as mutable more than once at a time + //[g2p]~^^^^^ ERROR cannot borrow `*f` as mutable more than once at a time + } + fn twice_ten_si<F: Fn(i32) -> i32>(f: &mut F) { + f(f(10)); + } + fn twice_ten_so<F: FnOnce(i32) -> i32>(f: Box<F>) { + f(f(10)); + //[lxl]~^ ERROR use of moved value: `*f` + //[nll]~^^ ERROR use of moved value: `*f` + //[g2p]~^^^ ERROR use of moved value: `*f` + } + + fn twice_ten_om(f: &mut FnMut(i32) -> i32) { + f(f(10)); + //[lxl]~^ ERROR cannot borrow `*f` as mutable more than once at a time + //[lxl]~| ERROR cannot borrow `*f` as mutable more than once at a time + //[nll]~^^^ ERROR cannot borrow `*f` as mutable more than once at a time + //[nll]~| ERROR cannot borrow `*f` as mutable more than once at a time + //[g2p]~^^^^^ ERROR cannot borrow `*f` as mutable more than once at a time + } + fn twice_ten_oi(f: &mut Fn(i32) -> i32) { + f(f(10)); + } + fn twice_ten_oo(f: Box<FnOnce(i32) -> i32>) { + f(f(10)); + //[lxl]~^ ERROR cannot move a value of type + //[lxl]~^^ ERROR cannot move a value of type + //[lxl]~^^^ ERROR use of moved value: `*f` + //[nll]~^^^^ ERROR cannot move a value of type + //[nll]~^^^^^ ERROR cannot move a value of type + //[nll]~^^^^^^ ERROR cannot move a value of type + //[nll]~^^^^^^^ ERROR cannot move a value of type + //[nll]~^^^^^^^^ ERROR use of moved value: `*f` + //[g2p]~^^^^^^^^^ ERROR cannot move a value of type + //[g2p]~^^^^^^^^^^ ERROR cannot move a value of type + //[g2p]~^^^^^^^^^^^ ERROR cannot move a value of type + //[g2p]~^^^^^^^^^^^^ ERROR cannot move a value of type + //[g2p]~^^^^^^^^^^^^^ ERROR use of moved value: `*f` + } + + twice_ten_sm(&mut |x| x + 1); + twice_ten_si(&mut |x| x + 1); + twice_ten_so(Box::new(|x| x + 1)); + twice_ten_om(&mut |x| x + 1); + twice_ten_oi(&mut |x| x + 1); + twice_ten_oo(Box::new(|x| x + 1)); +} + +trait TwoMethods { + fn m(&mut self, x: i32) -> i32 { x + 1 } + fn i(&self, x: i32) -> i32 { x + 1 } +} + +struct T; + +impl TwoMethods for T { } + +struct S; + +impl S { + fn m(&mut self, x: i32) -> i32 { x + 1 } + fn i(&self, x: i32) -> i32 { x + 1 } +} + +impl TwoMethods for [i32; 3] { } + +fn double_access<X: Copy>(m: &mut [X], s: &[X]) { + m[0] = s[1]; +} + +fn coerce_unsized() { + let mut a = [1, 2, 3]; + + // This is not okay. + double_access(&mut a, &a); + //[lxl]~^ ERROR cannot borrow `a` as immutable because it is also borrowed as mutable [E0502] + //[nll]~^^ ERROR cannot borrow `a` as immutable because it is also borrowed as mutable [E0502] + //[g2p]~^^^ ERROR cannot borrow `a` as immutable because it is also borrowed as mutable [E0502] + + // But this is okay. + a.m(a.i(10)); +} + +struct I(i32); + +impl Index<i32> for I { + type Output = i32; + fn index(&self, _: i32) -> &i32 { + &self.0 + } +} + +impl IndexMut<i32> for I { + fn index_mut(&mut self, _: i32) -> &mut i32 { + &mut self.0 + } +} + +fn coerce_index_op() { + let mut i = I(10); + i[i[3]] = 4; + //[lxl]~^ ERROR cannot borrow `i` as immutable because it is also borrowed as mutable [E0502] + //[nll]~^^ ERROR cannot borrow `i` as immutable because it is also borrowed as mutable [E0502] + + i[3] = i[4]; + + i[i[3]] = i[4]; + //[lxl]~^ ERROR cannot borrow `i` as immutable because it is also borrowed as mutable [E0502] + //[nll]~^^ ERROR cannot borrow `i` as immutable because it is also borrowed as mutable [E0502] +} + +struct A(i32); + +macro_rules! trivial_binop { + ($Trait:ident, $m:ident) => { + impl $Trait<i32> for A { fn $m(&mut self, rhs: i32) { self.0 = rhs; } } + } +} + +trivial_binop!(AddAssign, add_assign); +trivial_binop!(SubAssign, sub_assign); +trivial_binop!(MulAssign, mul_assign); +trivial_binop!(DivAssign, div_assign); +trivial_binop!(RemAssign, rem_assign); +trivial_binop!(BitAndAssign, bitand_assign); +trivial_binop!(BitOrAssign, bitor_assign); +trivial_binop!(BitXorAssign, bitxor_assign); +trivial_binop!(ShlAssign, shl_assign); +trivial_binop!(ShrAssign, shr_assign); + +fn overloaded_binops() { + let mut a = A(10); + a += a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a -= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a *= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a /= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a &= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a |= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a ^= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a <<= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed + a >>= a.0; + //[lxl]~^ ERROR cannot use `a.0` because it was mutably borrowed + //[nll]~^^ ERROR cannot use `a.0` because it was mutably borrowed +} + +fn main() { + + // As a reminder, this is the basic case we want to ensure we handle. + let mut v = vec![1, 2, 3]; + v.push(v.len()); + + // (as a rule, pnkfelix does not like to write tests with dead code.) + + deref_coercion(&mut 5); + overloaded_call_traits(); + + + let mut s = S; + s.m(s.i(10)); + + let mut t = T; + t.m(t.i(10)); + + coerce_unsized(); + coerce_index_op(); + overloaded_binops(); +} diff --git a/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs b/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs index cc85315263a..b5fda4985f2 100644 --- a/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs +++ b/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs @@ -8,9 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// revisions: lxl nll -//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows -//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll +// ignore-tidy-linelength + +// revisions: lxl_beyond nll_beyond nll_target + +//[lxl_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref +//[nll_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll +//[nll_target]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll // This is a corner case that the current implementation is (probably) // treating more conservatively than is necessary. But it also does @@ -19,6 +23,18 @@ // So this test is just making a note of the current behavior, with // the caveat that in the future, the rules may be loosened, at which // point this test might be thrown out. +// +// The convention for the listed revisions: "lxl" means lexical +// lifetimes (which can be easier to reason about). "nll" means +// non-lexical lifetimes. "nll_target" means the initial conservative +// two-phase borrows that only applies to autoref-introduced borrows. +// "nll_beyond" means the generalization of two-phase borrows to all +// `&mut`-borrows (doing so makes it easier to write code for specific +// corner cases). +// +// FIXME: in "nll_target", we currently see the same error reported +// twice. This is injected by `-Z two-phase-borrows`; not sure why as +// of yet. fn main() { let mut vec = vec![0, 1]; @@ -30,8 +46,10 @@ fn main() { // with the shared borrow. But in the current implementation, // its an error. delay = &mut vec; - //[lxl]~^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable - //[nll]~^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable + //[lxl_beyond]~^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable + //[nll_beyond]~^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable + //[nll_target]~^^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable + //[nll_target]~| ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable shared[0]; } |
