From d2d8fa2a0980fc6bf1a842cfff7d77ae9b95185f Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 1/9] Make analyze_move_out_from use a loop rather than recursion It will be simpler to make some of the changes that I need to make to analyze_move_out if it uses a loop rather than recursion. --- src/librustc/middle/borrowck/check_loans.rs | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index ece8d973236a6..8ec93bb51b6be 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -874,23 +874,30 @@ impl<'a> CheckLoanCtxt<'a> { // We must check every element of a move path. See // `borrowck-move-subcomponent.rs` for a test case. - // check for a conflicting loan: let mut ret = MoveOk; - self.each_in_scope_restriction(expr_id, move_path, |loan, _| { - // Any restriction prevents moves. - ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); - false - }); + let mut loan_path = move_path; + loop { + // check for a conflicting loan: + self.each_in_scope_restriction(expr_id, loan_path, |loan, _| { + // Any restriction prevents moves. + ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + false + }); - if ret != MoveOk { - return ret - } + if ret != MoveOk { + return ret + } - match *move_path { - LpVar(_) => MoveOk, - LpExtend(ref subpath, _, _) => { - self.analyze_move_out_from(expr_id, &**subpath) + match *loan_path { + LpVar(_) => { + ret = MoveOk; + break; + } + LpExtend(ref lp_base, _, _) => { + loan_path = &**lp_base; + } } } + ret } } From 8c0e1ce6c9410f4d928923ee93e7b5a4674ae2ed Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 2/9] Make check_for_move_of_borrowed_path take an &LoanPath rather than an &Rc It doesn't actually need the Rc, and it reduces the net number of pointer manipulations. --- src/librustc/middle/borrowck/check_loans.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 8ec93bb51b6be..45d15a9afac1e 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -454,7 +454,7 @@ impl<'a> CheckLoanCtxt<'a> { } Some(move_kind) => { self.check_for_move_of_borrowed_path(id, span, - &lp, move_kind); + &*lp, move_kind); if move_kind == move_data::Captured { MovedInCapture } else { @@ -474,20 +474,20 @@ impl<'a> CheckLoanCtxt<'a> { fn check_for_move_of_borrowed_path(&self, id: ast::NodeId, span: Span, - move_path: &Rc, + move_path: &LoanPath, move_kind: move_data::MoveKind) { - match self.analyze_move_out_from(id, &**move_path) { + match self.analyze_move_out_from(id, move_path) { MoveOk => { } MoveWhileBorrowed(loan_path, loan_span) => { let err_message = match move_kind { move_data::Captured => format!("cannot move `{}` into closure because it is borrowed", - self.bccx.loan_path_to_str(&**move_path).as_slice()), + self.bccx.loan_path_to_str(move_path).as_slice()), move_data::Declared | move_data::MoveExpr | move_data::MovePat => format!("cannot move out of `{}` because it is borrowed", - self.bccx.loan_path_to_str(&**move_path).as_slice()) + self.bccx.loan_path_to_str(move_path).as_slice()) }; self.bccx.span_err(span, err_message.as_slice()); From 45a1b977643456ceacfc81183e2d5743f7c61ce9 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 3/9] Make analyze_move_out_from more field-sensitive Currently analyze_move_out_from checks all restrictions on all base paths of the move path, but it only needs to check restrictions from loans of the base paths, and can disregard restrictions from loans of extensions of those base paths. --- src/librustc/middle/borrowck/check_loans.rs | 48 ++++++++++++++----- .../borrowck-field-sensitivity.rs | 17 ------- .../run-pass/borrowck-field-sensitivity.rs | 16 +++++++ 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 45d15a9afac1e..ab004e4a3e934 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -871,26 +871,47 @@ impl<'a> CheckLoanCtxt<'a> { self.tcx().map.node_to_str(expr_id), move_path.repr(self.tcx())); - // We must check every element of a move path. See - // `borrowck-move-subcomponent.rs` for a test case. - let mut ret = MoveOk; + + // First, we check for a restriction on the path P being used. This + // accounts for borrows of P but also borrows of subpaths, like P.a.b. + // Consider the following example: + // + // let x = &mut a.b.c; // Restricts a, a.b, and a.b.c + // let y = a; // Conflicts with restriction + + self.each_in_scope_restriction(expr_id, move_path, |loan, _restr| { + // Any restriction prevents moves. + ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + false + }); + + // Next, we must check for *loans* (not restrictions) on the path P or + // any base path. This rejects examples like the following: + // + // let x = &mut a.b; + // let y = a.b.c; + // + // Limiting this search to *loans* and not *restrictions* means that + // examples like the following continue to work: + // + // let x = &mut a.b; + // let y = a.c; + let mut loan_path = move_path; loop { - // check for a conflicting loan: - self.each_in_scope_restriction(expr_id, loan_path, |loan, _| { + self.each_in_scope_loan(expr_id, |loan| { // Any restriction prevents moves. - ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); - false + if *loan.loan_path == *loan_path { + ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + false + } else { + true + } }); - if ret != MoveOk { - return ret - } - match *loan_path { LpVar(_) => { - ret = MoveOk; break; } LpExtend(ref lp_base, _, _) => { @@ -898,6 +919,7 @@ impl<'a> CheckLoanCtxt<'a> { } } } - ret + + return ret; } } diff --git a/src/test/compile-fail/borrowck-field-sensitivity.rs b/src/test/compile-fail/borrowck-field-sensitivity.rs index 2fa9067af549a..72042b8373d84 100644 --- a/src/test/compile-fail/borrowck-field-sensitivity.rs +++ b/src/test/compile-fail/borrowck-field-sensitivity.rs @@ -84,20 +84,6 @@ fn fu_move_after_fu_move() { // The following functions aren't yet accepted, but they should be. -fn move_after_borrow_correct() { - let x = A { a: 1, b: box 2 }; - let p = &x.a; - drop(x.b); //~ ERROR cannot move out of `x.b` because it is borrowed - drop(*p); -} - -fn fu_move_after_borrow_correct() { - let x = A { a: 1, b: box 2 }; - let p = &x.a; - let _y = A { a: 3, .. x }; //~ ERROR cannot move out of `x.b` because it is borrowed - drop(*p); -} - fn copy_after_field_assign_after_uninit() { let mut x: A; x.a = 1; @@ -132,9 +118,6 @@ fn main() { fu_move_after_move(); fu_move_after_fu_move(); - move_after_borrow_correct(); - fu_move_after_borrow_correct(); - copy_after_field_assign_after_uninit(); borrow_after_field_assign_after_uninit(); move_after_field_assign_after_uninit(); diff --git a/src/test/run-pass/borrowck-field-sensitivity.rs b/src/test/run-pass/borrowck-field-sensitivity.rs index a297300daf1b2..33be47e504be2 100644 --- a/src/test/run-pass/borrowck-field-sensitivity.rs +++ b/src/test/run-pass/borrowck-field-sensitivity.rs @@ -73,6 +73,20 @@ fn borrow_after_fu_move() { drop(*p); } +fn move_after_borrow() { + let x = A { a: 1, b: box 2 }; + let p = &x.a; + drop(x.b); + drop(*p); +} + +fn fu_move_after_borrow() { + let x = A { a: 1, b: box 2 }; + let p = &x.a; + let _y = A { a: 3, .. x }; + drop(*p); +} + fn mut_borrow_after_mut_borrow() { let mut x = A { a: 1, b: box 2 }; let p = &mut x.a; @@ -225,6 +239,8 @@ fn main() { borrow_after_move(); borrow_after_fu_move(); + move_after_borrow(); + fu_move_after_borrow(); mut_borrow_after_mut_borrow(); move_after_move(); From 24b1b79cf1d2a27c3f8d1c518fcfa7a1ec832154 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 4/9] Make analyze_move_out_from take a BorrowKind Currently analyze_move_out_from ignores the BorrowKind of loans, but the same logic is useful when restricted to loans of specific borrow kinds. --- src/librustc/middle/borrowck/check_loans.rs | 27 +++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index ab004e4a3e934..d38107cd5fe89 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -476,7 +476,10 @@ impl<'a> CheckLoanCtxt<'a> { span: Span, move_path: &LoanPath, move_kind: move_data::MoveKind) { - match self.analyze_move_out_from(id, move_path) { + // We want to detect if there are any loans at all, so we search for + // any loans incompatible with MutBorrrow, since all other kinds of + // loans are incompatible with that. + match self.analyze_move_out_from(id, move_path, ty::MutBorrow) { MoveOk => { } MoveWhileBorrowed(loan_path, loan_span) => { let err_message = match move_kind { @@ -865,7 +868,8 @@ impl<'a> CheckLoanCtxt<'a> { pub fn analyze_move_out_from(&self, expr_id: ast::NodeId, - move_path: &LoanPath) + move_path: &LoanPath, + borrow_kind: ty::BorrowKind) -> MoveError { debug!("analyze_move_out_from(expr_id={:?}, move_path={})", self.tcx().map.node_to_str(expr_id), @@ -881,9 +885,12 @@ impl<'a> CheckLoanCtxt<'a> { // let y = a; // Conflicts with restriction self.each_in_scope_restriction(expr_id, move_path, |loan, _restr| { - // Any restriction prevents moves. - ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); - false + if incompatible(loan.kind, borrow_kind) { + ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + false + } else { + true + } }); // Next, we must check for *loans* (not restrictions) on the path P or @@ -901,8 +908,8 @@ impl<'a> CheckLoanCtxt<'a> { let mut loan_path = move_path; loop { self.each_in_scope_loan(expr_id, |loan| { - // Any restriction prevents moves. - if *loan.loan_path == *loan_path { + if *loan.loan_path == *loan_path && + incompatible(loan.kind, borrow_kind) { ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); false } else { @@ -921,5 +928,11 @@ impl<'a> CheckLoanCtxt<'a> { } return ret; + + fn incompatible(borrow_kind1: ty::BorrowKind, + borrow_kind2: ty::BorrowKind) + -> bool { + borrow_kind1 != ty::ImmBorrow || borrow_kind2 != ty::ImmBorrow + } } } From 036833ece95aa5fc9a1110c5691488193138eb8f Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 5/9] Rename analyze_move_out_from to analyze_restrictions_on_use Also rename MoveError to UseError and MoveOk / MoveWhileBorrowed to UseOk / UseWhileBorrowed. --- src/librustc/middle/borrowck/check_loans.rs | 36 ++++++++++----------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index d38107cd5fe89..2015572133f88 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -150,9 +150,9 @@ pub fn check_loans(bccx: &BorrowckCtxt, } #[deriving(PartialEq)] -enum MoveError { - MoveOk, - MoveWhileBorrowed(/*loan*/Rc, /*loan*/Span) +enum UseError { + UseOk, + UseWhileBorrowed(/*loan*/Rc, /*loan*/Span) } impl<'a> CheckLoanCtxt<'a> { @@ -479,9 +479,9 @@ impl<'a> CheckLoanCtxt<'a> { // We want to detect if there are any loans at all, so we search for // any loans incompatible with MutBorrrow, since all other kinds of // loans are incompatible with that. - match self.analyze_move_out_from(id, move_path, ty::MutBorrow) { - MoveOk => { } - MoveWhileBorrowed(loan_path, loan_span) => { + match self.analyze_restrictions_on_use(id, move_path, ty::MutBorrow) { + UseOk => { } + UseWhileBorrowed(loan_path, loan_span) => { let err_message = match move_kind { move_data::Captured => format!("cannot move `{}` into closure because it is borrowed", @@ -866,16 +866,16 @@ impl<'a> CheckLoanCtxt<'a> { self.bccx.loan_path_to_str(loan_path)).as_slice()); } - pub fn analyze_move_out_from(&self, - expr_id: ast::NodeId, - move_path: &LoanPath, - borrow_kind: ty::BorrowKind) - -> MoveError { - debug!("analyze_move_out_from(expr_id={:?}, move_path={})", + pub fn analyze_restrictions_on_use(&self, + expr_id: ast::NodeId, + use_path: &LoanPath, + borrow_kind: ty::BorrowKind) + -> UseError { + debug!("analyze_restrictions_on_use(expr_id={:?}, use_path={})", self.tcx().map.node_to_str(expr_id), - move_path.repr(self.tcx())); + use_path.repr(self.tcx())); - let mut ret = MoveOk; + let mut ret = UseOk; // First, we check for a restriction on the path P being used. This // accounts for borrows of P but also borrows of subpaths, like P.a.b. @@ -884,9 +884,9 @@ impl<'a> CheckLoanCtxt<'a> { // let x = &mut a.b.c; // Restricts a, a.b, and a.b.c // let y = a; // Conflicts with restriction - self.each_in_scope_restriction(expr_id, move_path, |loan, _restr| { + self.each_in_scope_restriction(expr_id, use_path, |loan, _restr| { if incompatible(loan.kind, borrow_kind) { - ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); false } else { true @@ -905,12 +905,12 @@ impl<'a> CheckLoanCtxt<'a> { // let x = &mut a.b; // let y = a.c; - let mut loan_path = move_path; + let mut loan_path = use_path; loop { self.each_in_scope_loan(expr_id, |loan| { if *loan.loan_path == *loan_path && incompatible(loan.kind, borrow_kind) { - ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span); + ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); false } else { true From 159e27aebb940926ccf1bad0b2b12087d36ad903 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 6/9] Fix all violations of stronger guarantees for mutable borrows Fix all violations in the Rust source tree of the stronger guarantee of a unique access path for mutable borrows as described in #12624. --- src/libarena/lib.rs | 3 +- src/libcollections/ringbuf.rs | 3 +- src/libcollections/vec.rs | 11 ++- src/libdebug/repr.rs | 6 +- src/libnative/io/net.rs | 5 +- src/libregex/parse/mod.rs | 9 +- src/librustc/middle/dataflow.rs | 3 +- src/librustc/middle/liveness.rs | 19 ++-- src/librustc/middle/privacy.rs | 3 +- src/libsyntax/ext/format.rs | 3 +- src/libsyntax/parse/attr.rs | 3 +- src/libsyntax/parse/lexer/mod.rs | 82 ++++++++++------ src/libsyntax/parse/parser.rs | 157 ++++++++++++++++++++----------- src/libsyntax/print/pp.rs | 9 +- src/libterm/terminfo/parm.rs | 3 +- 15 files changed, 206 insertions(+), 113 deletions(-) diff --git a/src/libarena/lib.rs b/src/libarena/lib.rs index 996369cbf6d7d..34264aa1b8145 100644 --- a/src/libarena/lib.rs +++ b/src/libarena/lib.rs @@ -406,7 +406,8 @@ impl TypedArenaChunk { None => {} Some(mut next) => { // We assume that the next chunk is completely filled. - next.destroy(next.capacity) + let capacity = next.capacity; + next.destroy(capacity) } } } diff --git a/src/libcollections/ringbuf.rs b/src/libcollections/ringbuf.rs index ae1925126cae7..29edf1db51d70 100644 --- a/src/libcollections/ringbuf.rs +++ b/src/libcollections/ringbuf.rs @@ -66,7 +66,8 @@ impl Deque for RingBuf { /// Return a mutable reference to the last element in the RingBuf fn back_mut<'a>(&'a mut self) -> Option<&'a mut T> { - if self.nelts > 0 { Some(self.get_mut(self.nelts - 1)) } else { None } + let nelts = self.nelts; + if nelts > 0 { Some(self.get_mut(nelts - 1)) } else { None } } /// Remove and return the first element in the RingBuf, or None if it is empty diff --git a/src/libcollections/vec.rs b/src/libcollections/vec.rs index acb7b2c27040f..61e732846a1a3 100644 --- a/src/libcollections/vec.rs +++ b/src/libcollections/vec.rs @@ -114,7 +114,8 @@ impl Vec { unsafe { let mut xs = Vec::with_capacity(length); while xs.len < length { - ptr::write(xs.as_mut_slice().unsafe_mut_ref(xs.len), op(xs.len)); + let len = xs.len; + ptr::write(xs.as_mut_slice().unsafe_mut_ref(len), op(len)); xs.len += 1; } xs @@ -210,7 +211,8 @@ impl Vec { unsafe { let mut xs = Vec::with_capacity(length); while xs.len < length { - ptr::write(xs.as_mut_slice().unsafe_mut_ref(xs.len), + let len = xs.len; + ptr::write(xs.as_mut_slice().unsafe_mut_ref(len), value.clone()); xs.len += 1; } @@ -321,9 +323,10 @@ impl Clone for Vec { let this_slice = self.as_slice(); while vector.len < len { unsafe { + let len = vector.len; ptr::write( - vector.as_mut_slice().unsafe_mut_ref(vector.len), - this_slice.unsafe_ref(vector.len).clone()); + vector.as_mut_slice().unsafe_mut_ref(len), + this_slice.unsafe_ref(len).clone()); } vector.len += 1; } diff --git a/src/libdebug/repr.rs b/src/libdebug/repr.rs index 86b71eb5b8d69..83eb4adfa9759 100644 --- a/src/libdebug/repr.rs +++ b/src/libdebug/repr.rs @@ -127,13 +127,15 @@ impl<'a> ReprVisitor<'a> { #[inline] pub fn get(&mut self, f: |&mut ReprVisitor, &T| -> bool) -> bool { unsafe { - f(self, mem::transmute::<*u8,&T>(self.ptr)) + let ptr = self.ptr; + f(self, mem::transmute::<*u8,&T>(ptr)) } } #[inline] pub fn visit_inner(&mut self, inner: *TyDesc) -> bool { - self.visit_ptr_inner(self.ptr, inner) + let ptr = self.ptr; + self.visit_ptr_inner(ptr, inner) } #[inline] diff --git a/src/libnative/io/net.rs b/src/libnative/io/net.rs index e7effbd6bdbfb..1c33114dc7173 100644 --- a/src/libnative/io/net.rs +++ b/src/libnative/io/net.rs @@ -637,7 +637,7 @@ impl rtio::RtioUdpSocket for UdpSocket { mem::size_of::() as libc::socklen_t; let dolock = || self.lock_nonblocking(); - let doread = |nb| unsafe { + let n = try!(read(fd, self.read_deadline, dolock, |nb| unsafe { let flags = if nb {c::MSG_DONTWAIT} else {0}; libc::recvfrom(fd, buf.as_mut_ptr() as *mut libc::c_void, @@ -645,8 +645,7 @@ impl rtio::RtioUdpSocket for UdpSocket { flags, storagep, &mut addrlen) as libc::c_int - }; - let n = try!(read(fd, self.read_deadline, dolock, doread)); + })); sockaddr_to_addr(&storage, addrlen as uint).and_then(|addr| { Ok((n as uint, addr)) }) diff --git a/src/libregex/parse/mod.rs b/src/libregex/parse/mod.rs index 29c807882ecb5..cedc40df300b6 100644 --- a/src/libregex/parse/mod.rs +++ b/src/libregex/parse/mod.rs @@ -345,18 +345,19 @@ impl<'a> Parser<'a> { } fn push_literal(&mut self, c: char) -> Result<(), Error> { + let flags = self.flags; match c { '.' => { - self.push(Dot(self.flags)) + self.push(Dot(flags)) } '^' => { - self.push(Begin(self.flags)) + self.push(Begin(flags)) } '$' => { - self.push(End(self.flags)) + self.push(End(flags)) } _ => { - self.push(Literal(c, self.flags)) + self.push(Literal(c, flags)) } } Ok(()) diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index bc5fad7f9343a..bc083dac6ac75 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -300,12 +300,13 @@ impl<'a, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, O> { } { + let words_per_id = self.words_per_id; let mut propcx = PropagationContext { dfcx: &mut *self, changed: true }; - let mut temp = Vec::from_elem(self.words_per_id, 0u); + let mut temp = Vec::from_elem(words_per_id, 0u); let mut loop_scopes = Vec::new(); while propcx.changed { diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 69677554421d4..cd876113807a9 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -547,11 +547,13 @@ struct Liveness<'a> { impl<'a> Liveness<'a> { fn new(ir: &'a mut IrMaps<'a>, specials: Specials) -> Liveness<'a> { + let num_live_nodes = ir.num_live_nodes; + let num_vars = ir.num_vars; Liveness { ir: ir, s: specials, - successors: Vec::from_elem(ir.num_live_nodes, invalid_node()), - users: Vec::from_elem(ir.num_live_nodes * ir.num_vars, invalid_users()), + successors: Vec::from_elem(num_live_nodes, invalid_node()), + users: Vec::from_elem(num_live_nodes * num_vars, invalid_users()), loop_scope: Vec::new(), break_ln: NodeMap::new(), cont_ln: NodeMap::new(), @@ -826,8 +828,9 @@ impl<'a> Liveness<'a> { debug!("compute: using id for block, {}", block_to_str(body)); + let exit_ln = self.s.exit_ln; let entry_ln: LiveNode = - self.with_loop_nodes(body.id, self.s.exit_ln, self.s.exit_ln, + self.with_loop_nodes(body.id, exit_ln, exit_ln, |this| this.propagate_through_fn_block(decl, body)); // hack to skip the loop unless debug! is enabled: @@ -847,12 +850,13 @@ impl<'a> Liveness<'a> { -> LiveNode { // the fallthrough exit is only for those cases where we do not // explicitly return: - self.init_from_succ(self.s.fallthrough_ln, self.s.exit_ln); + let s = self.s; + self.init_from_succ(s.fallthrough_ln, s.exit_ln); if blk.expr.is_none() { - self.acc(self.s.fallthrough_ln, self.s.no_ret_var, ACC_READ) + self.acc(s.fallthrough_ln, s.no_ret_var, ACC_READ) } - self.propagate_through_block(blk, self.s.fallthrough_ln) + self.propagate_through_block(blk, s.fallthrough_ln) } fn propagate_through_block(&mut self, blk: &Block, succ: LiveNode) @@ -1036,7 +1040,8 @@ impl<'a> Liveness<'a> { ExprRet(o_e) => { // ignore succ and subst exit_ln: - self.propagate_through_opt_expr(o_e, self.s.exit_ln) + let exit_ln = self.s.exit_ln; + self.propagate_through_opt_expr(o_e, exit_ln) } ExprBreak(opt_label) => { diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index f61854a7dfac6..112ecd66f4cc9 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -1019,9 +1019,10 @@ impl<'a> Visitor<()> for SanePrivacyVisitor<'a> { self.check_sane_privacy(item); } + let in_fn = self.in_fn; let orig_in_fn = replace(&mut self.in_fn, match item.node { ast::ItemMod(..) => false, // modules turn privacy back on - _ => self.in_fn, // otherwise we inherit + _ => in_fn, // otherwise we inherit }); visit::walk_item(self, item, ()); self.in_fn = orig_in_fn; diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index cfce4b1e0fc5e..ca7596925a9cb 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -202,7 +202,8 @@ impl<'a, 'b> Context<'a, 'b> { } parse::CountIsNextParam => { if self.check_positional_ok() { - self.verify_arg_type(Exact(self.next_arg), Unsigned); + let next_arg = self.next_arg; + self.verify_arg_type(Exact(next_arg), Unsigned); self.next_arg += 1; } } diff --git a/src/libsyntax/parse/attr.rs b/src/libsyntax/parse/attr.rs index 112cdb26131d2..c9122e3ceafb5 100644 --- a/src/libsyntax/parse/attr.rs +++ b/src/libsyntax/parse/attr.rs @@ -73,7 +73,8 @@ impl<'a> ParserAttr for Parser<'a> { let style = if self.eat(&token::NOT) { if !permit_inner { - self.span_err(self.span, + let span = self.span; + self.span_err(span, "an inner attribute is not permitted in \ this context"); } diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index 459cb6d31ed0a..f7eac0b323f7e 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -368,7 +368,8 @@ impl<'a> StringReader<'a> { } else { "unterminated block comment" }; - self.fatal_span(start_bpos, self.last_pos, msg); + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, msg); } else if self.curr_is('/') && self.nextch_is('*') { level += 1; self.bump(); @@ -419,7 +420,8 @@ impl<'a> StringReader<'a> { rslt.push_str(exponent.as_slice()); return Some(rslt); } else { - self.err_span(start_bpos, self.last_pos, "scan_exponent: bad fp literal"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "scan_exponent: bad fp literal"); rslt.push_str("1"); // arbitrary placeholder exponent return Some(rslt); } @@ -506,14 +508,16 @@ impl<'a> StringReader<'a> { else { Unsigned(ast::TyU64) }; } if num_str.len() == 0u { - self.err_span(start_bpos, self.last_pos, "no valid digits found for number"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "no valid digits found for number"); num_str = "1".to_string(); } let parsed = match from_str_radix::(num_str.as_slice(), base as uint) { Some(p) => p, None => { - self.err_span(start_bpos, self.last_pos, "int literal is too large"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "int literal is too large"); 1 } }; @@ -546,13 +550,15 @@ impl<'a> StringReader<'a> { if c == '3' && n == '2' { self.bump(); self.bump(); - self.check_float_base(start_bpos, self.last_pos, base); + let last_bpos = self.last_pos; + self.check_float_base(start_bpos, last_bpos, base); return token::LIT_FLOAT(str_to_ident(num_str.as_slice()), ast::TyF32); } else if c == '6' && n == '4' { self.bump(); self.bump(); - self.check_float_base(start_bpos, self.last_pos, base); + let last_bpos = self.last_pos; + self.check_float_base(start_bpos, last_bpos, base); return token::LIT_FLOAT(str_to_ident(num_str.as_slice()), ast::TyF64); /* FIXME (#2252): if this is out of range for either a @@ -562,25 +568,30 @@ impl<'a> StringReader<'a> { self.bump(); self.bump(); self.bump(); - self.check_float_base(start_bpos, self.last_pos, base); + let last_bpos = self.last_pos; + self.check_float_base(start_bpos, last_bpos, base); return token::LIT_FLOAT(str_to_ident(num_str.as_slice()), ast::TyF128); } - self.err_span(start_bpos, self.last_pos, "expected `f32`, `f64` or `f128` suffix"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "expected `f32`, `f64` or `f128` suffix"); } if is_float { - self.check_float_base(start_bpos, self.last_pos, base); + let last_bpos = self.last_pos; + self.check_float_base(start_bpos, last_bpos, base); return token::LIT_FLOAT_UNSUFFIXED(str_to_ident( num_str.as_slice())); } else { if num_str.len() == 0u { - self.err_span(start_bpos, self.last_pos, "no valid digits found for number"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "no valid digits found for number"); num_str = "1".to_string(); } let parsed = match from_str_radix::(num_str.as_slice(), base as uint) { Some(p) => p, None => { - self.err_span(start_bpos, self.last_pos, "int literal is too large"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "int literal is too large"); 1 } }; @@ -597,10 +608,12 @@ impl<'a> StringReader<'a> { let start_bpos = self.last_pos; for _ in range(0, n_hex_digits) { if self.is_eof() { - self.fatal_span(start_bpos, self.last_pos, "unterminated numeric character escape"); + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, "unterminated numeric character escape"); } if self.curr_is(delim) { - self.err_span(start_bpos, self.last_pos, "numeric character escape is too short"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "numeric character escape is too short"); break; } let c = self.curr.unwrap_or('\x00'); @@ -616,7 +629,8 @@ impl<'a> StringReader<'a> { match char::from_u32(accum_int) { Some(x) => x, None => { - self.err_span(start_bpos, self.last_pos, "illegal numeric character escape"); + let last_bpos = self.last_pos; + self.err_span(start_bpos, last_bpos, "illegal numeric character escape"); '?' } } @@ -773,17 +787,18 @@ impl<'a> StringReader<'a> { }); let keyword_checking_token = &token::IDENT(keyword_checking_ident, false); + let last_bpos = self.last_pos; if token::is_keyword(token::keywords::Self, keyword_checking_token) { self.err_span(start, - self.last_pos, + last_bpos, "invalid lifetime name: 'self \ is no longer a special lifetime"); } else if token::is_any_keyword(keyword_checking_token) && !token::is_keyword(token::keywords::Static, keyword_checking_token) { self.err_span(start, - self.last_pos, + last_bpos, "invalid lifetime name"); } return token::LIFETIME(ident); @@ -811,7 +826,8 @@ impl<'a> StringReader<'a> { 'u' => self.scan_numeric_escape(4u, '\''), 'U' => self.scan_numeric_escape(8u, '\''), c2 => { - self.err_span_char(escaped_pos, self.last_pos, + let last_bpos = self.last_pos; + self.err_span_char(escaped_pos, last_bpos, "unknown character escape", c2); c2 } @@ -820,17 +836,19 @@ impl<'a> StringReader<'a> { } } '\t' | '\n' | '\r' | '\'' => { - self.err_span_char( start, self.last_pos, + let last_bpos = self.last_pos; + self.err_span_char( start, last_bpos, "character constant must be escaped", c2); } _ => {} } if !self.curr_is('\'') { + let last_bpos = self.last_pos; self.fatal_span_verbose( // Byte offsetting here is okay because the // character before position `start` is an // ascii single quote. - start - BytePos(1), self.last_pos, + start - BytePos(1), last_bpos, "unterminated character constant".to_string()); } self.bump(); // advance curr past token @@ -842,7 +860,8 @@ impl<'a> StringReader<'a> { self.bump(); while !self.curr_is('"') { if self.is_eof() { - self.fatal_span(start_bpos, self.last_pos, "unterminated double quote string"); + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, "unterminated double quote string"); } let ch = self.curr.unwrap(); @@ -850,7 +869,8 @@ impl<'a> StringReader<'a> { match ch { '\\' => { if self.is_eof() { - self.fatal_span(start_bpos, self.last_pos, + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, "unterminated double quote string"); } @@ -876,7 +896,8 @@ impl<'a> StringReader<'a> { accum_str.push_char(self.scan_numeric_escape(8u, '"')); } c2 => { - self.err_span_char(escaped_pos, self.last_pos, + let last_bpos = self.last_pos; + self.err_span_char(escaped_pos, last_bpos, "unknown string escape", c2); } } @@ -897,19 +918,23 @@ impl<'a> StringReader<'a> { } if self.is_eof() { - self.fatal_span(start_bpos, self.last_pos, "unterminated raw string"); + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, "unterminated raw string"); } else if !self.curr_is('"') { - self.fatal_span_char(start_bpos, self.last_pos, + let last_bpos = self.last_pos; + let curr_char = self.curr.unwrap(); + self.fatal_span_char(start_bpos, last_bpos, "only `#` is allowed in raw string delimitation; \ found illegal character", - self.curr.unwrap()); + curr_char); } self.bump(); let content_start_bpos = self.last_pos; let mut content_end_bpos; 'outer: loop { if self.is_eof() { - self.fatal_span(start_bpos, self.last_pos, "unterminated raw string"); + let last_bpos = self.last_pos; + self.fatal_span(start_bpos, last_bpos, "unterminated raw string"); } if self.curr_is('"') { content_end_bpos = self.last_pos; @@ -956,8 +981,9 @@ impl<'a> StringReader<'a> { '^' => { return self.binop(token::CARET); } '%' => { return self.binop(token::PERCENT); } c => { - self.fatal_span_char(self.last_pos, self.pos, - "unknown start of token", c); + let last_bpos = self.last_pos; + let bpos = self.pos; + self.fatal_span_char(last_bpos, bpos, "unknown start of token", c); } } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d11d303059fa1..250ed4af57198 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -146,10 +146,12 @@ macro_rules! maybe_whole_expr ( INTERPOLATED(token::NtPath(ref pt)) => (**pt).clone(), _ => unreachable!() }; - Some($p.mk_expr($p.span.lo, $p.span.hi, ExprPath(pt))) + let span = $p.span; + Some($p.mk_expr(span.lo, span.hi, ExprPath(pt))) } INTERPOLATED(token::NtBlock(b)) => { - Some($p.mk_expr($p.span.lo, $p.span.hi, ExprBlock(b))) + let span = $p.span; + Some($p.mk_expr(span.lo, span.hi, ExprBlock(b))) } _ => None }; @@ -370,7 +372,8 @@ impl<'a> Parser<'a> { pub fn unexpected_last(&mut self, t: &token::Token) -> ! { let token_str = Parser::token_to_str(t); - self.span_fatal(self.last_span, format!("unexpected token: `{}`", + let last_span = self.last_span; + self.span_fatal(last_span, format!("unexpected token: `{}`", token_str).as_slice()); } @@ -441,7 +444,8 @@ impl<'a> Parser<'a> { && expected.iter().all(|t| *t != token::LBRACE) && self.look_ahead(1, |t| *t == token::RBRACE) { // matched; signal non-fatal error and recover. - self.span_err(self.span, + let span = self.span; + self.span_err(span, "unit-like struct construction is written with no trailing `{ }`"); self.eat(&token::LBRACE); self.eat(&token::RBRACE); @@ -560,7 +564,8 @@ impl<'a> Parser<'a> { pub fn check_strict_keywords(&mut self) { if token::is_strict_keyword(&self.token) { let token_str = self.this_token_to_str(); - self.span_err(self.span, + let span = self.span; + self.span_err(span, format!("found `{}` in ident position", token_str).as_slice()); } @@ -581,8 +586,9 @@ impl<'a> Parser<'a> { match self.token { token::BINOP(token::AND) => self.bump(), token::ANDAND => { - let lo = self.span.lo + BytePos(1); - self.replace_token(token::BINOP(token::AND), lo, self.span.hi) + let span = self.span; + let lo = span.lo + BytePos(1); + self.replace_token(token::BINOP(token::AND), lo, span.hi) } _ => { let token_str = self.this_token_to_str(); @@ -601,8 +607,9 @@ impl<'a> Parser<'a> { match self.token { token::BINOP(token::OR) => self.bump(), token::OROR => { - let lo = self.span.lo + BytePos(1); - self.replace_token(token::BINOP(token::OR), lo, self.span.hi) + let span = self.span; + let lo = span.lo + BytePos(1); + self.replace_token(token::BINOP(token::OR), lo, span.hi) } _ => { let found_token = self.this_token_to_str(); @@ -644,8 +651,9 @@ impl<'a> Parser<'a> { _ => false, }); if force || next_lifetime { - let lo = self.span.lo + BytePos(1); - self.replace_token(token::LT, lo, self.span.hi); + let span = self.span; + let lo = span.lo + BytePos(1); + self.replace_token(token::LT, lo, span.hi); true } else { false @@ -693,8 +701,9 @@ impl<'a> Parser<'a> { match self.token { token::GT => self.bump(), token::BINOP(token::SHR) => { - let lo = self.span.lo + BytePos(1); - self.replace_token(token::GT, lo, self.span.hi) + let span = self.span; + let lo = span.lo + BytePos(1); + self.replace_token(token::GT, lo, span.hi) } _ => { let gt_str = Parser::token_to_str(&token::GT); @@ -805,7 +814,8 @@ impl<'a> Parser<'a> { -> Vec { let result = self.parse_unspanned_seq(bra, ket, sep, f); if result.is_empty() { - self.span_err(self.last_span, + let last_span = self.last_span; + self.span_err(last_span, "nullary enum variants are written with no trailing `( )`"); } result @@ -1336,10 +1346,11 @@ impl<'a> Parser<'a> { } else if self.token == token::TILDE { // OWNED POINTER self.bump(); + let last_span = self.last_span; match self.token { token::LBRACKET => - self.obsolete(self.last_span, ObsoleteOwnedVector), - _ => self.obsolete(self.last_span, ObsoleteOwnedType), + self.obsolete(last_span, ObsoleteOwnedVector), + _ => self.obsolete(last_span, ObsoleteOwnedType), }; TyUniq(self.parse_ty(true)) } else if self.token == token::BINOP(token::STAR) { @@ -2375,17 +2386,18 @@ impl<'a> Parser<'a> { let e = self.parse_prefix_expr(); hi = e.span.hi; // HACK: turn ~[...] into a ~-vec + let last_span = self.last_span; ex = match e.node { ExprVec(..) | ExprRepeat(..) => { - self.obsolete(self.last_span, ObsoleteOwnedVector); + self.obsolete(last_span, ObsoleteOwnedVector); ExprVstore(e, ExprVstoreUniq) } ExprLit(lit) if lit_is_str(lit) => { - self.obsolete(self.last_span, ObsoleteOwnedExpr); + self.obsolete(last_span, ObsoleteOwnedExpr); ExprVstore(e, ExprVstoreUniq) } _ => { - self.obsolete(self.last_span, ObsoleteOwnedExpr); + self.obsolete(last_span, ObsoleteOwnedExpr); self.mk_unary(UnUniq, e) } }; @@ -2412,7 +2424,8 @@ impl<'a> Parser<'a> { // HACK: turn `box [...]` into a boxed-vec ex = match subexpression.node { ExprVec(..) | ExprRepeat(..) => { - self.obsolete(self.last_span, ObsoleteOwnedVector); + let last_span = self.last_span; + self.obsolete(last_span, ObsoleteOwnedVector); ExprVstore(subexpression, ExprVstoreUniq) } ExprLit(lit) if lit_is_str(lit) => { @@ -2843,8 +2856,9 @@ impl<'a> Parser<'a> { self.bump(); let sub = self.parse_pat(); pat = PatBox(sub); - hi = self.last_span.hi; - self.obsolete(self.last_span, ObsoleteOwnedPattern); + let last_span = self.last_span; + hi = last_span.hi; + self.obsolete(last_span, ObsoleteOwnedPattern); return box(GC) ast::Pat { id: ast::DUMMY_NODE_ID, node: pat, @@ -3061,7 +3075,8 @@ impl<'a> Parser<'a> { binding_mode: ast::BindingMode) -> ast::Pat_ { if !is_plain_ident(&self.token) { - self.span_fatal(self.last_span, + let last_span = self.last_span; + self.span_fatal(last_span, "expected identifier, found path"); } // why a path here, and not just an identifier? @@ -3079,8 +3094,9 @@ impl<'a> Parser<'a> { // binding mode then we do not end up here, because the lookahead // will direct us over to parse_enum_variant() if self.token == token::LPAREN { + let last_span = self.last_span; self.span_fatal( - self.last_span, + last_span, "expected identifier, found enum pattern"); } @@ -3144,7 +3160,8 @@ impl<'a> Parser<'a> { fn check_expected_item(p: &mut Parser, found_attrs: bool) { // If we have attributes then we should have an item if found_attrs { - p.span_err(p.last_span, "expected item after attributes"); + let last_span = p.last_span; + p.span_err(last_span, "expected item after attributes"); } } @@ -3333,7 +3350,8 @@ impl<'a> Parser<'a> { match self.token { token::SEMI => { if !attributes_box.is_empty() { - self.span_err(self.last_span, "expected item after attributes"); + let last_span = self.last_span; + self.span_err(last_span, "expected item after attributes"); attributes_box = Vec::new(); } self.bump(); // empty @@ -3409,7 +3427,8 @@ impl<'a> Parser<'a> { } if !attributes_box.is_empty() { - self.span_err(self.last_span, "expected item after attributes"); + let last_span = self.last_span; + self.span_err(last_span, "expected item after attributes"); } let hi = self.span.hi; @@ -3566,7 +3585,8 @@ impl<'a> Parser<'a> { if ty_param.default.is_some() { seen_default = true; } else if seen_default { - p.span_err(p.last_span, + let last_span = p.last_span; + p.span_err(last_span, "type parameters with a default must be trailing"); } ty_param @@ -3591,7 +3611,8 @@ impl<'a> Parser<'a> { fn forbid_lifetime(&mut self) { if Parser::token_is_lifetime(&self.token) { - self.span_fatal(self.span, "lifetime parameters must be declared \ + let span = self.span; + self.span_fatal(span, "lifetime parameters must be declared \ prior to type parameters"); } } @@ -3609,11 +3630,13 @@ impl<'a> Parser<'a> { p.bump(); if allow_variadic { if p.token != token::RPAREN { - p.span_fatal(p.span, + let span = p.span; + p.span_fatal(span, "`...` must be last in argument list for variadic function"); } } else { - p.span_fatal(p.span, + let span = p.span; + p.span_fatal(span, "only foreign functions are allowed to be variadic"); } None @@ -3756,7 +3779,8 @@ impl<'a> Parser<'a> { self.parse_mutability() } else { MutImmutable }; if self.is_self_ident() { - self.span_err(self.span, "cannot pass self by unsafe pointer"); + let span = self.span; + self.span_err(span, "cannot pass self by unsafe pointer"); self.bump(); } SelfValue @@ -4128,15 +4152,17 @@ impl<'a> Parser<'a> { token::RBRACE => {} #[cfg(stage0)] _ => { + let span = self.span; let token_str = self.this_token_to_str(); - self.span_fatal(self.span, + self.span_fatal(span, format!("expected `,`, or `\\}` but found `{}`", token_str).as_slice()) } #[cfg(not(stage0))] _ => { + let span = self.span; let token_str = self.this_token_to_str(); - self.span_fatal(self.span, + self.span_fatal(span, format!("expected `,`, or `}}` but found `{}`", token_str).as_slice()) } @@ -4170,7 +4196,8 @@ impl<'a> Parser<'a> { fn parse_for_sized(&mut self) -> Sized { if self.eat_keyword(keywords::For) { if !self.eat_keyword(keywords::Type) { - self.span_err(self.last_span, + let last_span = self.last_span; + self.span_err(last_span, "expected 'type' after for in trait item"); } DynSize @@ -4226,7 +4253,8 @@ impl<'a> Parser<'a> { if first && attrs_remaining_len > 0u { // We parsed attributes for the first item but didn't find it - self.span_err(self.last_span, "expected item after attributes"); + let last_span = self.last_span; + self.span_err(last_span, "expected item after attributes"); } ast::Mod { @@ -4458,7 +4486,8 @@ impl<'a> Parser<'a> { foreign_items: foreign_items } = self.parse_foreign_items(first_item_attrs, true); if ! attrs_remaining.is_empty() { - self.span_err(self.last_span, + let last_span = self.last_span; + self.span_err(last_span, "expected item after attributes"); } assert!(self.token == token::RBRACE); @@ -4494,8 +4523,9 @@ impl<'a> Parser<'a> { (path, the_ident) } _ => { + let span = self.span; let token_str = self.this_token_to_str(); - self.span_fatal(self.span, + self.span_fatal(span, format!("expected extern crate name but \ found `{}`", token_str).as_slice()); @@ -4535,8 +4565,9 @@ impl<'a> Parser<'a> { let m = self.parse_foreign_mod_items(abi, next); self.expect(&token::RBRACE); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, special_idents::invalid, ItemForeignMod(m), visibility, @@ -4663,8 +4694,9 @@ impl<'a> Parser<'a> { match abi::lookup(the_string) { Some(abi) => Some(abi), None => { + let last_span = self.last_span; self.span_err( - self.last_span, + last_span, format!("illegal ABI: expected one of [{}], \ found `{}`", abi::all_names().connect(", "), @@ -4720,7 +4752,8 @@ impl<'a> Parser<'a> { if next_is_mod || self.eat_keyword(keywords::Crate) { if next_is_mod { - self.span_err(mk_sp(lo, self.last_span.hi), + let last_span = self.last_span; + self.span_err(mk_sp(lo, last_span.hi), format!("`extern mod` is obsolete, use \ `extern crate` instead \ to refer to external \ @@ -4736,8 +4769,9 @@ impl<'a> Parser<'a> { let abi = opt_abi.unwrap_or(abi::C); let (ident, item_, extra_attrs) = self.parse_item_fn(NormalFn, abi); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4747,15 +4781,17 @@ impl<'a> Parser<'a> { return self.parse_item_foreign_mod(lo, opt_abi, visibility, attrs); } + let span = self.span; let token_str = self.this_token_to_str(); - self.span_fatal(self.span, + self.span_fatal(span, format!("expected `{}` or `fn` but found `{}`", "{", token_str).as_slice()); } let is_virtual = self.eat_keyword(keywords::Virtual); if is_virtual && !self.is_keyword(keywords::Struct) { - self.span_err(self.span, + let span = self.span; + self.span_err(span, "`virtual` keyword may only be used with `struct`"); } @@ -4764,8 +4800,9 @@ impl<'a> Parser<'a> { // STATIC ITEM self.bump(); let (ident, item_, extra_attrs) = self.parse_item_const(); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4778,8 +4815,9 @@ impl<'a> Parser<'a> { self.bump(); let (ident, item_, extra_attrs) = self.parse_item_fn(NormalFn, abi::Rust); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4798,8 +4836,9 @@ impl<'a> Parser<'a> { self.expect_keyword(keywords::Fn); let (ident, item_, extra_attrs) = self.parse_item_fn(UnsafeFn, abi); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4810,8 +4849,9 @@ impl<'a> Parser<'a> { // MODULE ITEM let (ident, item_, extra_attrs) = self.parse_item_mod(attrs.as_slice()); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4821,8 +4861,9 @@ impl<'a> Parser<'a> { if self.eat_keyword(keywords::Type) { // TYPE ITEM let (ident, item_, extra_attrs) = self.parse_item_type(); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4832,8 +4873,9 @@ impl<'a> Parser<'a> { if self.eat_keyword(keywords::Enum) { // ENUM ITEM let (ident, item_, extra_attrs) = self.parse_item_enum(); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4843,8 +4885,9 @@ impl<'a> Parser<'a> { if self.eat_keyword(keywords::Trait) { // TRAIT ITEM let (ident, item_, extra_attrs) = self.parse_item_trait(); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4854,8 +4897,9 @@ impl<'a> Parser<'a> { if self.eat_keyword(keywords::Impl) { // IMPL ITEM let (ident, item_, extra_attrs) = self.parse_item_impl(); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4865,8 +4909,9 @@ impl<'a> Parser<'a> { if self.eat_keyword(keywords::Struct) { // STRUCT ITEM let (ident, item_, extra_attrs) = self.parse_item_struct(is_virtual); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, ident, item_, visibility, @@ -4942,8 +4987,9 @@ impl<'a> Parser<'a> { span: mk_sp(self.span.lo, self.span.hi) }; let item_ = ItemMac(m); + let last_span = self.last_span; let item = self.mk_item(lo, - self.last_span.hi, + last_span.hi, id, item_, visibility, @@ -4960,7 +5006,8 @@ impl<'a> Parser<'a> { s.push_str("priv") } s.push_char('`'); - self.span_fatal(self.last_span, s.as_slice()); + let last_span = self.last_span; + self.span_fatal(last_span, s.as_slice()); } return IoviNone(attrs); } diff --git a/src/libsyntax/print/pp.rs b/src/libsyntax/print/pp.rs index 4fefa1f1d3d1e..672e08af2d8ff 100644 --- a/src/libsyntax/print/pp.rs +++ b/src/libsyntax/print/pp.rs @@ -322,7 +322,8 @@ impl Printer { b.offset, self.left, self.right); *self.token.get_mut(self.right) = t; *self.size.get_mut(self.right) = -self.right_total; - self.scan_push(self.right); + let right = self.right; + self.scan_push(right); Ok(()) } End => { @@ -334,7 +335,8 @@ impl Printer { self.advance_right(); *self.token.get_mut(self.right) = t; *self.size.get_mut(self.right) = -1; - self.scan_push(self.right); + let right = self.right; + self.scan_push(right); Ok(()) } } @@ -348,7 +350,8 @@ impl Printer { debug!("pp Break({})/buffer ~[{},{}]", b.offset, self.left, self.right); self.check_stack(0); - self.scan_push(self.right); + let right = self.right; + self.scan_push(right); *self.token.get_mut(self.right) = t; *self.size.get_mut(self.right) = -self.right_total; self.right_total += b.blank_space; diff --git a/src/libterm/terminfo/parm.rs b/src/libterm/terminfo/parm.rs index de70ac6f592b9..b49c698486450 100644 --- a/src/libterm/terminfo/parm.rs +++ b/src/libterm/terminfo/parm.rs @@ -347,7 +347,8 @@ pub fn expand(cap: &[u8], params: &[Param], vars: &mut Variables) let res = format(stack.pop().unwrap(), FormatOp::from_char(cur), *flags); if res.is_err() { return res } output.push_all(res.unwrap().as_slice()); - old_state = state; // will cause state to go to Nothing + // will cause state to go to Nothing + old_state = FormatPattern(*flags, *fstate); } else { return Err("stack is empty".to_string()) }, (FormatStateFlags,'#') => { flags.alternate = true; From d7de4e9affac24c6e96ae068954480bfa763908c Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:09 -0700 Subject: [PATCH 7/9] Enforce stronger guarantees for mutable borrows Implement the stronger guarantees for mutable borrows from #12624. This removes the ability to read from a mutably borrowed path for the duration of the borrow, and enforces a unique access path for any mutable borrow, for both reads and writes. This makes mutable borrows work better with concurrent accesses from multiple threads, and it opens the door for allowing moves out of mutably borrowed values, as long as a new value is written before the mutable borrow ends. This also aligns Rust more closely with academic languages based on substructural types and separation logic. The most common situation triggering an error after this change is a call to a function mutably borrowing self with self.field as one of the arguments. The workaround is to bind self.field to a temporary, but the need for these temporaries will hopefully go away after #6268 is fixed. Another situation that triggers an error is using the head expression of a match in an arm that binds a variable with a mutable reference. The use of the head expression needs to be replaced with an expression that reconstructs it from match-bound variables. This fixes #12624. [breaking-change] --- src/librustc/middle/borrowck/check_loans.rs | 24 +++++++++++++++++-- .../borrowck-vec-pattern-loan-from-mut.rs | 2 +- .../regions-escape-loop-via-vec.rs | 4 ++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 2015572133f88..2392db6301929 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -438,8 +438,7 @@ impl<'a> CheckLoanCtxt<'a> { Some(lp) => { let moved_value_use_kind = match mode { euv::Copy => { - // FIXME(#12624) -- If we are copying the value, - // we don't care if it's borrowed. + self.check_for_copy_of_frozen_path(id, span, &*lp); MovedInUse } euv::Move(_) => { @@ -471,6 +470,27 @@ impl<'a> CheckLoanCtxt<'a> { } } + fn check_for_copy_of_frozen_path(&self, + id: ast::NodeId, + span: Span, + copy_path: &LoanPath) { + match self.analyze_restrictions_on_use(id, copy_path, ty::ImmBorrow) { + UseOk => { } + UseWhileBorrowed(loan_path, loan_span) => { + self.bccx.span_err( + span, + format!("cannot use `{}` because it was mutably borrowed", + self.bccx.loan_path_to_str(copy_path).as_slice()) + .as_slice()); + self.bccx.span_note( + loan_span, + format!("borrow of `{}` occurs here", + self.bccx.loan_path_to_str(&*loan_path).as_slice()) + .as_slice()); + } + } + } + fn check_for_move_of_borrowed_path(&self, id: ast::NodeId, span: Span, diff --git a/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs b/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs index 393ec8b0b1b3b..ff029ce624c9b 100644 --- a/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs +++ b/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs @@ -12,7 +12,7 @@ fn a() { let mut v = vec!(1, 2, 3); let vb: &mut [int] = v.as_mut_slice(); match vb { - [_a, ..tail] => { + [_a, ..tail] => { //~ ERROR cannot use `vb[..]` because it was mutably borrowed v.push(tail[0] + tail[1]); //~ ERROR cannot borrow } _ => {} diff --git a/src/test/compile-fail/regions-escape-loop-via-vec.rs b/src/test/compile-fail/regions-escape-loop-via-vec.rs index 7d9629a522053..89350f1616760 100644 --- a/src/test/compile-fail/regions-escape-loop-via-vec.rs +++ b/src/test/compile-fail/regions-escape-loop-via-vec.rs @@ -12,8 +12,8 @@ fn broken() { let mut x = 3; let mut _y = vec!(&mut x); - while x < 10 { - let mut z = x; + while x < 10 { //~ ERROR cannot use `x` because it was mutably borrowed + let mut z = x; //~ ERROR cannot use `x` because it was mutably borrowed _y.push(&mut z); //~ ERROR `z` does not live long enough x += 1; //~ ERROR cannot assign } From 5878b5edb0036312ffee12fc730e69d57df31a66 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:10 -0700 Subject: [PATCH 8/9] Add new tests for uses of mutably borrowed paths --- .../compile-fail/borrowck-use-mut-borrow.rs | 93 +++++++++++++++++++ src/test/run-pass/borrowck-use-mut-borrow.rs | 57 ++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 src/test/compile-fail/borrowck-use-mut-borrow.rs create mode 100644 src/test/run-pass/borrowck-use-mut-borrow.rs diff --git a/src/test/compile-fail/borrowck-use-mut-borrow.rs b/src/test/compile-fail/borrowck-use-mut-borrow.rs new file mode 100644 index 0000000000000..7414bb930d4d6 --- /dev/null +++ b/src/test/compile-fail/borrowck-use-mut-borrow.rs @@ -0,0 +1,93 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct A { a: int, b: int } +struct B { a: int, b: Box } + +fn var_copy_after_var_borrow() { + let mut x: int = 1; + let p = &mut x; + drop(x); //~ ERROR cannot use `x` because it was mutably borrowed + *p = 2; +} + +fn var_copy_after_field_borrow() { + let mut x = A { a: 1, b: 2 }; + let p = &mut x.a; + drop(x); //~ ERROR cannot use `x` because it was mutably borrowed + *p = 3; +} + +fn field_copy_after_var_borrow() { + let mut x = A { a: 1, b: 2 }; + let p = &mut x; + drop(x.a); //~ ERROR cannot use `x.a` because it was mutably borrowed + p.a = 3; +} + +fn field_copy_after_field_borrow() { + let mut x = A { a: 1, b: 2 }; + let p = &mut x.a; + drop(x.a); //~ ERROR cannot use `x.a` because it was mutably borrowed + *p = 3; +} + +fn fu_field_copy_after_var_borrow() { + let mut x = A { a: 1, b: 2 }; + let p = &mut x; + let y = A { b: 3, .. x }; //~ ERROR cannot use `x.a` because it was mutably borrowed + drop(y); + p.a = 4; +} + +fn fu_field_copy_after_field_borrow() { + let mut x = A { a: 1, b: 2 }; + let p = &mut x.a; + let y = A { b: 3, .. x }; //~ ERROR cannot use `x.a` because it was mutably borrowed + drop(y); + *p = 4; +} + +fn var_deref_after_var_borrow() { + let mut x: Box = box 1; + let p = &mut x; + drop(*x); //~ ERROR cannot use `*x` because it was mutably borrowed + **p = 2; +} + +fn field_deref_after_var_borrow() { + let mut x = B { a: 1, b: box 2 }; + let p = &mut x; + drop(*x.b); //~ ERROR cannot use `*x.b` because it was mutably borrowed + p.a = 3; +} + +fn field_deref_after_field_borrow() { + let mut x = B { a: 1, b: box 2 }; + let p = &mut x.b; + drop(*x.b); //~ ERROR cannot use `*x.b` because it was mutably borrowed + **p = 3; +} + +fn main() { + var_copy_after_var_borrow(); + var_copy_after_field_borrow(); + + field_copy_after_var_borrow(); + field_copy_after_field_borrow(); + + fu_field_copy_after_var_borrow(); + fu_field_copy_after_field_borrow(); + + var_deref_after_var_borrow(); + field_deref_after_var_borrow(); + field_deref_after_field_borrow(); +} + diff --git a/src/test/run-pass/borrowck-use-mut-borrow.rs b/src/test/run-pass/borrowck-use-mut-borrow.rs new file mode 100644 index 0000000000000..cbfdd5961ffac --- /dev/null +++ b/src/test/run-pass/borrowck-use-mut-borrow.rs @@ -0,0 +1,57 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct A { a: int, b: Box } + +fn field_copy_after_field_borrow() { + let mut x = A { a: 1, b: box 2 }; + let p = &mut x.b; + drop(x.a); + **p = 3; +} + +fn fu_field_copy_after_field_borrow() { + let mut x = A { a: 1, b: box 2 }; + let p = &mut x.b; + let y = A { b: box 3, .. x }; + drop(y); + **p = 4; +} + +fn field_deref_after_field_borrow() { + let mut x = A { a: 1, b: box 2 }; + let p = &mut x.a; + drop(*x.b); + *p = 3; +} + +fn field_move_after_field_borrow() { + let mut x = A { a: 1, b: box 2 }; + let p = &mut x.a; + drop(x.b); + *p = 3; +} + +fn fu_field_move_after_field_borrow() { + let mut x = A { a: 1, b: box 2 }; + let p = &mut x.a; + let y = A { a: 3, .. x }; + drop(y); + *p = 4; +} + +fn main() { + field_copy_after_field_borrow(); + fu_field_copy_after_field_borrow(); + field_deref_after_field_borrow(); + field_move_after_field_borrow(); + fu_field_move_after_field_borrow(); +} + From 6fc788916c297d6e03464b80f12ba0e62fccccac Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Fri, 13 Jun 2014 20:48:10 -0700 Subject: [PATCH 9/9] Reorganize code in check_loans Move analyze_restrictions_on_use and check_if_path_is_moved so that all of the code related to assignments is in a contiguous block at the end of the file. --- src/librustc/middle/borrowck/check_loans.rs | 186 ++++++++++---------- 1 file changed, 93 insertions(+), 93 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 2392db6301929..111441180475e 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -523,6 +523,99 @@ impl<'a> CheckLoanCtxt<'a> { } } + pub fn analyze_restrictions_on_use(&self, + expr_id: ast::NodeId, + use_path: &LoanPath, + borrow_kind: ty::BorrowKind) + -> UseError { + debug!("analyze_restrictions_on_use(expr_id={:?}, use_path={})", + self.tcx().map.node_to_str(expr_id), + use_path.repr(self.tcx())); + + let mut ret = UseOk; + + // First, we check for a restriction on the path P being used. This + // accounts for borrows of P but also borrows of subpaths, like P.a.b. + // Consider the following example: + // + // let x = &mut a.b.c; // Restricts a, a.b, and a.b.c + // let y = a; // Conflicts with restriction + + self.each_in_scope_restriction(expr_id, use_path, |loan, _restr| { + if incompatible(loan.kind, borrow_kind) { + ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); + false + } else { + true + } + }); + + // Next, we must check for *loans* (not restrictions) on the path P or + // any base path. This rejects examples like the following: + // + // let x = &mut a.b; + // let y = a.b.c; + // + // Limiting this search to *loans* and not *restrictions* means that + // examples like the following continue to work: + // + // let x = &mut a.b; + // let y = a.c; + + let mut loan_path = use_path; + loop { + self.each_in_scope_loan(expr_id, |loan| { + if *loan.loan_path == *loan_path && + incompatible(loan.kind, borrow_kind) { + ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); + false + } else { + true + } + }); + + match *loan_path { + LpVar(_) => { + break; + } + LpExtend(ref lp_base, _, _) => { + loan_path = &**lp_base; + } + } + } + + return ret; + + fn incompatible(borrow_kind1: ty::BorrowKind, + borrow_kind2: ty::BorrowKind) + -> bool { + borrow_kind1 != ty::ImmBorrow || borrow_kind2 != ty::ImmBorrow + } + } + + fn check_if_path_is_moved(&self, + id: ast::NodeId, + span: Span, + use_kind: MovedValueUseKind, + lp: &Rc) { + /*! + * Reports an error if `expr` (which should be a path) + * is using a moved/uninitialized value + */ + + debug!("check_if_path_is_moved(id={:?}, use_kind={:?}, lp={})", + id, use_kind, lp.repr(self.bccx.tcx)); + self.move_data.each_move_of(id, lp, |move, moved_lp| { + self.bccx.report_use_of_moved_value( + span, + use_kind, + &**lp, + move, + moved_lp); + false + }); + } + fn check_if_assigned_path_is_moved(&self, id: ast::NodeId, span: Span, @@ -564,29 +657,6 @@ impl<'a> CheckLoanCtxt<'a> { } } - fn check_if_path_is_moved(&self, - id: ast::NodeId, - span: Span, - use_kind: MovedValueUseKind, - lp: &Rc) { - /*! - * Reports an error if `expr` (which should be a path) - * is using a moved/uninitialized value - */ - - debug!("check_if_path_is_moved(id={:?}, use_kind={:?}, lp={})", - id, use_kind, lp.repr(self.bccx.tcx)); - self.move_data.each_move_of(id, lp, |move, moved_lp| { - self.bccx.report_use_of_moved_value( - span, - use_kind, - &**lp, - move, - moved_lp); - false - }); - } - fn check_assignment(&self, assignment_id: ast::NodeId, assignment_span: Span, @@ -885,74 +955,4 @@ impl<'a> CheckLoanCtxt<'a> { format!("borrow of `{}` occurs here", self.bccx.loan_path_to_str(loan_path)).as_slice()); } - - pub fn analyze_restrictions_on_use(&self, - expr_id: ast::NodeId, - use_path: &LoanPath, - borrow_kind: ty::BorrowKind) - -> UseError { - debug!("analyze_restrictions_on_use(expr_id={:?}, use_path={})", - self.tcx().map.node_to_str(expr_id), - use_path.repr(self.tcx())); - - let mut ret = UseOk; - - // First, we check for a restriction on the path P being used. This - // accounts for borrows of P but also borrows of subpaths, like P.a.b. - // Consider the following example: - // - // let x = &mut a.b.c; // Restricts a, a.b, and a.b.c - // let y = a; // Conflicts with restriction - - self.each_in_scope_restriction(expr_id, use_path, |loan, _restr| { - if incompatible(loan.kind, borrow_kind) { - ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); - false - } else { - true - } - }); - - // Next, we must check for *loans* (not restrictions) on the path P or - // any base path. This rejects examples like the following: - // - // let x = &mut a.b; - // let y = a.b.c; - // - // Limiting this search to *loans* and not *restrictions* means that - // examples like the following continue to work: - // - // let x = &mut a.b; - // let y = a.c; - - let mut loan_path = use_path; - loop { - self.each_in_scope_loan(expr_id, |loan| { - if *loan.loan_path == *loan_path && - incompatible(loan.kind, borrow_kind) { - ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); - false - } else { - true - } - }); - - match *loan_path { - LpVar(_) => { - break; - } - LpExtend(ref lp_base, _, _) => { - loan_path = &**lp_base; - } - } - } - - return ret; - - fn incompatible(borrow_kind1: ty::BorrowKind, - borrow_kind2: ty::BorrowKind) - -> bool { - borrow_kind1 != ty::ImmBorrow || borrow_kind2 != ty::ImmBorrow - } - } }