OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

auto merge of #13418 : ktt3ja/rust/move-out-of, r=brson

Browse Source 
This commit changes the way move errors are reported when some value is
captured by a PatIdent. First, we collect all of the "cannot move out
of" errors before reporting them, and those errors with the same "move
source" are reported together. If the move is caused by a PatIdent (that
binds by value), we add a note indicating where it is and suggest the
user to put `ref` if they don't want the value to move. This makes the
"cannot move out of" error in match expression nicer (though the extra
note may not feel that helpful in other places :P). For example, with
the following code snippet,

```rust
enum Foo {
    Foo1(~u32, ~u32),
    Foo2(~u32),
    Foo3,
}

fn main() {
    let f = &Foo1(~1u32, ~2u32);
    match *f {
        Foo1(num1, num2) => (),
        Foo2(num) => (),
        Foo3 => ()
    }
}
```

Errors before the change:

```rust
test.rs:10:9: 10:25 error: cannot move out of dereference of `&`-pointer
test.rs:10         Foo1(num1, num2) => (),
                   ^~~~~~~~~~~~~~~~
test.rs:10:9: 10:25 error: cannot move out of dereference of `&`-pointer
test.rs:10         Foo1(num1, num2) => (),
                   ^~~~~~~~~~~~~~~~
test.rs:11:9: 11:18 error: cannot move out of dereference of `&`-pointer
test.rs:11         Foo2(num) => (),
                   ^~~~~~~~~
```

After:

```rust
test.rs:9:11: 9:13 error: cannot move out of dereference of `&`-pointer
test.rs:9     match *f {
                    ^~
test.rs:10:14: 10:18 note: attempting to move value to here (to prevent the move, use `ref num1` or `ref mut num1` to capture value by reference)
test.rs:10         Foo1(num1, num2) => (),
                        ^~~~
test.rs:10:20: 10:24 note: and here (use `ref num2` or `ref mut num2`)
test.rs:10         Foo1(num1, num2) => (),
                              ^~~~
test.rs:11:14: 11:17 note: and here (use `ref num` or `ref mut num`)
test.rs:11         Foo2(num) => (),
                        ^~~
```

Close #8064
This commit is contained in:
bors 2014-04-16 13:11:30 -07:00
commit b8d62147aa
7 changed files with 322 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
src/librustc/middle/borrowck/gather_loans/gather_moves.rs
View File

@ -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));
move_info.id, move_info.cmt.repr(bccx.tcx));
if !check_is_legal_to_move_from(bccx, cmt, cmt) {
return;
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)
}
}
}

20
src/librustc/middle/borrowck/gather_loans/mod.rs
View File

@ -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, ());
}
@ -865,7 +870,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);
}
}
}
@ -916,6 +922,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

169
src/librustc/middle/borrowck/gather_loans/move_error.rs Normal file
View File

@ -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));
}
}

58
src/test/compile-fail/borrowck-move-error-with-note.rs Normal file
View File

@ -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() {}

7
src/test/compile-fail/borrowck-move-out-of-vec-tail.rs
View File

@ -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!();

15
src/test/compile-fail/borrowck-vec-pattern-nesting.rs
View File

@ -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() {}

4
src/test/compile-fail/moves-based-on-type-block-bad.rs
View File

@ -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 => {}
}
})