Skip to content

Commit

Permalink
Rollup merge of rust-lang#52907 - pnkfelix:issue-52877-original-sourc…
Browse files Browse the repository at this point in the history
…e-should-precede-suggestions, r=petrochenkov

NLL: On "cannot move out of type" error, print original before rewrite

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:
```rust
    /// 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.

Fix rust-lang#52877
  • Loading branch information
Mark-Simulacrum authored Aug 1, 2018
2 parents cf6db38 + a5a3f76 commit afbd9ae
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 44 deletions.
17 changes: 11 additions & 6 deletions src/librustc_mir/borrow_check/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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(
Expand All @@ -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 { .. } => (),
Expand Down
10 changes: 6 additions & 4 deletions src/test/ui/borrowck/borrowck-move-error-with-note.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions src/test/ui/borrowck/borrowck-move-out-of-vec-tail.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
27 changes: 15 additions & 12 deletions src/test/ui/borrowck/borrowck-vec-pattern-nesting.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ error[E0508]: cannot move out of type `[std::boxed::Box<isize>]`, 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<isize>]`, a non-copy slice
--> $DIR/borrowck-vec-pattern-nesting.rs:57:13
Expand All @@ -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<isize>]`, a non-copy slice
--> $DIR/borrowck-vec-pattern-nesting.rs:70:13
Expand All @@ -62,18 +68,15 @@ error[E0508]: cannot move out of type `[std::boxed::Box<isize>]`, 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<isize>]`, a non-copy slice
--> $DIR/borrowck-vec-pattern-nesting.rs:82:13
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/borrowck/issue-51415.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion src/test/ui/codemap_tests/overlapping_spans.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
28 changes: 20 additions & 8 deletions src/test/ui/issue-12567.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
29 changes: 20 additions & 9 deletions src/test/ui/nll/move-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit afbd9ae

Please sign in to comment.