diff options
7 files changed, 323 insertions, 49 deletions
diff --git a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs index d07d6a066b0..f250b105d0b 100644 --- a/src/librustc/middle/borrowck/gather_loans/gather_moves.rs +++ b/src/librustc/middle/borrowck/gather_loans/gather_moves.rs @@ -14,12 +14,21 @@ use mc = middle::mem_categorization; use middle::borrowck::*; +use middle::borrowck::gather_loans::move_error::{MoveError, MoveErrorCollector}; +use middle::borrowck::gather_loans::move_error::MoveSpanAndPath; use middle::borrowck::move_data::*; use middle::moves; use middle::ty; use syntax::ast; use syntax::codemap::Span; -use util::ppaux::{Repr, UserString}; +use util::ppaux::Repr; + +struct GatherMoveInfo { + id: ast::NodeId, + kind: MoveKind, + cmt: mc::cmt, + span_path_opt: Option<MoveSpanAndPath> +} pub fn gather_decl(bccx: &BorrowckCtxt, move_data: &MoveData, @@ -32,20 +41,42 @@ pub fn gather_decl(bccx: &BorrowckCtxt, pub fn gather_move_from_expr(bccx: &BorrowckCtxt, move_data: &MoveData, + move_error_collector: &MoveErrorCollector, move_expr: &ast::Expr, cmt: mc::cmt) { - gather_move(bccx, move_data, move_expr.id, MoveExpr, cmt); + let move_info = GatherMoveInfo { + id: move_expr.id, + kind: MoveExpr, + cmt: cmt, + span_path_opt: None, + }; + gather_move(bccx, move_data, move_error_collector, move_info); } pub fn gather_move_from_pat(bccx: &BorrowckCtxt, move_data: &MoveData, + move_error_collector: &MoveErrorCollector, move_pat: &ast::Pat, cmt: mc::cmt) { - gather_move(bccx, move_data, move_pat.id, MovePat, cmt); + let pat_span_path_opt = match move_pat.node { + ast::PatIdent(_, ref path, _) => { + Some(MoveSpanAndPath::with_span_and_path(move_pat.span, + (*path).clone())) + }, + _ => None, + }; + let move_info = GatherMoveInfo { + id: move_pat.id, + kind: MovePat, + cmt: cmt, + span_path_opt: pat_span_path_opt, + }; + gather_move(bccx, move_data, move_error_collector, move_info); } pub fn gather_captures(bccx: &BorrowckCtxt, move_data: &MoveData, + move_error_collector: &MoveErrorCollector, closure_expr: &ast::Expr) { for captured_var in bccx.capture_map.get(&closure_expr.id).iter() { match captured_var.mode { @@ -53,7 +84,13 @@ pub fn gather_captures(bccx: &BorrowckCtxt, let cmt = bccx.cat_captured_var(closure_expr.id, closure_expr.span, captured_var); - gather_move(bccx, move_data, closure_expr.id, Captured, cmt); + let move_info = GatherMoveInfo { + id: closure_expr.id, + kind: Captured, + cmt: cmt, + span_path_opt: None + }; + gather_move(bccx, move_data, move_error_collector, move_info); } moves::CapCopy | moves::CapRef => {} } @@ -62,19 +99,27 @@ pub fn gather_captures(bccx: &BorrowckCtxt, fn gather_move(bccx: &BorrowckCtxt, move_data: &MoveData, - move_id: ast::NodeId, - move_kind: MoveKind, - cmt: mc::cmt) { + move_error_collector: &MoveErrorCollector, + move_info: GatherMoveInfo) { debug!("gather_move(move_id={}, cmt={})", - move_id, cmt.repr(bccx.tcx)); - - if !check_is_legal_to_move_from(bccx, cmt, cmt) { - return; + move_info.id, move_info.cmt.repr(bccx.tcx)); + + let potentially_illegal_move = + check_and_get_illegal_move_origin(bccx, move_info.cmt); + match potentially_illegal_move { + Some(illegal_move_origin) => { + let error = MoveError::with_move_info(illegal_move_origin, + move_info.span_path_opt); + move_error_collector.add_error(error); + return + } + None => () } - match opt_loan_path(cmt) { + match opt_loan_path(move_info.cmt) { Some(loan_path) => { - move_data.add_move(bccx.tcx, loan_path, move_id, move_kind); + move_data.add_move(bccx.tcx, loan_path, + move_info.id, move_info.kind); } None => { // move from rvalue or unsafe pointer, hence ok @@ -110,33 +155,28 @@ pub fn gather_move_and_assignment(bccx: &BorrowckCtxt, true); } -fn check_is_legal_to_move_from(bccx: &BorrowckCtxt, - cmt0: mc::cmt, - cmt: mc::cmt) -> bool { +fn check_and_get_illegal_move_origin(bccx: &BorrowckCtxt, + cmt: mc::cmt) -> Option<mc::cmt> { match cmt.cat { mc::cat_deref(_, _, mc::BorrowedPtr(..)) | mc::cat_deref(_, _, mc::GcPtr) | mc::cat_deref(_, _, mc::UnsafePtr(..)) | mc::cat_upvar(..) | mc::cat_static_item | mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => { - bccx.span_err( - cmt0.span, - format!("cannot move out of {}", - bccx.cmt_to_str(cmt))); - false + Some(cmt) } // Can move out of captured upvars only if the destination closure // type is 'once'. 1-shot stack closures emit the copied_upvar form // (see mem_categorization.rs). mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Once, .. }) => { - true + None } mc::cat_rvalue(..) | mc::cat_local(..) | mc::cat_arg(..) => { - true + None } mc::cat_downcast(b) | @@ -144,25 +184,20 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt, match ty::get(b.ty).sty { ty::ty_struct(did, _) | ty::ty_enum(did, _) => { if ty::has_dtor(bccx.tcx, did) { - bccx.span_err( - cmt0.span, - format!("cannot move out of type `{}`, \ - which defines the `Drop` trait", - b.ty.user_string(bccx.tcx))); - false + Some(cmt) } else { - check_is_legal_to_move_from(bccx, cmt0, b) + check_and_get_illegal_move_origin(bccx, b) } } _ => { - check_is_legal_to_move_from(bccx, cmt0, b) + check_and_get_illegal_move_origin(bccx, b) } } } mc::cat_deref(b, _, mc::OwnedPtr) | mc::cat_discr(b, _) => { - check_is_legal_to_move_from(bccx, cmt0, b) + check_and_get_illegal_move_origin(bccx, b) } } } diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index a6409131bed..e0fe50f048e 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -1,4 +1,4 @@ -// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT +// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -39,6 +39,7 @@ use syntax::ast::{Expr, FnDecl, Block, NodeId, Stmt, Pat, Local}; mod lifetime; mod restrictions; mod gather_moves; +mod move_error; /// Context used while gathering loans: /// @@ -70,6 +71,7 @@ struct GatherLoanCtxt<'a> { bccx: &'a BorrowckCtxt<'a>, id_range: IdRange, move_data: move_data::MoveData, + move_error_collector: move_error::MoveErrorCollector, all_loans: Vec<Loan>, item_ub: ast::NodeId, repeating_ids: Vec<ast::NodeId> } @@ -121,11 +123,13 @@ pub fn gather_loans_in_fn(bccx: &BorrowckCtxt, decl: &ast::FnDecl, body: &ast::B all_loans: Vec::new(), item_ub: body.id, repeating_ids: vec!(body.id), - move_data: MoveData::new() + move_data: MoveData::new(), + move_error_collector: move_error::MoveErrorCollector::new(), }; glcx.gather_fn_arg_patterns(decl, body); glcx.visit_block(body, ()); + glcx.report_potential_errors(); let GatherLoanCtxt { id_range, all_loans, move_data, .. } = glcx; (id_range, all_loans, move_data) } @@ -180,7 +184,7 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, if this.bccx.is_move(ex.id) { let cmt = this.bccx.cat_expr(ex); gather_moves::gather_move_from_expr( - this.bccx, &this.move_data, ex, cmt); + this.bccx, &this.move_data, &this.move_error_collector, ex, cmt); } // Special checks for various kinds of expressions: @@ -270,7 +274,8 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt, } ast::ExprFnBlock(..) | ast::ExprProc(..) => { - gather_moves::gather_captures(this.bccx, &this.move_data, ex); + gather_moves::gather_captures(this.bccx, &this.move_data, + &this.move_error_collector, ex); this.guarantee_captures(ex); visit::walk_expr(this, ex, ()); } @@ -874,7 +879,8 @@ impl<'a> GatherLoanCtxt<'a> { // No borrows here, but there may be moves if self.bccx.is_move(pat.id) { gather_moves::gather_move_from_pat( - self.bccx, &self.move_data, pat, cmt); + self.bccx, &self.move_data, + &self.move_error_collector, pat, cmt); } } } @@ -925,6 +931,10 @@ impl<'a> GatherLoanCtxt<'a> { pub fn pat_is_binding(&self, pat: &ast::Pat) -> bool { pat_util::pat_is_binding(self.bccx.tcx.def_map, pat) } + + pub fn report_potential_errors(&self) { + self.move_error_collector.report_potential_errors(self.bccx); + } } /// Context used while gathering loans on static initializers diff --git a/src/librustc/middle/borrowck/gather_loans/move_error.rs b/src/librustc/middle/borrowck/gather_loans/move_error.rs new file mode 100644 index 00000000000..c9a8df0f535 --- /dev/null +++ b/src/librustc/middle/borrowck/gather_loans/move_error.rs @@ -0,0 +1,169 @@ +// 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. + +use mc = middle::mem_categorization; +use middle::borrowck::BorrowckCtxt; +use middle::ty; + +use std::cell::RefCell; +use syntax::ast; +use syntax::codemap; +use syntax::print::pprust; +use util::ppaux::UserString; + +pub struct MoveErrorCollector { + errors: RefCell<Vec<MoveError>> +} + +impl MoveErrorCollector { + pub fn new() -> MoveErrorCollector { + MoveErrorCollector { + errors: RefCell::new(Vec::new()) + } + } + + pub fn add_error(&self, error: MoveError) { + self.errors.borrow_mut().push(error); + } + + pub fn report_potential_errors(&self, bccx: &BorrowckCtxt) { + report_move_errors(bccx, self.errors.borrow().deref()) + } +} + +pub struct MoveError { + move_from: mc::cmt, + move_to: Option<MoveSpanAndPath> +} + +impl MoveError { + pub fn with_move_info(move_from: mc::cmt, + move_to: Option<MoveSpanAndPath>) + -> MoveError { + MoveError { + move_from: move_from, + move_to: move_to, + } + } +} + +#[deriving(Clone)] +pub struct MoveSpanAndPath { + span: codemap::Span, + path: ast::Path +} + +impl MoveSpanAndPath { + pub fn with_span_and_path(span: codemap::Span, + path: ast::Path) + -> MoveSpanAndPath { + MoveSpanAndPath { + span: span, + path: path, + } + } +} + +pub struct GroupedMoveErrors { + move_from: mc::cmt, + move_to_places: Vec<MoveSpanAndPath> +} + +fn report_move_errors(bccx: &BorrowckCtxt, errors: &Vec<MoveError>) { + let grouped_errors = group_errors_with_same_origin(errors); + for error in grouped_errors.iter() { + report_cannot_move_out_of(bccx, error.move_from); + let mut is_first_note = true; + for move_to in error.move_to_places.iter() { + note_move_destination(bccx, move_to.span, + &move_to.path, is_first_note); + is_first_note = false; + } + } +} + +fn group_errors_with_same_origin(errors: &Vec<MoveError>) + -> Vec<GroupedMoveErrors> { + let mut grouped_errors = Vec::new(); + for error in errors.iter() { + append_to_grouped_errors(&mut grouped_errors, error) + } + return grouped_errors; + + fn append_to_grouped_errors(grouped_errors: &mut Vec<GroupedMoveErrors>, + error: &MoveError) { + let move_from_id = error.move_from.id; + let move_to = if error.move_to.is_some() { + vec!(error.move_to.clone().unwrap()) + } else { + Vec::new() + }; + for ge in grouped_errors.mut_iter() { + if move_from_id == ge.move_from.id && error.move_to.is_some() { + ge.move_to_places.push_all_move(move_to); + return + } + } + grouped_errors.push(GroupedMoveErrors { + move_from: error.move_from, + move_to_places: move_to + }) + } +} + +fn report_cannot_move_out_of(bccx: &BorrowckCtxt, move_from: mc::cmt) { + match move_from.cat { + mc::cat_deref(_, _, mc::BorrowedPtr(..)) | + mc::cat_deref(_, _, mc::GcPtr) | + mc::cat_deref(_, _, mc::UnsafePtr(..)) | + mc::cat_upvar(..) | mc::cat_static_item | + mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => { + bccx.span_err( + move_from.span, + format!("cannot move out of {}", + bccx.cmt_to_str(move_from))); + } + + mc::cat_downcast(b) | + mc::cat_interior(b, _) => { + match ty::get(b.ty).sty { + ty::ty_struct(did, _) + | ty::ty_enum(did, _) if ty::has_dtor(bccx.tcx, did) => { + bccx.span_err( + move_from.span, + format!("cannot move out of type `{}`, \ + which defines the `Drop` trait", + b.ty.user_string(bccx.tcx))); + }, + _ => fail!("this path should not cause illegal move") + } + } + _ => fail!("this path should not cause illegal move") + } +} + +fn note_move_destination(bccx: &BorrowckCtxt, + move_to_span: codemap::Span, + pat_ident_path: &ast::Path, + is_first_note: bool) { + let pat_name = pprust::path_to_str(pat_ident_path); + if is_first_note { + bccx.span_note( + move_to_span, + format!("attempting to move value to here (to prevent the move, \ + use `ref {0}` or `ref mut {0}` to capture value by \ + reference)", + pat_name)); + } else { + bccx.span_note(move_to_span, + format!("and here (use `ref {0}` or `ref mut {0}`)", + pat_name)); + } +} diff --git a/src/test/compile-fail/borrowck-move-error-with-note.rs b/src/test/compile-fail/borrowck-move-error-with-note.rs new file mode 100644 index 00000000000..ecc9e40c4c2 --- /dev/null +++ b/src/test/compile-fail/borrowck-move-error-with-note.rs @@ -0,0 +1,58 @@ +// 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. + +enum Foo { + Foo1(~u32, ~u32), + Foo2(~u32), + Foo3, +} + +fn blah() { + let f = &Foo1(~1u32, ~2u32); + match *f { //~ ERROR cannot move out of + Foo1(num1, //~ NOTE attempting to move value to here + num2) => (), //~ NOTE and here + Foo2(num) => (), //~ NOTE and here + Foo3 => () + } +} + +struct S {f:~str, g:~str} +impl Drop for S { + fn drop(&mut self) { println!("{}", self.f); } +} + +fn move_in_match() { + match S {f:~"foo", g:~"bar"} { + S { //~ ERROR cannot move out of type `S`, which defines the `Drop` trait + f: _s, //~ NOTE attempting to move value to here + g: _t //~ NOTE and here + } => {} + } +} + +// from issue-8064 +struct A { + a: ~int +} + +fn free<T>(_: T) {} + +fn blah2() { + let a = &A { a: ~1 }; + match a.a { //~ ERROR cannot move out of + n => { //~ NOTE attempting to move value to here + free(n) + } + } + free(a) +} + +fn main() {} diff --git a/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs b/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs index 6724d76d0dc..e834e61efdd 100644 --- a/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs +++ b/src/test/compile-fail/borrowck-move-out-of-vec-tail.rs @@ -25,9 +25,10 @@ pub fn main() { match x { [_, ..tail] => { match tail { - [Foo { string: a }, Foo { string: b }] => { - //~^ ERROR cannot move out of dereference of `&`-pointer - //~^^ ERROR cannot move out of dereference of `&`-pointer + [Foo { string: a }, //~ ERROR cannot move out of dereference of `&`-pointer + Foo { string: b }] => { + //~^^ NOTE attempting to move value to here + //~^^ NOTE and here } _ => { unreachable!(); diff --git a/src/test/compile-fail/borrowck-vec-pattern-nesting.rs b/src/test/compile-fail/borrowck-vec-pattern-nesting.rs index e96ccd2aa8b..7f0a6c84f8c 100644 --- a/src/test/compile-fail/borrowck-vec-pattern-nesting.rs +++ b/src/test/compile-fail/borrowck-vec-pattern-nesting.rs @@ -31,8 +31,8 @@ fn c() { let mut vec = vec!(~1, ~2, ~3); let vec: &mut [~int] = vec.as_mut_slice(); match vec { - [_a, .._b] => { - //~^ ERROR cannot move out + [_a, //~ ERROR cannot move out + .._b] => { //~^ NOTE attempting to move value to here // Note: `_a` is *moved* here, but `b` is borrowing, // hence illegal. @@ -49,9 +49,8 @@ fn d() { let mut vec = vec!(~1, ~2, ~3); let vec: &mut [~int] = vec.as_mut_slice(); match vec { - [.._a, _b] => { - //~^ ERROR cannot move out - } + [.._a, //~ ERROR cannot move out + _b] => {} //~ NOTE attempting to move value to here _ => {} } let a = vec[0]; //~ ERROR cannot move out @@ -62,11 +61,13 @@ fn e() { let vec: &mut [~int] = vec.as_mut_slice(); match vec { [_a, _b, _c] => {} //~ ERROR cannot move out - //~^ ERROR cannot move out - //~^^ ERROR cannot move out + //~^ NOTE attempting to move value to here + //~^^ NOTE and here + //~^^^ NOTE and here _ => {} } let a = vec[0]; //~ ERROR cannot move out + //~^ NOTE attempting to move value to here } fn main() {} diff --git a/src/test/compile-fail/moves-based-on-type-block-bad.rs b/src/test/compile-fail/moves-based-on-type-block-bad.rs index 4172d03d3de..4ec0831c588 100644 --- a/src/test/compile-fail/moves-based-on-type-block-bad.rs +++ b/src/test/compile-fail/moves-based-on-type-block-bad.rs @@ -26,9 +26,9 @@ fn main() { let s = S { x: ~Bar(~42) }; loop { f(&s, |hellothere| { - match hellothere.x { + match hellothere.x { //~ ERROR cannot move out ~Foo(_) => {} - ~Bar(x) => println!("{}", x.to_str()), //~ ERROR cannot move out + ~Bar(x) => println!("{}", x.to_str()), //~ NOTE attempting to move value to here ~Baz => {} } }) |