Skip to content

Commit

Permalink
Auto merge of #41651 - arielb1:missing-adjustment-2, r=eddyb
Browse files Browse the repository at this point in the history 
refactor the handling of lvalue ops

I think I got the code into a "mostly sane" situation.

Fixes #41604.

beta-nominating because fixes regression in #41578. I think I can do a smaller fix, but the previous code is too fragile.

r? @eddyb
  • Loading branch information
@bors
bors committed Apr 30, 2017
2 parents c0f86f5 + b7b3c23 commit 06fb4d2
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 230 deletions.
41 changes: 7 additions & 34 deletions src/librustc_typeck/check/autoderef.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@

use astconv::AstConv;

use super::FnCtxt;
use super::{FnCtxt, LvalueOp};

use check::coercion::AsCoercionSite;
use rustc::infer::InferOk;
use rustc::traits;
use rustc::ty::{self, Ty, TraitRef};
use rustc::ty::{ToPredicate, TypeFoldable};
use rustc::ty::{MethodCall, MethodCallee};
use rustc::ty::{LvaluePreference, NoPreference, PreferMutLvalue};
use rustc::ty::{LvaluePreference, NoPreference};
use rustc::hir;

use syntax_pos::Span;
Expand Down Expand Up @@ -213,39 +213,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
span: Span,
base_expr: Option<&hir::Expr>,
base_ty: Ty<'tcx>,
lvalue_pref: LvaluePreference)
pref: LvaluePreference)
-> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!("try_overloaded_deref({:?},{:?},{:?},{:?})",
span,
base_expr,
base_ty,
lvalue_pref);
// Try DerefMut first, if preferred.
let method = match (lvalue_pref, self.tcx.lang_items.deref_mut_trait()) {
(PreferMutLvalue, Some(trait_did)) => {
self.lookup_method_in_trait(span,
base_expr,
Symbol::intern("deref_mut"),
trait_did,
base_ty,
None)
}
_ => None,
};

// Otherwise, fall back to Deref.
let method = match (method, self.tcx.lang_items.deref_trait()) {
(None, Some(trait_did)) => {
self.lookup_method_in_trait(span,
base_expr,
Symbol::intern("deref"),
trait_did,
base_ty,
None)
}
(method, _) => method,
};
let rcvr = base_expr.map(|base_expr| super::AdjustedRcvr {
rcvr_expr: base_expr, autoderefs: 0, unsize: false
});

method
self.try_overloaded_lvalue_op(span, rcvr, base_ty, &[], pref, LvalueOp::Deref)
}
}
8 changes: 5 additions & 3 deletions src/librustc_typeck/check/callee.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
};

match self.lookup_method_in_trait_adjusted(call_expr.span,
Some(&callee_expr),
Some(super::AdjustedRcvr {
rcvr_expr: callee_expr,
autoderefs,
unsize: false
}),
method_name,
trait_def_id,
autoderefs,
false,
adjusted_ty,
None) {
None => continue,
Expand Down
178 changes: 61 additions & 117 deletions src/librustc_typeck/check/method/confirm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use super::probe;

use check::{FnCtxt, callee};
use check::{FnCtxt, LvalueOp, callee};
use hir::def_id::DefId;
use rustc::ty::subst::Substs;
use rustc::traits;
Expand Down Expand Up @@ -433,137 +433,81 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
for (i, &expr) in exprs.iter().rev().enumerate() {
debug!("convert_lvalue_derefs_to_mutable: i={} expr={:?}", i, expr);

// Count autoderefs. We don't need to fix up the autoref - the parent
// expression will fix them up for us.
let adjustment = self.tables.borrow().adjustments.get(&expr.id).cloned();
match adjustment {
Some(Adjustment { kind: Adjust::DerefRef { autoderefs, .. }, .. }) => {
if autoderefs > 0 {
let mut autoderef = self.autoderef(expr.span, self.node_ty(expr.id));
autoderef.nth(autoderefs).unwrap_or_else(|| {
span_bug!(expr.span,
"expr was deref-able {} times but now isn't?",
autoderefs);
});
autoderef.finalize(PreferMutLvalue, expr);
// Fix up the adjustment.
let autoderefs = match self.tables.borrow_mut().adjustments.get_mut(&expr.id) {
Some(&mut Adjustment {
kind: Adjust::DerefRef { autoderefs, ref mut autoref, .. }, ref mut target
}) => {
if let &mut Some(AutoBorrow::Ref(_, ref mut mutbl)) = autoref {
*mutbl = hir::Mutability::MutMutable;
*target = match target.sty {
ty::TyRef(r, ty::TypeAndMut { ty, .. }) =>
self.tcx.mk_ref(r, ty::TypeAndMut { ty, mutbl: *mutbl }),
_ => span_bug!(expr.span, "AutoBorrow::Ref resulted in non-ref {:?}",
target)
};
}
autoderefs
}
Some(_) | None => {}
Some(_) | None => 0
};

if autoderefs > 0 {
let mut autoderef = self.autoderef(expr.span, self.node_ty(expr.id));
autoderef.nth(autoderefs).unwrap_or_else(|| {
span_bug!(expr.span,
"expr was deref-able {} times but now isn't?",
autoderefs);
});
autoderef.finalize(PreferMutLvalue, expr);
}

// Don't retry the first one or we might infinite loop!
if i == 0 {
continue;
}
match expr.node {
hir::ExprIndex(ref base_expr, ref index_expr) => {
// If this is an overloaded index, the
// adjustment will include an extra layer of
// autoref because the method is an &self/&mut
// self method. We have to peel it off to get
// the raw adjustment that `try_index_step`
// expects. This is annoying and horrible. We
// ought to recode this routine so it doesn't
// (ab)use the normal type checking paths.
let adj = self.tables.borrow_mut().adjustments.remove(&base_expr.id);
let (autoderefs, unsize, adjusted_base_ty) = match adj {
Some(Adjustment {
kind: Adjust::DerefRef { autoderefs, autoref, unsize },
target
}) => {
match autoref {
None => {
assert!(!unsize);
}
Some(AutoBorrow::Ref(..)) => {}
Some(_) => {
span_bug!(base_expr.span,
"unexpected adjustment autoref {:?}",
adj);
}
}

(autoderefs, unsize, if unsize {
target.builtin_deref(false, NoPreference)
.expect("fixup: AutoBorrow::Ref is not &T")
.ty
} else {
let ty = self.node_ty(base_expr.id);
let mut ty = self.shallow_resolve(ty);
let mut method_type = |method_call: ty::MethodCall| {
self.tables.borrow().method_map.get(&method_call).map(|m| {
self.resolve_type_vars_if_possible(&m.ty)
})
};

if !ty.references_error() {
for i in 0..autoderefs {
ty = ty.adjust_for_autoderef(self.tcx,
base_expr.id,
base_expr.span,
i as u32,
&mut method_type);
}
}

ty
})
}
None => (0, false, self.node_ty(base_expr.id)),
Some(_) => {
span_bug!(base_expr.span, "unexpected adjustment type");
}
};

let index_expr_ty = self.node_ty(index_expr.id);
let adjusted_base_ty = self.resolve_type_vars_if_possible(&adjusted_base_ty);
let index_expr_ty = self.resolve_type_vars_if_possible(&index_expr_ty);

let result = self.try_index_step(ty::MethodCall::expr(expr.id),
expr,
&base_expr,
adjusted_base_ty,
autoderefs,
unsize,
PreferMutLvalue,
index_expr_ty);

if let Some((input_ty, return_ty)) = result {
self.demand_suptype(index_expr.span, input_ty, index_expr_ty);

let expr_ty = self.node_ty(expr.id);
self.demand_suptype(expr.span, expr_ty, return_ty);
} else {
// We could not perform a mutable index. Re-apply the
// immutable index adjustments - borrowck will detect
// this as an error.
if let Some(adjustment) = adjustment {
self.apply_adjustment(expr.id, adjustment);
}
self.tcx.sess.delay_span_bug(
expr.span, "convert_lvalue_derefs_to_mutable failed");
}
self.convert_lvalue_op_to_mutable(
LvalueOp::Index, expr, base_expr, &[index_expr_ty]);
}
hir::ExprUnary(hir::UnDeref, ref base_expr) => {
// if this is an overloaded deref, then re-evaluate with
// a preference for mut
let method_call = ty::MethodCall::expr(expr.id);
if self.tables.borrow().method_map.contains_key(&method_call) {
self.tables.borrow_mut().adjustments.remove(&base_expr.id);
let method = self.try_overloaded_deref(expr.span,
Some(&base_expr),
self.node_ty(base_expr.id),
PreferMutLvalue);
let ok = method.expect("re-trying deref failed");
let method = self.register_infer_ok_obligations(ok);
self.tables.borrow_mut().method_map.insert(method_call, method);
}
self.convert_lvalue_op_to_mutable(
LvalueOp::Deref, expr, base_expr, &[]);
}
_ => {}
}
}
}

fn convert_lvalue_op_to_mutable(&self,
op: LvalueOp,
expr: &hir::Expr,
base_expr: &hir::Expr,
arg_tys: &[Ty<'tcx>])
{
debug!("convert_lvalue_op_to_mutable({:?}, {:?}, {:?}, {:?})",
op, expr, base_expr, arg_tys);
let method_call = ty::MethodCall::expr(expr.id);
if !self.tables.borrow().method_map.contains_key(&method_call) {
debug!("convert_lvalue_op_to_mutable - builtin, nothing to do");
return
}

let base_ty = self.tables.borrow().adjustments.get(&base_expr.id)
.map_or_else(|| self.node_ty(expr.id), |adj| adj.target);
let base_ty = self.resolve_type_vars_if_possible(&base_ty);

// Need to deref because overloaded lvalue ops take self by-reference.
let base_ty = base_ty.builtin_deref(false, NoPreference)
.expect("lvalue op takes something that is not a ref")
.ty;

let method = self.try_overloaded_lvalue_op(
expr.span, None, base_ty, arg_tys, PreferMutLvalue, op);
let ok = method.expect("re-trying op failed");
let method = self.register_infer_ok_obligations(ok);
debug!("convert_lvalue_op_to_mutable: method={:?}", method);
self.tables.borrow_mut().method_map.insert(method_call, method);
}

///////////////////////////////////////////////////////////////////////////
// MISCELLANY

Expand Down
34 changes: 7 additions & 27 deletions src/librustc_typeck/check/method/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

//! Method lookup: the secret sauce of Rust. See `README.md`.

use check::FnCtxt;
use check::{FnCtxt, AdjustedRcvr};
use hir::def::Def;
use hir::def_id::DefId;
use rustc::ty::subst::Substs;
Expand Down Expand Up @@ -153,24 +153,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
supplied_method_types))
}

pub fn lookup_method_in_trait(&self,
span: Span,
self_expr: Option<&hir::Expr>,
m_name: ast::Name,
trait_def_id: DefId,
self_ty: ty::Ty<'tcx>,
opt_input_types: Option<Vec<ty::Ty<'tcx>>>)
-> Option<InferOk<'tcx, ty::MethodCallee<'tcx>>> {
self.lookup_method_in_trait_adjusted(span,
self_expr,
m_name,
trait_def_id,
0,
false,
self_ty,
opt_input_types)
}

/// `lookup_in_trait_adjusted` is used for overloaded operators.
/// It does a very narrow slice of what the normal probe/confirm path does.
/// In particular, it doesn't really do any probing: it simply constructs
Expand All @@ -184,18 +166,16 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
/// this method is basically the same as confirmation.
pub fn lookup_method_in_trait_adjusted(&self,
span: Span,
self_expr: Option<&hir::Expr>,
self_info: Option<AdjustedRcvr>,
m_name: ast::Name,
trait_def_id: DefId,
autoderefs: usize,
unsize: bool,
self_ty: ty::Ty<'tcx>,
opt_input_types: Option<Vec<ty::Ty<'tcx>>>)
-> Option<InferOk<'tcx, ty::MethodCallee<'tcx>>> {
debug!("lookup_in_trait_adjusted(self_ty={:?}, self_expr={:?}, \
debug!("lookup_in_trait_adjusted(self_ty={:?}, self_info={:?}, \
m_name={}, trait_def_id={:?})",
self_ty,
self_expr,
self_info,
m_name,
trait_def_id);

Expand Down Expand Up @@ -288,10 +268,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
obligations.push(traits::Obligation::new(cause, ty::Predicate::WellFormed(method_ty)));

// Insert any adjustments needed (always an autoref of some mutability).
if let Some(self_expr) = self_expr {
if let Some(AdjustedRcvr { rcvr_expr, autoderefs, unsize }) = self_info {
debug!("lookup_in_trait_adjusted: inserting adjustment if needed \
(self-id={}, autoderefs={}, unsize={}, fty={:?})",
self_expr.id, autoderefs, unsize, original_method_ty);
rcvr_expr.id, autoderefs, unsize, original_method_ty);

let original_sig = original_method_ty.fn_sig();
let autoref = match (&original_sig.input(0).skip_binder().sty,
Expand All @@ -308,7 +288,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
};

self.apply_adjustment(self_expr.id, Adjustment {
self.apply_adjustment(rcvr_expr.id, Adjustment {
kind: Adjust::DerefRef {
autoderefs: autoderefs,
autoref: autoref,
Expand Down
Loading

0 comments on commit 06fb4d2

Please sign in to comment.