Skip to content

Commit b15a146

Browse files
Mark-SimulacrumMark-Simulacrum
authored
Rollup merge of rust-lang#53995 - davidtwco:issue-53807, r=nikomatsakis
Too many errors for incorrect move in loop with NLL enabled Fixes rust-lang#53807. r? @nikomatsakis
2 parents 9a442e7 + b45648b commit b15a146

16 files changed

+115
-130
lines changed

Diff for: ‎src/Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -2173,6 +2173,7 @@ name = "rustc_errors"
21732173
version = "0.0.0"
21742174
dependencies = [
21752175
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
2176+
"log 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
21762177
"rustc_cratesio_shim 0.0.0",
21772178
"rustc_data_structures 0.0.0",
21782179
"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
@@ -98,6 +98,9 @@ impl<'a> DiagnosticBuilder<'a> {
9898
diagnostic = ::std::ptr::read(&self.diagnostic);
9999
::std::mem::forget(self);
100100
};
101+
// Logging here is useful to help track down where in logs an error was
102+
// actually emitted.
103+
debug!("buffer: diagnostic={:?}", diagnostic);
101104
buffered_diagnostics.push(diagnostic);
102105
}
103106

Diff for: ‎src/librustc_errors/lib.rs

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

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

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

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

59-
self.moved_error_reported.insert(root_place.clone());
65+
self.uninitialized_error_reported.insert(root_place.clone());
6066

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

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

8498
let mut err = self.tcx.cannot_act_on_moved_value(
@@ -166,7 +180,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
166180
}
167181
}
168182

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

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`.

0 commit comments

Comments
 (0)