auto merge of #17501 : pcwalton/rust/improve-method-lookup-autoderef, r=nikomatsakis

prefer `Deref` over `DerefMut` in all other circumstances.

Because the compiler now prefers `Deref`, this can break code that
looked like:

    let mut foo = bar.borrow_mut();
    (*foo).call_something_that_requires_mutable_self();

Replace this code with:

    let mut foo = bar.baz();
    (&mut *foo).call_something_that_requires_mutable_self();

Closes #12825.

[breaking-change]

r? @nikomatsakis

Author: bors

src/libgreen/basic.rs

@@ -116,7 +116,7 @@ impl EventLoop for BasicLoop {
             }
 
             unsafe {
-                let mut messages = self.messages.lock();
+                let messages = self.messages.lock();
                 // We block here if we have no messages to process and we may
                 // receive a message at a later date
                 if self.remotes.len() > 0 && messages.len() == 0 &&

src/librustc/middle/mem_categorization.rs

@@ -405,6 +405,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
             Some(adjustment) => {
                 match *adjustment {
                     ty::AdjustAddEnv(..) => {
+                        debug!("cat_expr(AdjustAddEnv): {}",
+                               expr.repr(self.tcx()));
                         // Convert a bare fn to a closure by adding NULL env.
                         // Result is an rvalue.
                         let expr_ty = if_ok!(self.expr_ty_adjusted(expr));
@@ -414,6 +416,8 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
                     ty::AdjustDerefRef(
                         ty::AutoDerefRef {
                             autoref: Some(_), ..}) => {
+                        debug!("cat_expr(AdjustDerefRef): {}",
+                               expr.repr(self.tcx()));
                         // Equivalent to &*expr or something similar.
                         // Result is an rvalue.
                         let expr_ty = if_ok!(self.expr_ty_adjusted(expr));
@@ -436,6 +440,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
                                autoderefs: uint)
                                -> McResult<cmt> {
         let mut cmt = if_ok!(self.cat_expr_unadjusted(expr));
+        debug!("cat_expr_autoderefd: autoderefs={}, cmt={}",
+               autoderefs,
+               cmt.repr(self.tcx()));
         for deref in range(1u, autoderefs + 1) {
             cmt = self.cat_deref(expr, cmt, deref, false);
         }
@@ -454,6 +461,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
 
           ast::ExprField(ref base, f_name, _) => {
             let base_cmt = if_ok!(self.cat_expr(&**base));
+            debug!("cat_expr(cat_field): id={} expr={} base={}",
+                   expr.id,
+                   expr.repr(self.tcx()),
+                   base_cmt.repr(self.tcx()));
             Ok(self.cat_field(expr, base_cmt, f_name.node, expr_ty))
           }
 

src/librustc/middle/typeck/check/method.rs

@@ -86,10 +86,11 @@ use middle::traits;
 use middle::ty::*;
 use middle::ty;
 use middle::typeck::astconv::AstConv;
-use middle::typeck::check::{FnCtxt, PreferMutLvalue, impl_self_ty};
+use middle::typeck::check::{FnCtxt, NoPreference, PreferMutLvalue};
+use middle::typeck::check::{impl_self_ty};
 use middle::typeck::check;
 use middle::typeck::infer;
-use middle::typeck::MethodCallee;
+use middle::typeck::{MethodCall, MethodCallee};
 use middle::typeck::{MethodOrigin, MethodParam, MethodTypeParam};
 use middle::typeck::{MethodStatic, MethodStaticUnboxedClosure, MethodObject, MethodTraitObject};
 use middle::typeck::check::regionmanip::replace_late_bound_regions_in_fn_sig;
@@ -353,11 +354,15 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
 
         let (_, _, result) =
             check::autoderef(
-                self.fcx, span, self_ty, self_expr_id, PreferMutLvalue,
+                self.fcx, span, self_ty, self_expr_id, NoPreference,
                 |self_ty, autoderefs| self.search_step(self_ty, autoderefs));
 
         match result {
-            Some(Some(result)) => Some(result),
+            Some(Some(result)) => {
+                self.fixup_derefs_on_method_receiver_if_necessary(&result,
+                                                                  self_ty);
+                Some(result)
+            }
             _ => None
         }
     }
@@ -430,7 +435,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
          */
 
         let span = self.self_expr.map_or(self.span, |e| e.span);
-        check::autoderef(self.fcx, span, self_ty, None, PreferMutLvalue, |self_ty, _| {
+        check::autoderef(self.fcx, span, self_ty, None, NoPreference, |self_ty, _| {
             match get(self_ty).sty {
                 ty_trait(box TyTrait { def_id, ref substs, bounds, .. }) => {
                     self.push_inherent_candidates_from_object(
@@ -458,7 +463,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
 
     fn push_bound_candidates(&mut self, self_ty: ty::t, restrict_to: Option<DefId>) {
         let span = self.self_expr.map_or(self.span, |e| e.span);
-        check::autoderef(self.fcx, span, self_ty, None, PreferMutLvalue, |self_ty, _| {
+        check::autoderef(self.fcx, span, self_ty, None, NoPreference, |self_ty, _| {
             match get(self_ty).sty {
                 ty_param(p) => {
                     self.push_inherent_candidates_from_param(self_ty, restrict_to, p);
@@ -1135,7 +1140,7 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
         };
 
         // This is hokey. We should have mutability inference as a
-        // variable.  But for now, try &const, then &, then &mut:
+        // variable.  But for now, try &, then &mut:
         let region =
             self.infcx().next_region_var(infer::Autoref(self.span));
         for mutbl in mutbls.iter() {
@@ -1381,6 +1386,77 @@ impl<'a, 'tcx> LookupContext<'a, 'tcx> {
         }
     }
 
+    fn fixup_derefs_on_method_receiver_if_necessary(
+            &self,
+            method_callee: &MethodCallee,
+            self_ty: ty::t) {
+        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(),
+            _ => return,
+        };
+
+        match ty::get(*sig.inputs.get(0)).sty {
+            ty::ty_rptr(_, ty::mt {
+                ty: _,
+                mutbl: ast::MutMutable,
+            }) => {}
+            _ => return,
+        }
+
+        // Fix up autoderefs and derefs.
+        let mut self_expr = match self.self_expr {
+            Some(expr) => expr,
+            None => return,
+        };
+        loop {
+            // Count autoderefs.
+            let autoderef_count = match self.fcx
+                                            .inh
+                                            .adjustments
+                                            .borrow()
+                                            .find(&self_expr.id) {
+                Some(&ty::AdjustDerefRef(ty::AutoDerefRef {
+                    autoderefs: autoderef_count,
+                    autoref: _
+                })) if autoderef_count > 0 => autoderef_count,
+                Some(_) | None => return,
+            };
+
+            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
+                }
+                _ => break,
+            }
+        }
+    }
+
     fn enforce_object_limitations(&self, candidate: &Candidate) {
         /*!
          * There are some limitations to calling functions through an

src/librustc/middle/typeck/check/mod.rs

@@ -2078,6 +2078,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     }
 }
 
+#[deriving(Show)]
 pub enum LvaluePreference {
     PreferMutLvalue,
     NoPreference

src/libsyntax/codemap.rs

@@ -290,6 +290,9 @@ impl FileMap {
     }
 
     /// get a line from the list of pre-computed line-beginnings
+    ///
+    /// NOTE(stage0, pcwalton): Remove `#[allow(unused_mut)]` after snapshot.
+    #[allow(unused_mut)]
     pub fn get_line(&self, line: int) -> String {
         let mut lines = self.lines.borrow_mut();
         let begin: BytePos = *lines.get(line as uint) - self.start_pos;
@@ -512,6 +515,8 @@ impl CodeMap {
         return a;
     }
 
+    // NOTE(stage0, pcwalton): Remove `#[allow(unused_mut)]` after snapshot.
+    #[allow(unused_mut)]
     fn lookup_line(&self, pos: BytePos) -> FileMapAndLine {
         let idx = self.lookup_filemap_idx(pos);
 

src/test/run-pass/dst-deref-mut.rs

@@ -27,7 +27,6 @@ impl DerefMut<[uint]> for Arr {
 }
 
 pub fn foo(arr: &mut Arr) {
-    assert!(arr.len() == 3);
     let x: &mut [uint] = &mut **arr;
     assert!(x[0] == 1);
     assert!(x[1] == 2);

src/test/run-pass/fixup-deref-mut.rs

@@ -0,0 +1,52 @@
+// Copyright 2014 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.
+
+// Generic unique/owned smaht pointer.
+struct Own<T> {
+    value: *mut T
+}
+
+impl<T> Deref<T> for Own<T> {
+    fn deref<'a>(&'a self) -> &'a T {
+        unsafe { &*self.value }
+    }
+}
+
+impl<T> DerefMut<T> for Own<T> {
+    fn deref_mut<'a>(&'a mut self) -> &'a mut T {
+        unsafe { &mut *self.value }
+    }
+}
+
+struct Point {
+    x: int,
+    y: int
+}
+
+impl Point {
+    fn get(&mut self) -> (int, int) {
+        (self.x, self.y)
+    }
+}
+
+fn test0(mut x: Own<Point>) {
+    let _ = x.get();
+}
+
+fn test1(mut x: Own<Own<Own<Point>>>) {
+    let _ = x.get();
+}
+
+fn test2(mut x: Own<Own<Own<Point>>>) {
+    let _ = (**x).get();
+}
+
+fn main() {}
+

src/test/run-pass/overloaded-autoderef-count.rs

@@ -74,7 +74,7 @@ pub fn main() {
     assert_eq!(p.counts(), (2, 2));
 
     p.get();
-    assert_eq!(p.counts(), (2, 3));
+    assert_eq!(p.counts(), (3, 2));
 
     // Check the final state.
     assert_eq!(*p, Point {x: 3, y: 0});