From 31e46ac0a95d58fa95dcd154a0f8a56089e2f5b9 Mon Sep 17 00:00:00 2001
From: Niko Matsakis <niko@alum.mit.edu>
Date: Sat, 6 Dec 2014 11:55:38 -0800
Subject: [PATCH] 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.)
---
src/librustc_typeck/check/method/mod.rs | 1 +
src/librustc_typeck/check/method/probe.rs | 39 +++++++++-----
...owck-return-variable-on-stack-via-clone.rs | 20 +++++++
...thod-mut-self-modifies-mut-slice-lvalue.rs | 54 +++++++++++++++++++
4 files changed, 102 insertions(+), 12 deletions(-)
create mode 100644 src/test/compile-fail/borrowck-return-variable-on-stack-via-clone.rs
create mode 100644 src/test/run-pass/method-mut-self-modifies-mut-slice-lvalue.rs
diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs
index b6c9d8b2d21..53bda93b28e 100644
--- a/src/librustc_typeck/check/method/mod.rs
+++ b/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))
}
diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs
index 6ff276edbce..18ab4f79b0b 100644
--- a/src/librustc_typeck/check/method/probe.rs
+++ b/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,
diff --git a/src/test/compile-fail/borrowck-return-variable-on-stack-via-clone.rs b/src/test/compile-fail/borrowck-return-variable-on-stack-via-clone.rs
new file mode 100644
index 00000000000..7529943f0bc
--- /dev/null
+++ b/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() { }
diff --git a/src/test/run-pass/method-mut-self-modifies-mut-slice-lvalue.rs b/src/test/run-pass/method-mut-self-modifies-mut-slice-lvalue.rs
new file mode 100644
index 00000000000..00319d57f8d
--- /dev/null
+++ b/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]);
+}