From a5a3f765eb36aee69bfdaba1f82760973b63c780 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 31 Jul 2018 17:22:12 +0200 Subject: [PATCH] NLL: On "cannot move out of type" error, print original source before rewrite. * Arguably this change is sometimes injecting noise into the output (namely in the cases where the suggested rewrite is inline with the suggestion and we end up highlighting the original source code). I would not be opposed to something more aggressive/dynamic, like revising the suggestion code to automatically print the original source when necessary (e.g. when the error does not have a span that includes the span of the suggestion). * Also, as another note on this change: The doc comment for `Diagnostic::span_suggestion` says: /// The message /// /// * should not end in any punctuation (a `:` is added automatically) /// * should not be a question /// * should not contain any parts like "the following", "as shown" but the `:` is *not* added when the emitted line appears out-of-line relative to the suggestion. I find that to be an unfortunate UI experience. ---- As a drive-by fix, also changed code to combine multiple suggestions for a pattern into a single multipart suggestion (which vastly improves user experience IMO). ---- Includes the updates to expected NLL diagnostics. --- src/librustc_mir/borrow_check/move_errors.rs | 17 +++++++---- .../borrowck-move-error-with-note.nll.stderr | 10 ++++--- .../borrowck-move-out-of-vec-tail.nll.stderr | 13 ++++++--- .../borrowck-vec-pattern-nesting.nll.stderr | 27 +++++++++-------- src/test/ui/borrowck/issue-51415.nll.stderr | 1 + .../overlapping_spans.nll.stderr | 5 +++- src/test/ui/issue-12567.nll.stderr | 28 +++++++++++++----- src/test/ui/nll/move-errors.stderr | 29 +++++++++++++------ 8 files changed, 86 insertions(+), 44 deletions(-) diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index 103f431d4bac9..4d988fef450b8 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -341,7 +341,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { // another match arm binds_to.sort(); binds_to.dedup(); - for local in binds_to { + let mut multipart_suggestion = Vec::with_capacity(binds_to.len()); + for (j, local) in binds_to.into_iter().enumerate() { let bind_to = &self.mir.local_decls[local]; let binding_span = bind_to.source_info.span; @@ -350,13 +351,15 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { Mutability::Not => "ref", Mutability::Mut => "ref mut", }; + if j == 0 { + err.span_label(binding_span, format!("data moved here")); + } else { + err.span_label(binding_span, format!("... and here")); + } match bind_to.name { Some(name) => { - err.span_suggestion( - binding_span, - "to prevent move, use ref or ref mut", - format!("{} {:?}", ref_kind, name), - ); + multipart_suggestion.push((binding_span, + format!("{} {}", ref_kind, name))); } None => { err.span_label( @@ -366,6 +369,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } } } + err.multipart_suggestion("to prevent move, use ref or ref mut", + multipart_suggestion); } // Nothing to suggest. GroupedMoveError::OtherIllegalMove { .. } => (), diff --git a/src/test/ui/borrowck/borrowck-move-error-with-note.nll.stderr b/src/test/ui/borrowck/borrowck-move-error-with-note.nll.stderr index a34c97974da11..1b913471924b7 100644 --- a/src/test/ui/borrowck/borrowck-move-error-with-note.nll.stderr +++ b/src/test/ui/borrowck/borrowck-move-error-with-note.nll.stderr @@ -19,14 +19,16 @@ error[E0509]: cannot move out of type `S`, which implements the `Drop` trait | LL | match (S {f: "foo".to_string(), g: "bar".to_string()}) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot move out of here +... +LL | f: _s, + | -- data moved here +LL | g: _t + | -- ... and here help: to prevent move, use ref or ref mut | LL | f: ref _s, - | ^^^^^^ -help: to prevent move, use ref or ref mut - | LL | g: ref _t - | ^^^^^^ + | error[E0507]: cannot move out of borrowed content --> $DIR/borrowck-move-error-with-note.rs:57:11 diff --git a/src/test/ui/borrowck/borrowck-move-out-of-vec-tail.nll.stderr b/src/test/ui/borrowck/borrowck-move-out-of-vec-tail.nll.stderr index d01b24507d9fe..95a7894d53254 100644 --- a/src/test/ui/borrowck/borrowck-move-out-of-vec-tail.nll.stderr +++ b/src/test/ui/borrowck/borrowck-move-out-of-vec-tail.nll.stderr @@ -3,14 +3,19 @@ error[E0508]: cannot move out of type `[Foo]`, a non-copy slice | LL | match tail { | ^^^^ cannot move out of here +LL | &[Foo { string: a }, + | - data moved here +... +LL | Foo { string: b }] => { + | - ... and here help: to prevent move, use ref or ref mut | LL | &[Foo { string: ref a }, - | ^^^^^ -help: to prevent move, use ref or ref mut - | +LL | //~^ ERROR cannot move out of type `[Foo]` +LL | //~| cannot move out +LL | //~| to prevent move LL | Foo { string: ref b }] => { - | ^^^^^ + | error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-vec-pattern-nesting.nll.stderr b/src/test/ui/borrowck/borrowck-vec-pattern-nesting.nll.stderr index 50ef3ba40e7b2..2779132590e2c 100644 --- a/src/test/ui/borrowck/borrowck-vec-pattern-nesting.nll.stderr +++ b/src/test/ui/borrowck/borrowck-vec-pattern-nesting.nll.stderr @@ -28,7 +28,10 @@ error[E0508]: cannot move out of type `[std::boxed::Box]`, a non-copy sli LL | match vec { | ^^^ cannot move out of here LL | &mut [_a, //~ ERROR cannot move out - | -- help: to prevent move, use ref or ref mut: `ref _a` + | -- + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref _a` error[E0508]: cannot move out of type `[std::boxed::Box]`, a non-copy slice --> $DIR/borrowck-vec-pattern-nesting.rs:57:13 @@ -46,7 +49,10 @@ LL | match vec { | ^^^ cannot move out of here ... LL | _b] => {} - | -- help: to prevent move, use ref or ref mut: `ref _b` + | -- + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref _b` error[E0508]: cannot move out of type `[std::boxed::Box]`, a non-copy slice --> $DIR/borrowck-vec-pattern-nesting.rs:70:13 @@ -62,18 +68,15 @@ error[E0508]: cannot move out of type `[std::boxed::Box]`, a non-copy sli | LL | match vec { | ^^^ cannot move out of here +LL | &mut [_a, _b, _c] => {} //~ ERROR cannot move out + | -- -- -- ... and here + | | | + | | ... and here + | data moved here help: to prevent move, use ref or ref mut | -LL | &mut [ref _a, _b, _c] => {} //~ ERROR cannot move out - | ^^^^^^ -help: to prevent move, use ref or ref mut - | -LL | &mut [_a, ref _b, _c] => {} //~ ERROR cannot move out - | ^^^^^^ -help: to prevent move, use ref or ref mut - | -LL | &mut [_a, _b, ref _c] => {} //~ ERROR cannot move out - | ^^^^^^ +LL | &mut [ref _a, ref _b, ref _c] => {} //~ ERROR cannot move out + | ^^^^^^ ^^^^^^ ^^^^^^ error[E0508]: cannot move out of type `[std::boxed::Box]`, a non-copy slice --> $DIR/borrowck-vec-pattern-nesting.rs:82:13 diff --git a/src/test/ui/borrowck/issue-51415.nll.stderr b/src/test/ui/borrowck/issue-51415.nll.stderr index d872c7efe2bc1..20713c3392e8e 100644 --- a/src/test/ui/borrowck/issue-51415.nll.stderr +++ b/src/test/ui/borrowck/issue-51415.nll.stderr @@ -4,6 +4,7 @@ error[E0507]: cannot move out of borrowed content LL | let opt = a.iter().enumerate().find(|(_, &s)| { | ^^^^^-^ | | | + | | data moved here | | help: to prevent move, use ref or ref mut: `ref s` | cannot move out of borrowed content diff --git a/src/test/ui/codemap_tests/overlapping_spans.nll.stderr b/src/test/ui/codemap_tests/overlapping_spans.nll.stderr index 34616a8de45ed..a1fbcf1430db9 100644 --- a/src/test/ui/codemap_tests/overlapping_spans.nll.stderr +++ b/src/test/ui/codemap_tests/overlapping_spans.nll.stderr @@ -4,7 +4,10 @@ error[E0509]: cannot move out of type `S`, which implements the `Drop` trait LL | match (S {f:"foo".to_string()}) { | ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot move out of here LL | S {f:_s} => {} //~ ERROR cannot move out - | -- help: to prevent move, use ref or ref mut: `ref _s` + | -- + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref _s` error: aborting due to previous error diff --git a/src/test/ui/issue-12567.nll.stderr b/src/test/ui/issue-12567.nll.stderr index a040f7c3c8d1b..29bda252b9115 100644 --- a/src/test/ui/issue-12567.nll.stderr +++ b/src/test/ui/issue-12567.nll.stderr @@ -3,28 +3,40 @@ error[E0508]: cannot move out of type `[T]`, a non-copy slice | LL | match (l1, l2) { | ^^^^^^^^ cannot move out of here +LL | (&[], &[]) => println!("both empty"), +LL | (&[], &[hd, ..]) | (&[hd, ..], &[]) + | -- data moved here +... +LL | (&[hd1, ..], &[hd2, ..]) + | --- ... and here help: to prevent move, use ref or ref mut | LL | (&[], &[ref hd, ..]) | (&[hd, ..], &[]) - | ^^^^^^ -help: to prevent move, use ref or ref mut - | +LL | => println!("one empty"), +LL | //~^^ ERROR: cannot move out of type `[T]`, a non-copy slice +LL | //~^^^ ERROR: cannot move out of type `[T]`, a non-copy slice LL | (&[hd1, ..], &[ref hd2, ..]) - | ^^^^^^^ + | error[E0508]: cannot move out of type `[T]`, a non-copy slice --> $DIR/issue-12567.rs:14:11 | LL | match (l1, l2) { | ^^^^^^^^ cannot move out of here +LL | (&[], &[]) => println!("both empty"), +LL | (&[], &[hd, ..]) | (&[hd, ..], &[]) + | -- data moved here +... +LL | (&[hd1, ..], &[hd2, ..]) + | --- ... and here help: to prevent move, use ref or ref mut | LL | (&[], &[ref hd, ..]) | (&[hd, ..], &[]) - | ^^^^^^ -help: to prevent move, use ref or ref mut - | +LL | => println!("one empty"), +LL | //~^^ ERROR: cannot move out of type `[T]`, a non-copy slice +LL | //~^^^ ERROR: cannot move out of type `[T]`, a non-copy slice LL | (&[ref hd1, ..], &[hd2, ..]) - | ^^^^^^^ + | error: aborting due to 2 previous errors diff --git a/src/test/ui/nll/move-errors.stderr b/src/test/ui/nll/move-errors.stderr index 3f2c651ae3a6a..53d60d3f6d649 100644 --- a/src/test/ui/nll/move-errors.stderr +++ b/src/test/ui/nll/move-errors.stderr @@ -59,6 +59,7 @@ error[E0509]: cannot move out of type `D`, which implements the `Drop` trait LL | let C(D(s)) = c; | - ^ cannot move out of here | | + | data moved here | help: to prevent move, use ref or ref mut: `ref s` error[E0507]: cannot move out of borrowed content @@ -88,7 +89,10 @@ LL | match x { | ^ cannot move out of here ... LL | B::U(D(s)) => (), - | - help: to prevent move, use ref or ref mut: `ref s` + | - + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref s` error[E0509]: cannot move out of type `D`, which implements the `Drop` trait --> $DIR/move-errors.rs:105:11 @@ -97,7 +101,10 @@ LL | match x { | ^ cannot move out of here ... LL | (D(s), &t) => (), - | - help: to prevent move, use ref or ref mut: `ref s` + | - + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref s` error[E0507]: cannot move out of borrowed content --> $DIR/move-errors.rs:105:11 @@ -106,21 +113,25 @@ LL | match x { | ^ cannot move out of borrowed content ... LL | (D(s), &t) => (), - | - help: to prevent move, use ref or ref mut: `ref t` + | - + | | + | data moved here + | help: to prevent move, use ref or ref mut: `ref t` error[E0509]: cannot move out of type `F`, which implements the `Drop` trait --> $DIR/move-errors.rs:115:11 | LL | match x { | ^ cannot move out of here +LL | //~^ ERROR +LL | F(s, mut t) => (), + | - ----- ... and here + | | + | data moved here help: to prevent move, use ref or ref mut | -LL | F(ref s, mut t) => (), - | ^^^^^ -help: to prevent move, use ref or ref mut - | -LL | F(s, ref mut t) => (), - | ^^^^^^^^^ +LL | F(ref s, ref mut t) => (), + | ^^^^^ ^^^^^^^^^ error[E0507]: cannot move out of borrowed content --> $DIR/move-errors.rs:123:11