Extend two-phase borrows to apply to method receiver autorefs

sapphire-arches · sapphire-arches · commit d37a7ab32b4f · 2018-03-24T22:00:38.000-04:00
This is required to compile things like

src/test/ui/borrowck/two-phase-method-receivers.rs
diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs
@@ -434,7 +434,8 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
                     let f = self.expr_ty.fn_sig(fcx.tcx);
                     let res = fcx.try_coerce(self.expr,
                                              self.expr_ty,
-                                             fcx.tcx.mk_fn_ptr(f));
+                                             fcx.tcx.mk_fn_ptr(f),
+                                             false);
                     if !res.is_ok() {
                         return Err(CastError::NonScalar);
                     }
@@ -616,7 +617,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
     }
 
     fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> bool {
-        fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty).is_ok()
+        fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, false).is_ok()
     }
 }
 
diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs
@@ -84,6 +84,12 @@ struct Coerce<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
     fcx: &'a FnCtxt<'a, 'gcx, 'tcx>,
     cause: ObligationCause<'tcx>,
     use_lub: bool,
+    /// Determines whether or not allow_two_phase_borrow is set on any
+    /// autoref adjustments we create while coercing. We don't want to
+    /// allow deref coercions to create two-phase borrows, at least initially,
+    /// but we do need two-phase borrows for function argument reborrows.
+    /// See #47489 and #48598
+    allow_two_phase: bool,
 }
 
 impl<'a, 'gcx, 'tcx> Deref for Coerce<'a, 'gcx, 'tcx> {
@@ -123,10 +129,13 @@ fn success<'tcx>(adj: Vec<Adjustment<'tcx>>,
 }
 
 impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
-    fn new(fcx: &'f FnCtxt<'f, 'gcx, 'tcx>, cause: ObligationCause<'tcx>) -> Self {
+    fn new(fcx: &'f FnCtxt<'f, 'gcx, 'tcx>,
+           cause: ObligationCause<'tcx>,
+           allow_two_phase: bool) -> Self {
         Coerce {
             fcx,
             cause,
+            allow_two_phase,
             use_lub: false,
         }
     }
@@ -424,10 +433,7 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
         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,
+                allow_two_phase_borrow: self.allow_two_phase,
             }
         };
         adjustments.push(Adjustment {
@@ -473,6 +479,9 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
                 let mutbl = match mt_b.mutbl {
                     hir::MutImmutable => AutoBorrowMutability::Immutable,
                     hir::MutMutable => AutoBorrowMutability::Mutable {
+                        // We don't allow two-phase borrows here, at least for initial
+                        // implementation. If it happens that this coercion is a function argument,
+                        // the reborrow in coerce_borrowed_ptr will pick it up.
                         allow_two_phase_borrow: false,
                     }
                 };
@@ -751,13 +760,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     pub fn try_coerce(&self,
                       expr: &hir::Expr,
                       expr_ty: Ty<'tcx>,
-                      target: Ty<'tcx>)
+                      target: Ty<'tcx>,
+                      allow_two_phase: bool)
                       -> RelateResult<'tcx, Ty<'tcx>> {
         let source = self.resolve_type_vars_with_obligations(expr_ty);
         debug!("coercion::try({:?}: {:?} -> {:?})", expr, source, target);
 
         let cause = self.cause(expr.span, ObligationCauseCode::ExprAssignable);
-        let coerce = Coerce::new(self, cause);
+        let coerce = Coerce::new(self, cause, allow_two_phase);
         let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;
 
         let (adjustments, _) = self.register_infer_ok_obligations(ok);
@@ -771,7 +781,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
         debug!("coercion::can({:?} -> {:?})", source, target);
 
         let cause = self.cause(syntax_pos::DUMMY_SP, ObligationCauseCode::ExprAssignable);
-        let coerce = Coerce::new(self, cause);
+        // We don't ever need two-phase here since we throw out the result of the coercion
+        let coerce = Coerce::new(self, cause, false);
         self.probe(|_| coerce.coerce(source, target)).is_ok()
     }
 
@@ -840,7 +851,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             return Ok(fn_ptr);
         }
 
-        let mut coerce = Coerce::new(self, cause.clone());
+        // Configure a Coerce instance to compute the LUB.
+        // We don't allow two-phase borrows on any autorefs this creates since we
+        // probably aren't processing function arguments here and even if we were,
+        // they're going to get autorefed again anyway and we can apply 2-phase borrows
+        // at that time.
+        let mut coerce = Coerce::new(self, cause.clone(), false);
         coerce.use_lub = true;
 
         // First try to coerce the new expression to the type of the previous ones,
@@ -1106,7 +1122,8 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
             if self.pushed == 0 {
                 // Special-case the first expression we are coercing.
                 // To be honest, I'm not entirely sure why we do this.
-                fcx.try_coerce(expression, expression_ty, self.expected_ty)
+                // We don't allow two-phase borrows, see comment in try_find_coercion_lub for why
+                fcx.try_coerce(expression, expression_ty, self.expected_ty, false)
             } else {
                 match self.expressions {
                     Expressions::Dynamic(ref exprs) =>
diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs
@@ -79,9 +79,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     pub fn demand_coerce(&self,
                          expr: &hir::Expr,
                          checked_ty: Ty<'tcx>,
-                         expected: Ty<'tcx>)
+                         expected: Ty<'tcx>,
+                         allow_two_phase: bool)
                          -> Ty<'tcx> {
-        let (ty, err) = self.demand_coerce_diag(expr, checked_ty, expected);
+        let (ty, err) = self.demand_coerce_diag(expr, checked_ty, expected, allow_two_phase);
         if let Some(mut err) = err {
             err.emit();
         }
@@ -96,11 +97,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
     pub fn demand_coerce_diag(&self,
                               expr: &hir::Expr,
                               checked_ty: Ty<'tcx>,
-                              expected: Ty<'tcx>)
+                              expected: Ty<'tcx>,
+                              allow_two_phase: bool)
                               -> (Ty<'tcx>, Option<DiagnosticBuilder<'tcx>>) {
         let expected = self.resolve_type_vars_with_obligations(expected);
 
-        let e = match self.try_coerce(expr, checked_ty, expected) {
+        let e = match self.try_coerce(expr, checked_ty, expected, allow_two_phase) {
             Ok(ty) => return (ty, None),
             Err(e) => e
         };
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
@@ -2649,7 +2649,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 //    to, which is `expected_ty` if `rvalue_hint` returns an
                 //    `ExpectHasType(expected_ty)`, or the `formal_ty` otherwise.
                 let coerce_ty = expected.and_then(|e| e.only_has_type(self));
-                self.demand_coerce(&arg, checked_ty, coerce_ty.unwrap_or(formal_ty));
+                self.demand_coerce(&arg, checked_ty, coerce_ty.unwrap_or(formal_ty), true);
 
                 // 3. Relate the expected type and the formal one,
                 //    if the expected type was used for the coercion.
@@ -2812,7 +2812,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
             expr,
             ExpectHasType(expected),
             needs);
-        self.demand_coerce(expr, ty, expected)
+        self.demand_coerce(expr, ty, expected, false)
     }
 
     fn check_expr_with_hint(&self, expr: &'gcx hir::Expr,
@@ -4112,7 +4112,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                   let base_t = self.structurally_resolved_type(expr.span, base_t);
                   match self.lookup_indexing(expr, base, base_t, idx_t, needs) {
                       Some((index_ty, element_ty)) => {
-                          self.demand_coerce(idx, idx_t, index_ty);
+                          // two-phase not needed because index_ty is never mutable
+                          self.demand_coerce(idx, idx_t, index_ty, false);
                           element_ty
                       }
                       None => {
diff --git a/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs b/src/test/compile-fail/borrowck/two-phase-nonrecv-autoref.rs
@@ -8,7 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// revisions: lxl nll
+// revisions: ast lxl nll
+//[ast]compile-flags:
 //[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
 //[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
 
@@ -33,17 +34,14 @@
 
 use std::ops::{Index, IndexMut};
 
-// 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]
+    //[ast]~^ ERROR cannot use `*x` because it was mutably borrowed [E0503]
+    // Above error is a known limitation of AST borrowck
 }
 
 // While adding a flag to adjustments (indicating whether they
@@ -74,22 +72,25 @@ fn overloaded_call_traits() {
         //[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
         //[g2p]~^^^ ERROR cannot borrow `*f` as mutable more than once at a time
+        //[ast]~^^^^ 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`
+        //[lxl]~^    ERROR use of moved value: `*f`
+        //[nll]~^^   ERROR use of moved value: `*f`
+        //[g2p]~^^^  ERROR use of moved value: `*f`
+        //[ast]~^^^^ 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
-        //[g2p]~^^^ 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
+        //[ast]~^^^^ ERROR cannot borrow `*f` as mutable more than once at a time
     }
     fn twice_ten_oi(f: &mut Fn(i32) -> i32) {
         f(f(10));
@@ -105,6 +106,7 @@ fn overloaded_call_traits() {
         //[g2p]~^^^^^^^       ERROR cannot move a value of type
         //[g2p]~^^^^^^^^      ERROR cannot move a value of type
         //[g2p]~^^^^^^^^^     ERROR use of moved value: `*f`
+        //[ast]~^^^^^^^^^^    ERROR use of moved value: `*f`
     }
 
     twice_ten_sm(&mut |x| x + 1);
@@ -142,12 +144,15 @@ fn coerce_unsized() {
 
     // 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]
+    //[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]
+    //[ast]~^^^^ ERROR cannot borrow `a` as immutable because it is also borrowed as mutable [E0502]
 
     // But this is okay.
     a.m(a.i(10));
+    //[ast]~^ ERROR cannot borrow `a` as immutable because it is also borrowed as mutable [E0502]
+    // Above error is an expected limitation of AST borrowck
 }
 
 struct I(i32);
@@ -168,21 +173,25 @@ impl IndexMut<i32> for I {
 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]
+    //[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]
+    //[ast]~^^^ 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]
+    //[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]
+    //[ast]~^^^ ERROR cannot borrow `i` as immutable because it is also borrowed as mutable [E0502]
 }
 
 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());
+    //[ast]~^ ERROR cannot borrow `v` as immutable because it is also borrowed as mutable [E0502]
+    // Error above is an expected limitation of AST borrowck
 
     // (as a rule, pnkfelix does not like to write tests with dead code.)
 
@@ -192,9 +201,13 @@ fn main() {
 
     let mut s = S;
     s.m(s.i(10));
+    //[ast]~^ ERROR cannot borrow `s` as immutable because it is also borrowed as mutable [E0502]
+    // Error above is an expected limitation of AST borrowck
 
     let mut t = T;
     t.m(t.i(10));
+    //[ast]~^ ERROR cannot borrow `t` as immutable because it is also borrowed as mutable [E0502]
+    // Error above is an expected limitation of AST borrowck
 
     coerce_unsized();
     coerce_index_op();
diff --git a/src/test/ui/borrowck/two-phase-method-receivers.rs b/src/test/ui/borrowck/two-phase-method-receivers.rs
@@ -0,0 +1,29 @@
+// 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
+//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
+//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
+
+// run-pass
+
+struct Foo<'a> {
+    x: &'a i32
+}
+
+impl<'a> Foo<'a> {
+    fn method(&mut self, _: &i32) {
+    }
+}
+
+fn main() {
+    let a = &mut Foo { x: &22 };
+    Foo::method(a, a.x);
+}

Original file line number	Diff line number	Diff line change
`@@ -434,7 +434,8 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {`
`434`	`434`	`let f = self.expr_ty.fn_sig(fcx.tcx);`
`435`	`435`	`let res = fcx.try_coerce(self.expr,`
`436`	`436`	`self.expr_ty,`
`437`		`- fcx.tcx.mk_fn_ptr(f));`
	`437`	`+ fcx.tcx.mk_fn_ptr(f),`
	`438`	`+ false);`
`438`	`439`	`if !res.is_ok() {`
`439`	`440`	`return Err(CastError::NonScalar);`
`440`	`441`	`}`
`@@ -616,7 +617,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {`
`616`	`617`	`}`
`617`	`618`
`618`	`619`	`fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> bool {`
`619`		`- fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty).is_ok()`
	`620`	`+ fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, false).is_ok()`
`620`	`621`	`}`
`621`	`622`	`}`
`622`	`623`