Skip to content

Commit 8f37677

Browse files
borsbors
committed
Auto merge of #53995 - davidtwco:issue-53807, r=nikomatsakis
NLL: Deduplicate errors for incorrect move in loop Fixes #53807. r? @nikomatsakis
2 parents 79fcc58 + 88ca341 commit 8f37677

15 files changed

+111
-121
lines changed

Diff for: ‎src/Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -2183,6 +2183,7 @@ name = "rustc_errors"
21832183
version = "0.0.0"
21842184
dependencies = [
21852185
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
2186+
"log 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
21862187
"rustc_cratesio_shim 0.0.0",
21872188
"rustc_data_structures 0.0.0",
21882189
"serialize 0.0.0",

Diff for: ‎src/librustc_errors/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ path = "lib.rs"
99
crate-type = ["dylib"]
1010

1111
[dependencies]
12+
log = "0.4"
1213
serialize = { path = "../libserialize" }
1314
syntax_pos = { path = "../libsyntax_pos" }
1415
rustc_data_structures = { path = "../librustc_data_structures" }

Diff for: ‎src/librustc_errors/diagnostic_builder.rs

+3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ impl<'a> DiagnosticBuilder<'a> {
108108
diagnostic = ::std::ptr::read(&self.diagnostic);
109109
::std::mem::forget(self);
110110
};
111+
// Logging here is useful to help track down where in logs an error was
112+
// actually emitted.
113+
debug!("buffer: diagnostic={:?}", diagnostic);
111114
buffered_diagnostics.push(diagnostic);
112115
}
113116

Diff for: ‎src/librustc_errors/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ extern crate atty;
2323
extern crate termcolor;
2424
#[cfg(unix)]
2525
extern crate libc;
26+
#[macro_use]
27+
extern crate log;
2628
extern crate rustc_data_structures;
2729
extern crate serialize as rustc_serialize;
2830
extern crate syntax_pos;

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

+27-4
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@ use rustc_data_structures::indexed_vec::Idx;
3333
use smallvec::SmallVec;
3434

3535
use std::rc::Rc;
36+
use std::collections::BTreeMap;
3637

3738
use syntax_pos::Span;
3839

3940
use dataflow::indexes::BorrowIndex;
4041
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex};
42+
use dataflow::move_paths::indexes::MoveOutIndex;
4143
use dataflow::Borrows;
4244
use dataflow::DataflowResultsConsumer;
4345
use dataflow::FlowAtLocation;
@@ -255,7 +257,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
255257
locals_are_invalidated_at_exit,
256258
access_place_error_reported: FxHashSet(),
257259
reservation_error_reported: FxHashSet(),
258-
moved_error_reported: FxHashSet(),
260+
move_error_reported: BTreeMap::new(),
261+
uninitialized_error_reported: FxHashSet(),
259262
errors_buffer,
260263
nonlexical_regioncx: regioncx,
261264
used_mut: FxHashSet(),
@@ -333,6 +336,11 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
333336
}
334337
}
335338

339+
// Buffer any move errors that we collected and de-duplicated.
340+
for (_, (_, diag)) in mbcx.move_error_reported {
341+
diag.buffer(&mut mbcx.errors_buffer);
342+
}
343+
336344
if mbcx.errors_buffer.len() > 0 {
337345
mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());
338346

@@ -408,9 +416,24 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
408416
/// but it is currently inconvenient to track down the BorrowIndex
409417
/// at the time we detect and report a reservation error.
410418
reservation_error_reported: FxHashSet<Place<'tcx>>,
411-
/// This field keeps track of errors reported in the checking of moved variables,
419+
/// This field keeps track of move errors that are to be reported for given move indicies.
420+
///
421+
/// There are situations where many errors can be reported for a single move out (see #53807)
422+
/// and we want only the best of those errors.
423+
///
424+
/// The `report_use_of_moved_or_uninitialized` function checks this map and replaces the
425+
/// diagnostic (if there is one) if the `Place` of the error being reported is a prefix of the
426+
/// `Place` of the previous most diagnostic. This happens instead of buffering the error. Once
427+
/// all move errors have been reported, any diagnostics in this map are added to the buffer
428+
/// to be emitted.
429+
///
430+
/// `BTreeMap` is used to preserve the order of insertions when iterating. This is necessary
431+
/// when errors in the map are being re-added to the error buffer so that errors with the
432+
/// same primary span come out in a consistent order.
433+
move_error_reported: BTreeMap<Vec<MoveOutIndex>, (Place<'tcx>, DiagnosticBuilder<'cx>)>,
434+
/// This field keeps track of errors reported in the checking of uninitialized variables,
412435
/// so that we don't report seemingly duplicate errors.
413-
moved_error_reported: FxHashSet<Place<'tcx>>,
436+
uninitialized_error_reported: FxHashSet<Place<'tcx>>,
414437
/// Errors to be reported buffer
415438
errors_buffer: Vec<Diagnostic>,
416439
/// This field keeps track of all the local variables that are declared mut and are mutated.
@@ -801,7 +824,7 @@ enum LocalMutationIsAllowed {
801824
No,
802825
}
803826

804-
#[derive(Copy, Clone)]
827+
#[derive(Copy, Clone, Debug)]
805828
enum InitializationRequiringAction {
806829
Update,
807830
Borrow,

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)