Skip to content

Commit b45648b

Browse files
davidtwcodavidtwco
committed
De-duplicate moved variable errors.
By introducing a new map that tracks the errors reported and the `Place`s that spawned those errors against the move out that the error was referring to, we are able to silence duplicate errors by emitting only the error which corresponds to the most specific `Place` (that which other `Place`s which reported errors are prefixes of). This generally is an improvement, however there is a case - `liveness-move-in-while` - where the output regresses.
1 parent 308d197 commit b45648b

12 files changed

+108
-130
lines changed

Diff for: ‎src/librustc_mir/borrow_check/error_reporting.rs

+23-3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
3838
(place, span): (&Place<'tcx>, Span),
3939
mpi: MovePathIndex,
4040
) {
41+
debug!(
42+
"report_use_of_moved_or_uninitialized: context={:?} desired_action={:?} place={:?} \
43+
span={:?} mpi={:?}",
44+
context, desired_action, place, span, mpi
45+
);
46+
4147
let use_spans = self
4248
.move_spans(place, context.loc)
4349
.or_else(|| self.borrow_spans(span, context.loc));
@@ -49,15 +55,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
4955
if mois.is_empty() {
5056
let root_place = self.prefixes(&place, PrefixSet::All).last().unwrap();
5157

52-
if self.moved_error_reported.contains(&root_place.clone()) {
58+
if self.uninitialized_error_reported.contains(&root_place.clone()) {
5359
debug!(
5460
"report_use_of_moved_or_uninitialized place: error about {:?} suppressed",
5561
root_place
5662
);
5763
return;
5864
}
5965

60-
self.moved_error_reported.insert(root_place.clone());
66+
self.uninitialized_error_reported.insert(root_place.clone());
6167

6268
let item_msg = match self.describe_place_with_options(place, IncludingDowncast(true)) {
6369
Some(name) => format!("`{}`", name),
@@ -80,6 +86,14 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
8086

8187
err.buffer(&mut self.errors_buffer);
8288
} else {
89+
if let Some((reported_place, _)) = self.move_error_reported.get(&mois) {
90+
if self.prefixes(&reported_place, PrefixSet::All).any(|p| p == place) {
91+
debug!("report_use_of_moved_or_uninitialized place: error suppressed \
92+
mois={:?}", mois);
93+
return;
94+
}
95+
}
96+
8397
let msg = ""; //FIXME: add "partially " or "collaterally "
8498

8599
let mut err = self.tcx.cannot_act_on_moved_value(
@@ -167,7 +181,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
167181
}
168182
}
169183

170-
err.buffer(&mut self.errors_buffer);
184+
if let Some((_, mut old_err)) = self.move_error_reported.insert(
185+
mois,
186+
(place.clone(), err)
187+
) {
188+
// Cancel the old error so it doesn't ICE.
189+
old_err.cancel();
190+
}
171191
}
172192
}
173193

Diff for: ‎src/librustc_mir/borrow_check/mod.rs

+23-5
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use rustc::ty::{self, ParamEnv, TyCtxt, Ty};
2727

2828
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, Level};
2929
use rustc_data_structures::graph::dominators::Dominators;
30-
use rustc_data_structures::fx::FxHashSet;
30+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
3131
use rustc_data_structures::indexed_set::IdxSet;
3232
use rustc_data_structures::indexed_vec::Idx;
3333
use smallvec::SmallVec;
@@ -38,6 +38,7 @@ use syntax_pos::Span;
3838

3939
use dataflow::indexes::BorrowIndex;
4040
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex};
41+
use dataflow::move_paths::indexes::MoveOutIndex;
4142
use dataflow::Borrows;
4243
use dataflow::DataflowResultsConsumer;
4344
use dataflow::FlowAtLocation;
@@ -247,7 +248,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
247248
},
248249
access_place_error_reported: FxHashSet(),
249250
reservation_error_reported: FxHashSet(),
250-
moved_error_reported: FxHashSet(),
251+
move_error_reported: FxHashMap(),
252+
uninitialized_error_reported: FxHashSet(),
251253
errors_buffer,
252254
nonlexical_regioncx: regioncx,
253255
used_mut: FxHashSet(),
@@ -325,6 +327,11 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
325327
}
326328
}
327329

330+
// Buffer any move errors that we collected and de-duplicated.
331+
for (_, (_, diag)) in mbcx.move_error_reported.drain() {
332+
diag.buffer(&mut mbcx.errors_buffer);
333+
}
334+
328335
if mbcx.errors_buffer.len() > 0 {
329336
mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());
330337

@@ -400,9 +407,20 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
400407
/// but it is currently inconvenient to track down the BorrowIndex
401408
/// at the time we detect and report a reservation error.
402409
reservation_error_reported: FxHashSet<Place<'tcx>>,
403-
/// This field keeps track of errors reported in the checking of moved variables,
410+
/// This field keeps track of move errors that are to be reported for given move indicies.
411+
///
412+
/// There are situations where many errors can be reported for a single move out (see #53807)
413+
/// and we want only the best of those errors.
414+
///
415+
/// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the
416+
/// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the
417+
/// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once
418+
/// all move errors have been reported, any diagnostics in this map are added to the buffer
419+
/// to be emitted.
420+
move_error_reported: FxHashMap<Vec<MoveOutIndex>, (Place<'tcx>, DiagnosticBuilder<'cx>)>,
421+
/// This field keeps track of errors reported in the checking of uninitialized variables,
404422
/// so that we don't report seemingly duplicate errors.
405-
moved_error_reported: FxHashSet<Place<'tcx>>,
423+
uninitialized_error_reported: FxHashSet<Place<'tcx>>,
406424
/// Errors to be reported buffer
407425
errors_buffer: Vec<Diagnostic>,
408426
/// This field keeps track of all the local variables that are declared mut and are mutated.
@@ -793,7 +811,7 @@ enum LocalMutationIsAllowed {
793811
No,
794812
}
795813

796-
#[derive(Copy, Clone)]
814+
#[derive(Copy, Clone, Debug)]
797815
enum InitializationRequiringAction {
798816
Update,
799817
Borrow,

Diff for: ‎src/test/ui/borrowck/borrowck-multiple-captures.nll.stderr

+8-8
Original file line numberDiff line numberDiff line change
@@ -26,31 +26,31 @@ LL | drop(x2); //~ ERROR cannot move `x2` into closure because it is bor
2626
LL | borrow(&*p2);
2727
| ---- borrow later used here
2828

29-
error[E0382]: use of moved value: `x1`
29+
error[E0382]: use of moved value: `x2`
3030
--> $DIR/borrowck-multiple-captures.rs:35:19
3131
|
32-
LL | drop(x1);
32+
LL | drop(x2);
3333
| -- value moved here
34-
...
3534
LL | thread::spawn(move|| {
3635
| ^^^^^^ value used here after move
3736
LL | drop(x1); //~ ERROR capture of moved value: `x1`
37+
LL | drop(x2); //~ ERROR capture of moved value: `x2`
3838
| -- use occurs due to use in closure
3939
|
40-
= note: move occurs because `x1` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
40+
= note: move occurs because `x2` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
4141

42-
error[E0382]: use of moved value: `x2`
42+
error[E0382]: use of moved value: `x1`
4343
--> $DIR/borrowck-multiple-captures.rs:35:19
4444
|
45-
LL | drop(x2);
45+
LL | drop(x1);
4646
| -- value moved here
47+
...
4748
LL | thread::spawn(move|| {
4849
| ^^^^^^ value used here after move
4950
LL | drop(x1); //~ ERROR capture of moved value: `x1`
50-
LL | drop(x2); //~ ERROR capture of moved value: `x2`
5151
| -- use occurs due to use in closure
5252
|
53-
= note: move occurs because `x2` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
53+
= note: move occurs because `x1` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
5454

5555
error[E0382]: use of moved value: `x`
5656
--> $DIR/borrowck-multiple-captures.rs:46:14

Diff for: ‎src/test/ui/borrowck/issue-41962.rs

-3
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@ pub fn main(){
1818
}
1919
//~^^ ERROR use of partially moved value: `maybe` (Ast) [E0382]
2020
//~| ERROR use of moved value: `(maybe as std::prelude::v1::Some).0` (Ast) [E0382]
21-
//~| ERROR use of moved value: `maybe` (Mir) [E0382]
22-
//~| ERROR use of moved value: `maybe` (Mir) [E0382]
2321
//~| ERROR use of moved value (Mir) [E0382]
24-
//~| ERROR borrow of moved value: `maybe` (Mir) [E0382]
2522
}
2623
}

Diff for: ‎src/test/ui/borrowck/issue-41962.stderr

+1-32
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,6 @@ LL | if let Some(thing) = maybe {
1616
|
1717
= note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
1818

19-
error[E0382]: use of moved value: `maybe` (Mir)
20-
--> $DIR/issue-41962.rs:17:16
21-
|
22-
LL | if let Some(thing) = maybe {
23-
| ^^^^^-----^
24-
| | |
25-
| | value moved here
26-
| value used here after move
27-
|
28-
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
29-
3019
error[E0382]: use of moved value (Mir)
3120
--> $DIR/issue-41962.rs:17:21
3221
|
@@ -35,26 +24,6 @@ LL | if let Some(thing) = maybe {
3524
|
3625
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
3726

38-
error[E0382]: use of moved value: `maybe` (Mir)
39-
--> $DIR/issue-41962.rs:17:30
40-
|
41-
LL | if let Some(thing) = maybe {
42-
| ----- ^^^^^ value used here after move
43-
| |
44-
| value moved here
45-
|
46-
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
47-
48-
error[E0382]: borrow of moved value: `maybe` (Mir)
49-
--> $DIR/issue-41962.rs:17:30
50-
|
51-
LL | if let Some(thing) = maybe {
52-
| ----- ^^^^^ value borrowed here after move
53-
| |
54-
| value moved here
55-
|
56-
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
57-
58-
error: aborting due to 6 previous errors
27+
error: aborting due to 3 previous errors
5928

6029
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/issues/issue-17385.nll.stderr

+1-42
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,3 @@
1-
error[E0382]: use of moved value: `foo`
2-
--> $DIR/issue-17385.rs:28:11
3-
|
4-
LL | drop(foo);
5-
| --- value moved here
6-
LL | match foo { //~ ERROR use of moved value
7-
| ^^^ value used here after move
8-
|
9-
= note: move occurs because `foo` has type `X`, which does not implement the `Copy` trait
10-
11-
error[E0382]: borrow of moved value: `foo`
12-
--> $DIR/issue-17385.rs:28:11
13-
|
14-
LL | drop(foo);
15-
| --- value moved here
16-
LL | match foo { //~ ERROR use of moved value
17-
| ^^^ value borrowed here after move
18-
|
19-
= note: move occurs because `foo` has type `X`, which does not implement the `Copy` trait
20-
211
error[E0382]: use of moved value: `foo.0`
222
--> $DIR/issue-17385.rs:29:11
233
|
@@ -39,27 +19,6 @@ LL | match e { //~ ERROR use of moved value
3919
|
4020
= note: move occurs because `e` has type `Enum`, which does not implement the `Copy` trait
4121

42-
error[E0382]: borrow of moved value: `e`
43-
--> $DIR/issue-17385.rs:35:11
44-
|
45-
LL | drop(e);
46-
| - value moved here
47-
LL | match e { //~ ERROR use of moved value
48-
| ^ value borrowed here after move
49-
|
50-
= note: move occurs because `e` has type `Enum`, which does not implement the `Copy` trait
51-
52-
error[E0382]: use of moved value: `e`
53-
--> $DIR/issue-17385.rs:36:9
54-
|
55-
LL | drop(e);
56-
| - value moved here
57-
LL | match e { //~ ERROR use of moved value
58-
LL | Enum::Variant1 => unreachable!(),
59-
| ^^^^^^^^^^^^^^ value used here after move
60-
|
61-
= note: move occurs because `e` has type `Enum`, which does not implement the `Copy` trait
62-
63-
error: aborting due to 6 previous errors
22+
error: aborting due to 2 previous errors
6423

6524
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/liveness/liveness-move-in-while.nll.stderr

+1-9
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,6 @@ LL | while true { while true { while true { x = y; x.clone(); } } }
88
|
99
= note: move occurs because `y` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
1010

11-
error[E0382]: use of moved value: `y`
12-
--> $DIR/liveness-move-in-while.rs:18:52
13-
|
14-
LL | while true { while true { while true { x = y; x.clone(); } } }
15-
| ^ value moved here in previous iteration of loop
16-
|
17-
= note: move occurs because `y` has type `std::boxed::Box<isize>`, which does not implement the `Copy` trait
18-
19-
error: aborting due to 2 previous errors
11+
error: aborting due to previous error
2012

2113
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/nll/issue-53807.nll.stderr

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
error[E0382]: use of moved value
2+
--> $DIR/issue-53807.rs:14:21
3+
|
4+
LL | if let Some(thing) = maybe {
5+
| ^^^^^ value moved here in previous iteration of loop
6+
|
7+
= note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
8+
9+
error: aborting due to previous error
10+
11+
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/nll/issue-53807.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
pub fn main(){
12+
let maybe = Some(vec![true, true]);
13+
loop {
14+
if let Some(thing) = maybe {
15+
}
16+
}
17+
}

Diff for: ‎src/test/ui/nll/issue-53807.stderr

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
error[E0382]: use of partially moved value: `maybe`
2+
--> $DIR/issue-53807.rs:14:30
3+
|
4+
LL | if let Some(thing) = maybe {
5+
| ----- ^^^^^ value used here after move
6+
| |
7+
| value moved here
8+
|
9+
= note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
10+
11+
error[E0382]: use of moved value: `(maybe as std::prelude::v1::Some).0`
12+
--> $DIR/issue-53807.rs:14:21
13+
|
14+
LL | if let Some(thing) = maybe {
15+
| ^^^^^ value moved here in previous iteration of loop
16+
|
17+
= note: move occurs because the value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
18+
19+
error: aborting due to 2 previous errors
20+
21+
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/no-capture-arc.nll.stderr

+1-14
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,6 @@ LL | assert_eq!((*arc_v)[2], 3);
1111
|
1212
= note: move occurs because `arc_v` has type `std::sync::Arc<std::vec::Vec<i32>>`, which does not implement the `Copy` trait
1313

14-
error[E0382]: borrow of moved value: `arc_v`
15-
--> $DIR/no-capture-arc.rs:26:23
16-
|
17-
LL | thread::spawn(move|| {
18-
| ------ value moved into closure here
19-
LL | assert_eq!((*arc_v)[3], 4);
20-
| ----- variable moved due to use in closure
21-
...
22-
LL | println!("{:?}", *arc_v);
23-
| ^^^^^ value borrowed here after move
24-
|
25-
= note: move occurs because `arc_v` has type `std::sync::Arc<std::vec::Vec<i32>>`, which does not implement the `Copy` trait
26-
27-
error: aborting due to 2 previous errors
14+
error: aborting due to previous error
2815

2916
For more information about this error, try `rustc --explain E0382`.

Diff for: ‎src/test/ui/no-reuse-move-arc.nll.stderr

+1-14
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,6 @@ LL | assert_eq!((*arc_v)[2], 3); //~ ERROR use of moved value: `arc_v`
1111
|
1212
= note: move occurs because `arc_v` has type `std::sync::Arc<std::vec::Vec<i32>>`, which does not implement the `Copy` trait
1313

14-
error[E0382]: borrow of moved value: `arc_v`
15-
--> $DIR/no-reuse-move-arc.rs:24:23
16-
|
17-
LL | thread::spawn(move|| {
18-
| ------ value moved into closure here
19-
LL | assert_eq!((*arc_v)[3], 4);
20-
| ----- variable moved due to use in closure
21-
...
22-
LL | println!("{:?}", *arc_v); //~ ERROR use of moved value: `arc_v`
23-
| ^^^^^ value borrowed here after move
24-
|
25-
= note: move occurs because `arc_v` has type `std::sync::Arc<std::vec::Vec<i32>>`, which does not implement the `Copy` trait
26-
27-
error: aborting due to 2 previous errors
14+
error: aborting due to previous error
2815

2916
For more information about this error, try `rustc --explain E0382`.

0 commit comments

Comments
 (0)