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.

Author: bors

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,
@@ -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]

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 {

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);
             }
 

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.

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

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 |

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,

src/librustc/ty/adjustment.rs

@@ -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),

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)),

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);
             }

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,

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
         }

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 };

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,