Use a new type to track if two-phase borrows are allowed

Because more type safe is more better, and random boolean parameters everywhere
were not the greatest thing.
This commit is contained in:
bobtwinkles 2018-03-27 23:48:50 -04:00
parent d37a7ab32b
commit 96ae0ee382
5 changed files with 41 additions and 16 deletions

View File

@ -119,6 +119,23 @@ impl<'a, 'gcx, 'tcx> OverloadedDeref<'tcx> {
}
}
/// At least for initial deployment, we want to limit two-phase borrows to
/// only a few specific cases. Right now, those mostly "things that desugar"
/// into method calls
/// - using x.some_method() syntax, where some_method takes &mut self
/// - using Foo::some_method(&mut x, ...) syntax
/// - binary assignment operators (+=, -=, *=, etc.)
/// Anything else should be rejected until generalized two phase borrow support
/// is implemented. Right now, dataflow can't handle the general case where there
/// is more than one use of a mutable borrow, and we don't want to accept too much
/// new code via two-phase borrows, so we try to limit where we create two-phase
/// capable mutable borrows.
/// See #49434 for tracking.
pub enum AllowTwoPhase {
Yes,
No
}
#[derive(Copy, Clone, PartialEq, Debug, RustcEncodable, RustcDecodable)]
pub enum AutoBorrowMutability {
Mutable { allow_two_phase_borrow: bool },

View File

@ -47,6 +47,7 @@ use rustc::hir;
use rustc::session::Session;
use rustc::traits;
use rustc::ty::{self, Ty, TypeFoldable};
use rustc::ty::adjustment::AllowTwoPhase;
use rustc::ty::cast::{CastKind, CastTy};
use rustc::ty::subst::Substs;
use rustc::middle::lang_items;
@ -435,7 +436,7 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
let res = fcx.try_coerce(self.expr,
self.expr_ty,
fcx.tcx.mk_fn_ptr(f),
false);
AllowTwoPhase::No);
if !res.is_ok() {
return Err(CastError::NonScalar);
}
@ -617,7 +618,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, false).is_ok()
fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No).is_ok()
}
}

View File

@ -67,7 +67,7 @@ use rustc::hir::def_id::DefId;
use rustc::infer::{Coercion, InferResult, InferOk};
use rustc::infer::type_variable::TypeVariableOrigin;
use rustc::traits::{self, ObligationCause, ObligationCauseCode};
use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability};
use rustc::ty::adjustment::{Adjustment, Adjust, AllowTwoPhase, AutoBorrow, AutoBorrowMutability};
use rustc::ty::{self, TypeAndMut, Ty, ClosureSubsts};
use rustc::ty::fold::TypeFoldable;
use rustc::ty::error::TypeError;
@ -89,7 +89,8 @@ struct Coerce<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
/// 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,
/// See docs on the "AllowTwoPhase" type for a more detailed discussion
allow_two_phase: AllowTwoPhase,
}
impl<'a, 'gcx, 'tcx> Deref for Coerce<'a, 'gcx, 'tcx> {
@ -131,7 +132,7 @@ 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>,
allow_two_phase: bool) -> Self {
allow_two_phase: AllowTwoPhase) -> Self {
Coerce {
fcx,
cause,
@ -433,7 +434,10 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
let mutbl = match mt_b.mutbl {
hir::MutImmutable => AutoBorrowMutability::Immutable,
hir::MutMutable => AutoBorrowMutability::Mutable {
allow_two_phase_borrow: self.allow_two_phase,
allow_two_phase_borrow: match self.allow_two_phase {
AllowTwoPhase::Yes => true,
AllowTwoPhase::No => false
},
}
};
adjustments.push(Adjustment {
@ -761,7 +765,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr: &hir::Expr,
expr_ty: Ty<'tcx>,
target: Ty<'tcx>,
allow_two_phase: bool)
allow_two_phase: AllowTwoPhase)
-> RelateResult<'tcx, Ty<'tcx>> {
let source = self.resolve_type_vars_with_obligations(expr_ty);
debug!("coercion::try({:?}: {:?} -> {:?})", expr, source, target);
@ -782,7 +786,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let cause = self.cause(syntax_pos::DUMMY_SP, ObligationCauseCode::ExprAssignable);
// We don't ever need two-phase here since we throw out the result of the coercion
let coerce = Coerce::new(self, cause, false);
let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
self.probe(|_| coerce.coerce(source, target)).is_ok()
}
@ -856,7 +860,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// 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);
let mut coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No);
coerce.use_lub = true;
// First try to coerce the new expression to the type of the previous ones,
@ -1123,7 +1127,7 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
// Special-case the first expression we are coercing.
// To be honest, I'm not entirely sure why we do this.
// 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)
fcx.try_coerce(expression, expression_ty, self.expected_ty, AllowTwoPhase::No)
} else {
match self.expressions {
Expressions::Dynamic(ref exprs) =>

View File

@ -21,6 +21,7 @@ use rustc::hir;
use rustc::hir::print;
use rustc::hir::def::Def;
use rustc::ty::{self, Ty, AssociatedItem};
use rustc::ty::adjustment::AllowTwoPhase;
use errors::{DiagnosticBuilder, CodeMapper};
use super::method::probe;
@ -80,7 +81,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
allow_two_phase: bool)
allow_two_phase: AllowTwoPhase)
-> Ty<'tcx> {
let (ty, err) = self.demand_coerce_diag(expr, checked_ty, expected, allow_two_phase);
if let Some(mut err) = err {
@ -98,7 +99,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>,
allow_two_phase: bool)
allow_two_phase: AllowTwoPhase)
-> (Ty<'tcx>, Option<DiagnosticBuilder<'tcx>>) {
let expected = self.resolve_type_vars_with_obligations(expected);

View File

@ -97,7 +97,7 @@ use rustc::mir::interpret::{GlobalId};
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, AutoBorrowMutability};
use rustc::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability};
use rustc::ty::fold::TypeFoldable;
use rustc::ty::maps::Providers;
use rustc::ty::util::{Representability, IntTypeExt, Discr};
@ -2649,7 +2649,8 @@ 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), true);
// We're processing function arguments so we definitely want to use two-phase borrows.
self.demand_coerce(&arg, checked_ty, coerce_ty.unwrap_or(formal_ty), AllowTwoPhase::Yes);
// 3. Relate the expected type and the formal one,
// if the expected type was used for the coercion.
@ -2812,7 +2813,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
expr,
ExpectHasType(expected),
needs);
self.demand_coerce(expr, ty, expected, false)
// checks don't need two phase
self.demand_coerce(expr, ty, expected, AllowTwoPhase::No)
}
fn check_expr_with_hint(&self, expr: &'gcx hir::Expr,
@ -4113,7 +4115,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
match self.lookup_indexing(expr, base, base_t, idx_t, needs) {
Some((index_ty, element_ty)) => {
// two-phase not needed because index_ty is never mutable
self.demand_coerce(idx, idx_t, index_ty, false);
self.demand_coerce(idx, idx_t, index_ty, AllowTwoPhase::No);
element_ty
}
None => {