During method resolution, only reborrow if we are not doing an auto-ref.

The current behavior leads to adjustments like `&&*` being applied
instead of just `&` (when the unmodified receiver is a `&T` or an `&mut
T`). This causes both safety errors and unexpected behavior. The safety
errors result from regionck not being prepared for auto-ref-ref-like
adjustments; this is worth fixing on its own, but I think the best way
to do it is to modify regionck to use expr-use-visitor (and fix
expr-use-visitor as well, which I don't think properly invokes `borrow`
for each level of auto-ref), and for now it's simpler to just not
produce the adjustment in question. (I have a separate patch porting
regionck to use exprusevisitor for a different bug, so that is coming.)

Author: nikomatsakis

src/librustc_typeck/check/method/mod.rs

@@ -100,6 +100,7 @@ pub fn lookup<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
            call_expr.repr(fcx.tcx()),
            self_expr.repr(fcx.tcx()));
 
+    let self_ty = fcx.infcx().resolve_type_vars_if_possible(self_ty);
     let pick = try!(probe::probe(fcx, span, method_name, self_ty, call_expr.id));
     Ok(confirm::confirm(fcx, span, self_expr, call_expr, self_ty, pick, supplied_method_types))
 }

src/librustc_typeck/check/method/probe.rs

@@ -168,7 +168,7 @@ fn create_steps<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
         check::autoderef(
             fcx, span, self_ty, None, NoPreference,
             |t, d| {
-                let adjustment = consider_reborrow(t, d);
+                let adjustment = AutoDeref(d);
                 steps.push(CandidateStep { self_ty: t, adjustment: adjustment });
                 None::<()> // keep iterating until we can't anymore
             });
@@ -185,14 +185,6 @@ fn create_steps<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
     }
 
     return steps;
-
-    fn consider_reborrow(ty: Ty, d: uint) -> PickAdjustment {
-        // Insert a `&*` or `&mut *` if this is a reference type:
-        match ty.sty {
-            ty::ty_rptr(_, ref mt) => AutoRef(mt.mutbl, box AutoDeref(d+1)),
-            _ => AutoDeref(d),
-        }
-    }
 }
 
 impl<'a,'tcx> ProbeContext<'a,'tcx> {
@@ -626,7 +618,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             return None;
         }
 
-        match self.pick_adjusted_method(step) {
+        match self.pick_by_value_method(step) {
             Some(result) => return Some(result),
             None => {}
         }
@@ -644,11 +636,34 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
         }
     }
 
-    fn pick_adjusted_method(&mut self,
+    fn pick_by_value_method(&mut self,
                             step: &CandidateStep<'tcx>)
                             -> Option<PickResult<'tcx>>
     {
-        self.pick_method(step.self_ty).map(|r| self.adjust(r, step.adjustment.clone()))
+        /*!
+         * For each type `T` in the step list, this attempts to find a
+         * method where the (transformed) self type is exactly `T`. We
+         * do however do one transformation on the adjustment: if we
+         * are passing a region pointer in, we will potentially
+         * *reborrow* it to a shorter lifetime. This allows us to
+         * transparently pass `&mut` pointers, in particular, without
+         * consuming them for their entire lifetime.
+         */
+
+        let adjustment = match step.adjustment {
+            AutoDeref(d) => consider_reborrow(step.self_ty, d),
+            AutoUnsizeLength(..) | AutoRef(..) => step.adjustment.clone(),
+        };
+
+        return self.pick_method(step.self_ty).map(|r| self.adjust(r, adjustment.clone()));
+
+        fn consider_reborrow(ty: Ty, d: uint) -> PickAdjustment {
+            // Insert a `&*` or `&mut *` if this is a reference type:
+            match ty.sty {
+                ty::ty_rptr(_, ref mt) => AutoRef(mt.mutbl, box AutoDeref(d+1)),
+                _ => AutoDeref(d),
+            }
+        }
     }
 
     fn pick_autorefd_method(&mut self,

src/test/compile-fail/borrowck-return-variable-on-stack-via-clone.rs

@@ -0,0 +1,20 @@
+// 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.
+
+// Check that when we clone a `&T` pointer we properly relate the
+// lifetime of the pointer which results to the pointer being cloned.
+// Bugs in method resolution have sometimes broken this connection.
+// Issue #19261.
+
+fn leak<'a, T>(x: T) -> &'a T {
+    (&x).clone() //~ ERROR `x` does not live long enough
+}
+
+fn main() { }

src/test/run-pass/method-mut-self-modifies-mut-slice-lvalue.rs

@@ -0,0 +1,54 @@
+// 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.
+
+// Test that an `&mut self` method, when invoked on an lvalue whose
+// type is `&mut [u8]`, passes in a pointer to the lvalue and not a
+// temporary. Issue #19147.
+
+use std::raw;
+use std::mem;
+use std::slice;
+use std::io::IoResult;
+
+trait MyWriter {
+    fn my_write(&mut self, buf: &[u8]) -> IoResult<()>;
+}
+
+impl<'a> MyWriter for &'a mut [u8] {
+    fn my_write(&mut self, buf: &[u8]) -> IoResult<()> {
+        slice::bytes::copy_memory(*self, buf);
+
+        let write_len = buf.len();
+        unsafe {
+            *self = mem::transmute(raw::Slice {
+                data: self.as_ptr().offset(write_len as int),
+                len: self.len() - write_len,
+            });
+        }
+
+        Ok(())
+    }
+}
+
+fn main() {
+    let mut buf = [0_u8, .. 6];
+
+    {
+        let mut writer = buf.as_mut_slice();
+        writer.my_write(&[0, 1, 2]).unwrap();
+        writer.my_write(&[3, 4, 5]).unwrap();
+    }
+
+    // If `my_write` is not modifying `buf` in place, then we will
+    // wind up with `[3, 4, 5, 0, 0, 0]` because the first call to
+    // `my_write()` doesn't update the starting point for the write.
+
+    assert_eq!(buf, [0, 1, 2, 3, 4, 5]);
+}