Skip to content

Commit

Permalink
librustc: Improve method autoderef/deref/index behavior more, and enable
Browse files Browse the repository at this point in the history
`IndexMut` on mutable vectors.

This fixes a bug whereby the mutability fixups for method behavior were
not kicking in after autoderef failed to happen at any level. It also
adds support for `Index` to the fixer-upper.

Closes rust-lang#12825.
  • Loading branch information
pcwalton committed Oct 14, 2014
1 parent dfd5281 commit f7fb387
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 56 deletions.
7 changes: 3 additions & 4 deletions src/libcollections/vec.rs
Expand Up @@ -452,13 +452,13 @@ impl<T> Index<uint,T> for Vec<T> {
}
}

// FIXME(#12825) Indexing will always try IndexMut first and that causes issues.
/*impl<T> IndexMut<uint,T> for Vec<T> {
#[cfg(not(stage0))]
impl<T> IndexMut<uint,T> for Vec<T> {
#[inline]
fn index_mut<'a>(&'a mut self, index: &uint) -> &'a mut T {
self.get_mut(*index)
}
}*/
}

#[cfg(stage0)]
impl<T> ops::Slice<uint, [T]> for Vec<T> {
Expand Down Expand Up @@ -2191,7 +2191,6 @@ impl<T> Vec<T> {
}
}


#[cfg(test)]
mod tests {
extern crate test;
Expand Down
107 changes: 66 additions & 41 deletions src/librustc/middle/typeck/check/method.rs
Expand Up @@ -359,8 +359,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {

match result {
Some(Some(result)) => {
self.fixup_derefs_on_method_receiver_if_necessary(&result,
self_ty);
self.fixup_derefs_on_method_receiver_if_necessary(&result);
Some(result)
}
_ => None
Expand Down Expand Up @@ -1388,8 +1387,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {

fn fixup_derefs_on_method_receiver_if_necessary(
&self,
method_callee: &MethodCallee,
self_ty: ty::t) {
method_callee: &MethodCallee) {
let sig = match ty::get(method_callee.ty).sty {
ty::ty_bare_fn(ref f) => f.sig.clone(),
ty::ty_closure(ref f) => f.sig.clone(),
Expand All @@ -1404,55 +1402,82 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
_ => return,
}

// Fix up autoderefs and derefs.
let mut self_expr = match self.self_expr {
Some(expr) => expr,
None => return,
};
// Gather up expressions we want to munge.
let mut exprs = Vec::new();
match self.self_expr {
Some(expr) => exprs.push(expr),
None => {}
}
loop {
if exprs.len() == 0 {
break
}
let last = exprs[exprs.len() - 1];
match last.node {
ast::ExprParen(ref expr) |
ast::ExprField(ref expr, _, _) |
ast::ExprTupField(ref expr, _, _) |
ast::ExprSlice(ref expr, _, _, _) |
ast::ExprIndex(ref expr, _) |
ast::ExprUnary(ast::UnDeref, ref expr) => exprs.push(&**expr),
_ => break,
}
}

// Fix up autoderefs and derefs.
for (i, expr) in exprs.iter().rev().enumerate() {
// Count autoderefs.
let autoderef_count = match self.fcx
.inh
.adjustments
.borrow()
.find(&self_expr.id) {
.find(&expr.id) {
Some(&ty::AdjustDerefRef(ty::AutoDerefRef {
autoderefs: autoderef_count,
autoref: _
})) if autoderef_count > 0 => autoderef_count,
Some(_) | None => return,
})) => autoderef_count,
Some(_) | None => 0,
};

check::autoderef(self.fcx,
self_expr.span,
self.fcx.expr_ty(self_expr),
Some(self_expr.id),
PreferMutLvalue,
|_, autoderefs| {
if autoderefs == autoderef_count + 1 {
Some(())
} else {
None
}
});

match self_expr.node {
ast::ExprParen(ref expr) |
ast::ExprIndex(ref expr, _) |
ast::ExprField(ref expr, _, _) |
ast::ExprTupField(ref expr, _, _) |
ast::ExprSlice(ref expr, _, _, _) => self_expr = &**expr,
ast::ExprUnary(ast::UnDeref, ref expr) => {
drop(check::try_overloaded_deref(
self.fcx,
self_expr.span,
Some(MethodCall::expr(self_expr.id)),
Some(self_expr),
self_ty,
PreferMutLvalue));
self_expr = &**expr
if autoderef_count > 0 {
check::autoderef(self.fcx,
expr.span,
self.fcx.expr_ty(*expr),
Some(expr.id),
PreferMutLvalue,
|_, autoderefs| {
if autoderefs == autoderef_count + 1 {
Some(())
} else {
None
}
});
}

// Don't retry the first one or we might infinite loop!
if i != 0 {
match expr.node {
ast::ExprIndex(ref base_expr, ref index_expr) => {
check::try_overloaded_index(
self.fcx,
Some(MethodCall::expr(expr.id)),
*expr,
&**base_expr,
self.fcx.expr_ty(&**base_expr),
index_expr,
PreferMutLvalue);
}
ast::ExprUnary(ast::UnDeref, ref base_expr) => {
check::try_overloaded_deref(
self.fcx,
expr.span,
Some(MethodCall::expr(expr.id)),
Some(&**base_expr),
self.fcx.expr_ty(&**base_expr),
PreferMutLvalue);
}
_ => {}
}
_ => break,
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/librustc/middle/typeck/check/mod.rs
Expand Up @@ -2975,12 +2975,10 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
expr: &ast::Expr,
method_name: ast::SpannedIdent,
args: &[P<ast::Expr>],
tps: &[P<ast::Ty>]) {
tps: &[P<ast::Ty>],
lvalue_pref: LvaluePreference) {
let rcvr = &*args[0];
// We can't know if we need &mut self before we look up the method,
// so treat the receiver as mutable just in case - only explicit
// overloaded dereferences care about the distinction.
check_expr_with_lvalue_pref(fcx, &*rcvr, PreferMutLvalue);
check_expr_with_lvalue_pref(fcx, &*rcvr, lvalue_pref);

// no need to check for bot/err -- callee does that
let expr_t = structurally_resolved_type(fcx,
Expand Down Expand Up @@ -4195,7 +4193,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
}
}
ast::ExprMethodCall(ident, ref tps, ref args) => {
check_method_call(fcx, expr, ident, args.as_slice(), tps.as_slice());
check_method_call(fcx, expr, ident, args.as_slice(), tps.as_slice(), lvalue_pref);
let mut arg_tys = args.iter().map(|a| fcx.expr_ty(&**a));
let (args_bot, args_err) = arg_tys.fold((false, false),
|(rest_bot, rest_err), a| {
Expand Down
8 changes: 3 additions & 5 deletions src/test/run-pass/overloaded-deref-count.rs
Expand Up @@ -68,12 +68,10 @@ pub fn main() {
*n -= 3; // Mutable deref + assignment with binary operation.
assert_eq!(n.counts(), (2, 3));

// Mutable deref used for calling a method taking &self.
// N.B. This is required because method lookup hasn't been performed so
// we don't know whether the called method takes mutable self, before
// the dereference itself is type-checked (a chicken-and-egg problem).
// Immutable deref used for calling a method taking &self. (The
// typechecker is smarter now about doing this.)
(*n).to_string();
assert_eq!(n.counts(), (2, 4));
assert_eq!(n.counts(), (3, 3));

// Mutable deref used for calling a method taking &mut self.
(*v).push(2);
Expand Down

2 comments on commit f7fb387

@pcwalton
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=pnkfelix

@alexcrichton
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors: retry

Please sign in to comment.