From 5ab33a297521c2d5885422bc1744f6d9dab8f3f7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 1 May 2013 08:49:48 -0400 Subject: [PATCH] correct incorrect handling of overloaded operators, exposing various other bits of rot --- src/libcore/ptr.rs | 16 +-- .../middle/borrowck/gather_loans/mod.rs | 63 +++--------- src/librustc/middle/typeck/check/regionck.rs | 9 +- src/libstd/arc.rs | 8 +- src/libstd/rope.rs | 28 +++--- src/libstd/sort.rs | 97 ++++++++++--------- src/libstd/std.rc | 7 ++ .../borrowck-loan-in-overloaded-op.rs | 10 +- src/test/run-pass/auto-ref-slice-plus-ref.rs | 8 +- 9 files changed, 117 insertions(+), 129 deletions(-) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 86b36834bbd..85e46a0feff 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -296,34 +296,34 @@ impl Ord for *const T { // Equality for region pointers #[cfg(notest)] -impl<'self,T:Eq> Eq for &'self const T { +impl<'self,T:Eq> Eq for &'self T { #[inline(always)] - fn eq(&self, other: & &'self const T) -> bool { + fn eq(&self, other: & &'self T) -> bool { return *(*self) == *(*other); } #[inline(always)] - fn ne(&self, other: & &'self const T) -> bool { + fn ne(&self, other: & &'self T) -> bool { return *(*self) != *(*other); } } // Comparison for region pointers #[cfg(notest)] -impl<'self,T:Ord> Ord for &'self const T { +impl<'self,T:Ord> Ord for &'self T { #[inline(always)] - fn lt(&self, other: & &'self const T) -> bool { + fn lt(&self, other: & &'self T) -> bool { *(*self) < *(*other) } #[inline(always)] - fn le(&self, other: & &'self const T) -> bool { + fn le(&self, other: & &'self T) -> bool { *(*self) <= *(*other) } #[inline(always)] - fn ge(&self, other: & &'self const T) -> bool { + fn ge(&self, other: & &'self T) -> bool { *(*self) >= *(*other) } #[inline(always)] - fn gt(&self, other: & &'self const T) -> bool { + fn gt(&self, other: & &'self T) -> bool { *(*self) > *(*other) } } diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index 8a986a22d4c..ecdf260bfff 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -68,8 +68,7 @@ struct GatherLoanCtxt { id_range: id_range, all_loans: @mut ~[Loan], item_ub: ast::node_id, - repeating_ids: ~[ast::node_id], - ignore_adjustments: HashSet + repeating_ids: ~[ast::node_id] } pub fn gather_loans(bccx: @BorrowckCtxt, @@ -79,8 +78,7 @@ pub fn gather_loans(bccx: @BorrowckCtxt, id_range: id_range::max(), all_loans: @mut ~[], item_ub: body.node.id, - repeating_ids: ~[body.node.id], - ignore_adjustments: HashSet::new() + repeating_ids: ~[body.node.id] }; let v = visit::mk_vt(@visit::Visitor {visit_expr: gather_loans_in_expr, visit_block: gather_loans_in_block, @@ -147,13 +145,8 @@ fn gather_loans_in_expr(ex: @ast::expr, self.id_range.add(ex.callee_id); // If this expression is borrowed, have to ensure it remains valid: - { - let this = &mut *self; // FIXME(#5074) - if !this.ignore_adjustments.contains(&ex.id) { - for tcx.adjustments.find(&ex.id).each |&adjustments| { - this.guarantee_adjustments(ex, *adjustments); - } - } + for tcx.adjustments.find(&ex.id).each |&adjustments| { + self.guarantee_adjustments(ex, *adjustments); } // Special checks for various kinds of expressions: @@ -178,46 +171,20 @@ fn gather_loans_in_expr(ex: @ast::expr, visit::visit_expr(ex, self, vt); } - ast::expr_index(rcvr, _) | - ast::expr_binary(_, rcvr, _) | - ast::expr_unary(_, rcvr) | - ast::expr_assign_op(_, rcvr, _) + ast::expr_index(_, arg) | + ast::expr_binary(_, _, arg) if self.bccx.method_map.contains_key(&ex.id) => { - // Receivers in method calls are always passed by ref. - // - // Here, in an overloaded operator, the call is this expression, - // and hence the scope of the borrow is this call. - // - // FIX? / NOT REALLY---technically we should check the other - // argument and consider the argument mode. But how annoying. - // And this problem when goes away when argument modes are - // phased out. So I elect to leave this undone. - let scope_r = ty::re_scope(ex.id); - let rcvr_cmt = self.bccx.cat_expr(rcvr); - self.guarantee_valid(rcvr.id, rcvr.span, rcvr_cmt, m_imm, scope_r); - - // FIXME (#3387): Total hack: Ignore adjustments for the left-hand - // side. Their regions will be inferred to be too large. - self.ignore_adjustments.insert(rcvr.id); - - visit::visit_expr(ex, self, vt); + // Arguments in method calls are always passed by ref. + // + // Currently these do not use adjustments, so we have to + // hardcode this check here (note that the receiver DOES use + // adjustments). + let scope_r = ty::re_scope(ex.id); + let arg_cmt = self.bccx.cat_expr(arg); + self.guarantee_valid(arg.id, arg.span, arg_cmt, m_imm, scope_r); + visit::visit_expr(ex, self, vt); } - // FIXME--#3387 - // ast::expr_binary(_, lhs, rhs) => { - // // Universal comparison operators like ==, >=, etc - // // take their arguments by reference. - // let lhs_ty = ty::expr_ty(self.tcx(), lhs); - // if !ty::type_is_scalar(lhs_ty) { - // let scope_r = ty::re_scope(ex.id); - // let lhs_cmt = self.bccx.cat_expr(lhs); - // self.guarantee_valid(lhs_cmt, m_imm, scope_r); - // let rhs_cmt = self.bccx.cat_expr(rhs); - // self.guarantee_valid(rhs_cmt, m_imm, scope_r); - // } - // visit::visit_expr(ex, self, vt); - // } - // see explanation attached to the `root_ub` field: ast::expr_while(cond, ref body) => { // during the condition, can only root for the condition diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index fd19738b321..1b0a22752a5 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -236,9 +236,16 @@ fn visit_expr(expr: @ast::expr, rcx: @mut Rcx, v: rvt) { // overloaded. See #3511. let tcx = rcx.fcx.tcx(); match expr.node { + // You'd think that x += y where `+=` is overloaded would be a + // cleanup scope. You'd be... kind of right. In fact the + // handling of `+=` and friends in trans for overloaded + // operators is a hopeless mess and I can't figure out how to + // represent it. - ndm + // + // ast::expr_assign_op(*) | + ast::expr_index(*) | ast::expr_binary(*) | - ast::expr_assign_op(*) | ast::expr_unary(*) if has_method_map => { tcx.region_maps.record_cleanup_scope(expr.id); } diff --git a/src/libstd/arc.rs b/src/libstd/arc.rs index 027bf93b481..98d7a01b928 100644 --- a/src/libstd/arc.rs +++ b/src/libstd/arc.rs @@ -598,8 +598,8 @@ mod tests { let arc = ~RWARC(1); let arc2 = (*arc).clone(); do task::try || { - do arc2.write_downgrade |write_mode| { - do (&write_mode).write |one| { + do arc2.write_downgrade |mut write_mode| { + do write_mode.write |one| { assert!(*one == 2); } } @@ -733,8 +733,8 @@ mod tests { } // Downgrader (us) - do arc.write_downgrade |write_mode| { - do (&write_mode).write_cond |state, cond| { + do arc.write_downgrade |mut write_mode| { + do write_mode.write_cond |state, cond| { wc1.send(()); // send to another writer who will wake us up while *state == 0 { cond.wait(); diff --git a/src/libstd/rope.rs b/src/libstd/rope.rs index 1292d2a8585..93364f8a319 100644 --- a/src/libstd/rope.rs +++ b/src/libstd/rope.rs @@ -1256,22 +1256,24 @@ mod tests { match (r) { node::Empty => return ~"", node::Content(x) => { - let str = @mut ~""; - fn aux(str: @mut ~str, node: @node::Node) { + let mut str = ~""; + fn aux(str: &mut ~str, node: @node::Node) { match (*node) { - node::Leaf(x) => { - *str += str::slice( - *x.content, x.byte_offset, - x.byte_offset + x.byte_len).to_owned(); - } - node::Concat(ref x) => { - aux(str, x.left); - aux(str, x.right); - } + node::Leaf(x) => { + str::push_str( + str, + str::slice( + *x.content, x.byte_offset, + x.byte_offset + x.byte_len)); + } + node::Concat(ref x) => { + aux(str, x.left); + aux(str, x.right); + } } } - aux(str, x); - return *str + aux(&mut str, x); + return str } } } diff --git a/src/libstd/sort.rs b/src/libstd/sort.rs index f3d30ecd5cd..119b47c904e 100644 --- a/src/libstd/sort.rs +++ b/src/libstd/sort.rs @@ -22,12 +22,12 @@ type Le<'self, T> = &'self fn(v1: &T, v2: &T) -> bool; * Has worst case O(n log n) performance, best case O(n), but * is not space efficient. This is a stable sort. */ -pub fn merge_sort(v: &const [T], le: Le) -> ~[T] { +pub fn merge_sort(v: &[T], le: Le) -> ~[T] { type Slice = (uint, uint); return merge_sort_(v, (0u, len(v)), le); - fn merge_sort_(v: &const [T], slice: Slice, le: Le) + fn merge_sort_(v: &[T], slice: Slice, le: Le) -> ~[T] { let begin = slice.first(); let end = slice.second(); @@ -179,7 +179,7 @@ fn qsort3(arr: &mut [T], left: int, right: int) { */ pub fn quick_sort3(arr: &mut [T]) { if arr.len() <= 1 { return; } - let len = arr.len() - 1; // FIXME(#5074) nested calls + let len = arr.len(); // FIXME(#5074) nested calls qsort3(arr, 0, (len - 1) as int); } @@ -263,7 +263,7 @@ fn binarysort(array: &mut [T], start: uint) { assert!(left == right); let n = start-left; - copy_vec(array, left+1, array, left, n); + shift_vec(array, left+1, left, n); array[left] = pivot; start += 1; } @@ -309,8 +309,8 @@ fn count_run_ascending(array: &mut [T]) -> uint { return run; } -fn gallop_left(key: &const T, - array: &const [T], +fn gallop_left(key: &T, + array: &[T], hint: uint) -> uint { let size = array.len(); @@ -360,8 +360,8 @@ fn gallop_left(key: &const T, return ofs; } -fn gallop_right(key: &const T, - array: &const [T], +fn gallop_right(key: &T, + array: &[T], hint: uint) -> uint { let size = array.len(); @@ -457,15 +457,15 @@ impl MergeState { } let k = { // constrain lifetime of slice below - let slice = vec::mut_slice(array, b1, b1+l1); - gallop_right(&const array[b2], slice, 0) + let slice = vec::slice(array, b1, b1+l1); + gallop_right(&array[b2], slice, 0) }; b1 += k; l1 -= k; if l1 != 0 { let l2 = { // constrain lifetime of slice below - let slice = vec::mut_slice(array, b2, b2+l2); - gallop_left(&const array[b1+l1-1],slice,l2-1) + let slice = vec::slice(array, b2, b2+l2); + gallop_left(&array[b1+l1-1],slice,l2-1) }; if l2 > 0 { if l1 <= l2 { @@ -497,11 +497,11 @@ impl MergeState { dest += 1; c2 += 1; len2 -= 1; if len2 == 0 { - copy_vec(array, dest, tmp, 0, len1); + copy_vec(array, dest, tmp.slice(0, len1)); return; } if len1 == 1 { - copy_vec(array, dest, array, c2, len2); + shift_vec(array, dest, c2, len2); array[dest+len2] <-> tmp[c1]; return; } @@ -539,10 +539,12 @@ impl MergeState { loop { assert!(len1 > 1 && len2 != 0); - let tmp_view = vec::const_slice(tmp, c1, c1+len1); - count1 = gallop_right(&const array[c2], tmp_view, 0); + count1 = { + let tmp_view = vec::slice(tmp, c1, c1+len1); + gallop_right(&array[c2], tmp_view, 0) + }; if count1 != 0 { - copy_vec(array, dest, tmp, c1, count1); + copy_vec(array, dest, tmp.slice(c1, c1+count1)); dest += count1; c1 += count1; len1 -= count1; if len1 <= 1 { break_outer = true; break; } } @@ -550,10 +552,12 @@ impl MergeState { dest += 1; c2 += 1; len2 -= 1; if len2 == 0 { break_outer = true; break; } - let tmp_view = vec::const_slice(array, c2, c2+len2); - count2 = gallop_left(&const tmp[c1], tmp_view, 0); + count2 = { + let tmp_view = vec::slice(array, c2, c2+len2); + gallop_left(&tmp[c1], tmp_view, 0) + }; if count2 != 0 { - copy_vec(array, dest, array, c2, count2); + shift_vec(array, dest, c2, count2); dest += count2; c2 += count2; len2 -= count2; if len2 == 0 { break_outer = true; break; } } @@ -573,14 +577,14 @@ impl MergeState { if len1 == 1 { assert!(len2 > 0); - copy_vec(array, dest, array, c2, len2); + shift_vec(array, dest, c2, len2); array[dest+len2] <-> tmp[c1]; } else if len1 == 0 { fail!(~"Comparison violates its contract!"); } else { assert!(len2 == 0); assert!(len1 > 1); - copy_vec(array, dest, tmp, c1, len1); + copy_vec(array, dest, tmp.slice(c1, c1+len1)); } } @@ -603,13 +607,13 @@ impl MergeState { dest -= 1; c1 -= 1; len1 -= 1; if len1 == 0 { - copy_vec(array, dest-(len2-1), tmp, 0, len2); + copy_vec(array, dest-(len2-1), tmp.slice(0, len2)); return; } if len2 == 1 { dest -= len1; c1 -= len1; - copy_vec(array, dest+1, array, c1+1, len1); + shift_vec(array, dest+1, c1+1, len1); array[dest] <-> tmp[c2]; return; } @@ -650,12 +654,12 @@ impl MergeState { { // constrain scope of tmp_view: let tmp_view = vec::mut_slice (array, base1, base1+len1); count1 = len1 - gallop_right( - &const tmp[c2], tmp_view, len1-1); + &tmp[c2], tmp_view, len1-1); } if count1 != 0 { dest -= count1; c1 -= count1; len1 -= count1; - copy_vec(array, dest+1, array, c1+1, count1); + shift_vec(array, dest+1, c1+1, count1); if len1 == 0 { break_outer = true; break; } } @@ -666,14 +670,14 @@ impl MergeState { let count2; { // constrain scope of tmp_view let tmp_view = vec::mut_slice(tmp, 0, len2); - count2 = len2 - gallop_left(&const array[c1], + count2 = len2 - gallop_left(&array[c1], tmp_view, len2-1); } if count2 != 0 { dest -= count2; c2 -= count2; len2 -= count2; - copy_vec(array, dest+1, tmp, c2+1, count2); + copy_vec(array, dest+1, tmp.slice(c2+1, c2+1+count2)); if len2 <= 1 { break_outer = true; break; } } array[dest] <-> array[c1]; @@ -695,14 +699,14 @@ impl MergeState { assert!(len1 > 0); dest -= len1; c1 -= len1; - copy_vec(array, dest+1, array, c1+1, len1); + shift_vec(array, dest+1, c1+1, len1); array[dest] <-> tmp[c2]; } else if len2 == 0 { fail!(~"Comparison violates its contract!"); } else { assert!(len1 == 0); assert!(len2 != 0); - copy_vec(array, dest-(len2-1), tmp, 0, len2); + copy_vec(array, dest-(len2-1), tmp.slice(0, len2)); } } @@ -738,21 +742,25 @@ impl MergeState { #[inline(always)] fn copy_vec(dest: &mut [T], s1: uint, - from: &const [T], - s2: uint, - len: uint) { - assert!(s1+len <= dest.len() && s2+len <= from.len()); + from: &[T]) { + assert!(s1+from.len() <= dest.len()); - let mut slice = ~[]; - for uint::range(s2, s2+len) |i| { - slice.push(from[i]); - } - - for slice.eachi |i, v| { + for from.eachi |i, v| { dest[s1+i] = *v; } } +#[inline(always)] +fn shift_vec(dest: &mut [T], + s1: uint, + s2: uint, + len: uint) { + assert!(s1+len <= dest.len()); + + let tmp = dest.slice(s2, s2+len).to_vec(); + copy_vec(dest, s1, tmp); +} + #[cfg(test)] mod test_qsort3 { use sort::*; @@ -764,8 +772,7 @@ mod test_qsort3 { quick_sort3::(v1); let mut i = 0; while i < len { - // debug!(v2[i]); - assert!((v2[i] == v1[i])); + assert_eq!(v2[i], v1[i]); i += 1; } } @@ -1036,7 +1043,7 @@ mod big_tests { tabulate_managed(low, high); } - fn multiplyVec(arr: &const [T], num: uint) -> ~[T] { + fn multiplyVec(arr: &[T], num: uint) -> ~[T] { let size = arr.len(); let res = do vec::from_fn(num) |i| { arr[i % size] @@ -1052,7 +1059,7 @@ mod big_tests { } fn tabulate_unique(lo: uint, hi: uint) { - fn isSorted(arr: &const [T]) { + fn isSorted(arr: &[T]) { for uint::range(0, arr.len()-1) |i| { if arr[i] > arr[i+1] { fail!(~"Array not sorted"); @@ -1123,7 +1130,7 @@ mod big_tests { } fn tabulate_managed(lo: uint, hi: uint) { - fn isSorted(arr: &const [@T]) { + fn isSorted(arr: &[@T]) { for uint::range(0, arr.len()-1) |i| { if arr[i] > arr[i+1] { fail!(~"Array not sorted"); diff --git a/src/libstd/std.rc b/src/libstd/std.rc index d9af8b111bb..9d1ddb8ec54 100644 --- a/src/libstd/std.rc +++ b/src/libstd/std.rc @@ -69,7 +69,14 @@ pub mod list; pub mod priority_queue; pub mod rope; pub mod smallintmap; + +#[cfg(stage0)] +#[path="sort_stage0.rs"] pub mod sort; + +#[cfg(not(stage0))] +pub mod sort; + pub mod dlist; pub mod treemap; diff --git a/src/test/compile-fail/borrowck-loan-in-overloaded-op.rs b/src/test/compile-fail/borrowck-loan-in-overloaded-op.rs index 482d1b6b8b6..0361213af22 100644 --- a/src/test/compile-fail/borrowck-loan-in-overloaded-op.rs +++ b/src/test/compile-fail/borrowck-loan-in-overloaded-op.rs @@ -8,18 +8,16 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// xfail-test #3387 - struct foo(~uint); impl Add for foo { - fn add(f: &foo) -> foo { - foo(~(**self + **(*f))) + fn add(&self, f: &foo) -> foo { + foo(~(***self + **(*f))) } } fn main() { let x = foo(~3); - let _y = x + x; - //~^ ERROR moving out of immutable local variable prohibited due to outstanding loan + let _y = x + {x}; // the `{x}` forces a move to occur + //~^ ERROR cannot move out of `x` } diff --git a/src/test/run-pass/auto-ref-slice-plus-ref.rs b/src/test/run-pass/auto-ref-slice-plus-ref.rs index 1dc56132875..07629651e56 100644 --- a/src/test/run-pass/auto-ref-slice-plus-ref.rs +++ b/src/test/run-pass/auto-ref-slice-plus-ref.rs @@ -17,13 +17,13 @@ trait MyIter { } impl<'self> MyIter for &'self [int] { - fn test_imm(&self) { assert!(self[0] == 1) } - fn test_const(&const self) { assert!(self[0] == 1) } + fn test_imm(&self) { assert_eq!(self[0], 1) } + fn test_const(&const self) { assert_eq!(self[0], 1) } } impl<'self> MyIter for &'self str { - fn test_imm(&self) { assert!(*self == "test") } - fn test_const(&const self) { assert!(*self == "test") } + fn test_imm(&self) { assert_eq!(*self, "test") } + fn test_const(&const self) { assert_eq!(self[0], 't') } } pub fn main() {