From 02c6ebde7e9bca72baa0623d9c5f6a86a30ec486 Mon Sep 17 00:00:00 2001 From: P1start Date: Sat, 13 Sep 2014 14:03:34 +1200 Subject: [PATCH] Change the `use of moved value` error to be more accurate Previously it output `partially moved` to eagerly. This updates it to be more accurate and output `collaterally moved` for use of values that were invalidated by moves out of different fields in the same struct. Closes #15630. --- src/librustc/middle/borrowck/mod.rs | 59 +++++++++++++++---- .../borrowck-box-insensitivity.rs | 15 +++-- .../borrowck-field-sensitivity.rs | 4 +- .../compile-fail/liveness-use-after-move.rs | 2 +- .../use-after-move-self-based-on-type.rs | 2 +- src/test/compile-fail/use-after-move-self.rs | 2 +- 6 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 734108d035a..b411620dac0 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -461,24 +461,58 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { MovedInCapture => "capture", }; - match the_move.kind { + let (ol, moved_lp_msg) = match the_move.kind { move_data::Declared => { self.tcx.sess.span_err( use_span, format!("{} of possibly uninitialized variable: `{}`", verb, self.loan_path_to_string(lp)).as_slice()); + (self.loan_path_to_string(moved_lp), + String::new()) } _ => { - let partially = if lp == moved_lp {""} else {"partially "}; + // If moved_lp is something like `x.a`, and lp is something like `x.b`, we would + // normally generate a rather confusing message: + // + // error: use of moved value: `x.b` + // note: `x.a` moved here... + // + // What we want to do instead is get the 'common ancestor' of the two moves and + // use that for most of the message instead, giving is something like this: + // + // error: use of moved value: `x` + // note: `x` moved here (through moving `x.a`)... + + let common = moved_lp.common(lp); + let has_common = common.is_some(); + let has_fork = moved_lp.has_fork(lp); + let (nl, ol, moved_lp_msg) = + if has_fork && has_common { + let nl = self.loan_path_to_string(&common.unwrap()); + let ol = nl.clone(); + let moved_lp_msg = format!(" (through moving `{}`)", + self.loan_path_to_string(moved_lp)); + (nl, ol, moved_lp_msg) + } else { + (self.loan_path_to_string(lp), + self.loan_path_to_string(moved_lp), + String::new()) + }; + + let partial = moved_lp.depth() > lp.depth(); + let msg = if !has_fork && partial { "partially " } + else if has_fork && !has_common { "collaterally "} + else { "" }; self.tcx.sess.span_err( use_span, format!("{} of {}moved value: `{}`", verb, - partially, - self.loan_path_to_string(lp)).as_slice()); + msg, + nl).as_slice()); + (ol, moved_lp_msg) } - } + }; match the_move.kind { move_data::Declared => {} @@ -501,8 +535,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { "moved by default (use `copy` to override)"); self.tcx.sess.span_note( expr_span, - format!("`{}` moved here because it has type `{}`, which is {}", - self.loan_path_to_string(moved_lp), + format!("`{}` moved here{} because it has type `{}`, which is {}", + ol, + moved_lp_msg, expr_ty.user_string(self.tcx), suggestion).as_slice()); } @@ -510,10 +545,11 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { move_data::MovePat => { let pat_ty = ty::node_id_to_type(self.tcx, the_move.id); self.tcx.sess.span_note(self.tcx.map.span(the_move.id), - format!("`{}` moved here because it has type `{}`, \ + format!("`{}` moved here{} because it has type `{}`, \ which is moved by default (use `ref` to \ override)", - self.loan_path_to_string(moved_lp), + ol, + moved_lp_msg, pat_ty.user_string(self.tcx)).as_slice()); } @@ -536,9 +572,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { capture that instead to override)"); self.tcx.sess.span_note( expr_span, - format!("`{}` moved into closure environment here because it \ + format!("`{}` moved into closure environment here{} because it \ has type `{}`, which is {}", - self.loan_path_to_string(moved_lp), + ol, + moved_lp_msg, expr_ty.user_string(self.tcx), suggestion).as_slice()); } diff --git a/src/test/compile-fail/borrowck-box-insensitivity.rs b/src/test/compile-fail/borrowck-box-insensitivity.rs index c9b384e0b00..d05c03547ac 100644 --- a/src/test/compile-fail/borrowck-box-insensitivity.rs +++ b/src/test/compile-fail/borrowck-box-insensitivity.rs @@ -31,19 +31,22 @@ struct D { fn copy_after_move() { let a = box A { x: box 0, y: 1 }; let _x = a.x; - let _y = a.y; //~ ERROR use of partially moved + let _y = a.y; //~ ERROR use of moved + //~^^ NOTE `a` moved here (through moving `a.x`) } fn move_after_move() { let a = box B { x: box 0, y: box 1 }; let _x = a.x; - let _y = a.y; //~ ERROR use of partially moved + let _y = a.y; //~ ERROR use of moved + //~^^ NOTE `a` moved here (through moving `a.x`) } fn borrow_after_move() { let a = box A { x: box 0, y: 1 }; let _x = a.x; - let _y = &a.y; //~ ERROR use of partially moved + let _y = &a.y; //~ ERROR use of moved + //~^^ NOTE `a` moved here (through moving `a.x`) } fn move_after_borrow() { @@ -79,19 +82,19 @@ fn mut_borrow_after_borrow() { fn copy_after_move_nested() { let a = box C { x: box A { x: box 0, y: 1 }, y: 2 }; let _x = a.x.x; - let _y = a.y; //~ ERROR use of partially moved + let _y = a.y; //~ ERROR use of collaterally moved } fn move_after_move_nested() { let a = box D { x: box A { x: box 0, y: 1 }, y: box 2 }; let _x = a.x.x; - let _y = a.y; //~ ERROR use of partially moved + let _y = a.y; //~ ERROR use of collaterally moved } fn borrow_after_move_nested() { let a = box C { x: box A { x: box 0, y: 1 }, y: 2 }; let _x = a.x.x; - let _y = &a.y; //~ ERROR use of partially moved + let _y = &a.y; //~ ERROR use of collaterally moved } fn move_after_borrow_nested() { diff --git a/src/test/compile-fail/borrowck-field-sensitivity.rs b/src/test/compile-fail/borrowck-field-sensitivity.rs index 72042b8373d..49c93e3aa9e 100644 --- a/src/test/compile-fail/borrowck-field-sensitivity.rs +++ b/src/test/compile-fail/borrowck-field-sensitivity.rs @@ -13,13 +13,13 @@ struct A { a: int, b: Box } fn deref_after_move() { let x = A { a: 1, b: box 2 }; drop(x.b); - drop(*x.b); //~ ERROR use of partially moved value: `*x.b` + drop(*x.b); //~ ERROR use of moved value: `*x.b` } fn deref_after_fu_move() { let x = A { a: 1, b: box 2 }; let y = A { a: 3, .. x }; - drop(*x.b); //~ ERROR use of partially moved value: `*x.b` + drop(*x.b); //~ ERROR use of moved value: `*x.b` } fn borrow_after_move() { diff --git a/src/test/compile-fail/liveness-use-after-move.rs b/src/test/compile-fail/liveness-use-after-move.rs index 0b8e65fee08..cd7401a65ae 100644 --- a/src/test/compile-fail/liveness-use-after-move.rs +++ b/src/test/compile-fail/liveness-use-after-move.rs @@ -13,6 +13,6 @@ extern crate debug; fn main() { let x = box 5i; let y = x; - println!("{:?}", *x); //~ ERROR use of partially moved value: `*x` + println!("{:?}", *x); //~ ERROR use of moved value: `*x` y.clone(); } diff --git a/src/test/compile-fail/use-after-move-self-based-on-type.rs b/src/test/compile-fail/use-after-move-self-based-on-type.rs index b11650a6a4f..a1b7f83da2f 100644 --- a/src/test/compile-fail/use-after-move-self-based-on-type.rs +++ b/src/test/compile-fail/use-after-move-self-based-on-type.rs @@ -19,7 +19,7 @@ impl Drop for S { impl S { pub fn foo(self) -> int { self.bar(); - return self.x; //~ ERROR use of partially moved value: `self.x` + return self.x; //~ ERROR use of moved value: `self.x` } pub fn bar(self) {} diff --git a/src/test/compile-fail/use-after-move-self.rs b/src/test/compile-fail/use-after-move-self.rs index 22c3ec7c341..607d6163208 100644 --- a/src/test/compile-fail/use-after-move-self.rs +++ b/src/test/compile-fail/use-after-move-self.rs @@ -16,7 +16,7 @@ struct S { impl S { pub fn foo(self) -> int { self.bar(); - return *self.x; //~ ERROR use of partially moved value: `*self.x` + return *self.x; //~ ERROR use of moved value: `*self.x` } pub fn bar(self) {}