diff options
| -rw-r--r-- | src/librustc_mir/borrow_check/error_reporting.rs | 17 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/mod.rs | 92 | ||||
| -rw-r--r-- | src/test/compile-fail/borrowck/borrowck-issue-48962.rs | 38 | ||||
| -rw-r--r-- | src/test/run-pass/issue-48962.rs | 43 |
4 files changed, 167 insertions, 23 deletions
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 56371d809b2..aaed1dd871b 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -37,6 +37,23 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .collect::<Vec<_>>(); if mois.is_empty() { + let root_place = self.prefixes(&place, PrefixSet::All) + .last() + .unwrap(); + + if self.moved_error_reported + .contains(&root_place.clone()) + { + debug!( + "report_use_of_moved_or_uninitialized place: error about {:?} suppressed", + root_place + ); + return; + } + + self.moved_error_reported + .insert(root_place.clone()); + let item_msg = match self.describe_place(place) { Some(name) => format!("`{}`", name), None => "value".to_owned(), diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 305df37466d..fe7aedb4127 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -239,6 +239,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( }, access_place_error_reported: FxHashSet(), reservation_error_reported: FxHashSet(), + moved_error_reported: FxHashSet(), nonlexical_regioncx: opt_regioncx, nonlexical_cause_info: None, }; @@ -285,6 +286,9 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> { /// but it is currently inconvenient to track down the BorrowIndex /// at the time we detect and report a reservation error. reservation_error_reported: FxHashSet<Place<'tcx>>, + /// This field keeps track of errors reported in the checking of moved variables, + /// so that we don't report report seemingly duplicate errors. + moved_error_reported: FxHashSet<Place<'tcx>>, /// Non-lexical region inference context, if NLL is enabled. This /// contains the results from region inference and lets us e.g. /// find out which CFG points are contained in each borrow region. @@ -368,7 +372,7 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx LocalMutationIsAllowed::No, flow_state, ); - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Use, (output, span), @@ -965,7 +969,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // Write of P[i] or *P, or WriteAndRead of any P, requires P init'd. match mode { MutateMode::WriteAndRead => { - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Update, place_span, @@ -1025,7 +1029,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { flow_state, ); - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Borrow, (place, span), @@ -1053,7 +1057,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { LocalMutationIsAllowed::No, flow_state, ); - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Use, (place, span), @@ -1100,7 +1104,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); // Finally, check if path was already moved. - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Use, (place, span), @@ -1118,7 +1122,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); // Finally, check if path was already moved. - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Use, (place, span), @@ -1269,7 +1273,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { LocalMutationIsAllowed::No, flow_state, ); - // We do not need to call `check_if_path_is_moved` + // We do not need to call `check_if_path_or_subpath_is_moved` // again, as we already called it when we made the // initial reservation. } @@ -1304,7 +1308,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { } } - fn check_if_path_is_moved( + fn check_if_full_path_is_moved( &mut self, context: Context, desired_action: InitializationRequiringAction, @@ -1322,18 +1326,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // // 1. Move of `a.b.c`, use of `a.b.c` // 2. Move of `a.b.c`, use of `a.b.c.d` (without first reinitializing `a.b.c.d`) - // 3. Move of `a.b.c`, use of `a` or `a.b` - // 4. Uninitialized `(a.b.c: &_)`, use of `*a.b.c`; note that with + // 3. Uninitialized `(a.b.c: &_)`, use of `*a.b.c`; note that with // partial initialization support, one might have `a.x` // initialized but not `a.b`. // // OK scenarios: // - // 5. Move of `a.b.c`, use of `a.b.d` - // 6. Uninitialized `a.x`, initialized `a.b`, use of `a.b` - // 7. Copied `(a.b: &_)`, use of `*(a.b).c`; note that `a.b` + // 4. Move of `a.b.c`, use of `a.b.d` + // 5. Uninitialized `a.x`, initialized `a.b`, use of `a.b` + // 6. Copied `(a.b: &_)`, use of `*(a.b).c`; note that `a.b` // must have been initialized for the use to be sound. - // 8. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d` + // 7. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d` // The dataflow tracks shallow prefixes distinctly (that is, // field-accesses on P distinctly from P itself), in order to @@ -1352,9 +1355,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // have a MovePath, that should capture the initialization // state for the place scenario. // - // This code covers scenarios 1, 2, and 4. + // This code covers scenarios 1, 2, and 3. - debug!("check_if_path_is_moved part1 place: {:?}", place); + debug!("check_if_full_path_is_moved place: {:?}", place); match self.move_path_closest_to(place) { Ok(mpi) => { if maybe_uninits.contains(&mpi) { @@ -1374,9 +1377,41 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // ancestors; dataflow recurs on children when parents // move (to support partial (re)inits). // - // (I.e. querying parents breaks scenario 8; but may want + // (I.e. querying parents breaks scenario 7; but may want // to do such a query based on partial-init feature-gate.) } + } + + fn check_if_path_or_subpath_is_moved( + &mut self, + context: Context, + desired_action: InitializationRequiringAction, + place_span: (&Place<'tcx>, Span), + flow_state: &Flows<'cx, 'gcx, 'tcx>, + ) { + // FIXME: analogous code in check_loans first maps `place` to + // its base_path ... but is that what we want here? + let place = self.base_path(place_span.0); + + let maybe_uninits = &flow_state.uninits; + let curr_move_outs = &flow_state.move_outs; + + // Bad scenarios: + // + // 1. Move of `a.b.c`, use of `a` or `a.b` + // partial initialization support, one might have `a.x` + // initialized but not `a.b`. + // 2. All bad scenarios from `check_if_full_path_is_moved` + // + // OK scenarios: + // + // 3. Move of `a.b.c`, use of `a.b.d` + // 4. Uninitialized `a.x`, initialized `a.b`, use of `a.b` + // 5. Copied `(a.b: &_)`, use of `*(a.b).c`; note that `a.b` + // must have been initialized for the use to be sound. + // 6. Move of `a.b.c` then reinit of `a.b.c.d`, use of `a.b.c.d` + + self.check_if_full_path_is_moved(context, desired_action, place_span, flow_state); // A move of any shallow suffix of `place` also interferes // with an attempt to use `place`. This is scenario 3 above. @@ -1384,8 +1419,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // (Distinct from handling of scenarios 1+2+4 above because // `place` does not interfere with suffixes of its prefixes, // e.g. `a.b.c` does not interfere with `a.b.d`) + // + // This code covers scenario 1. - debug!("check_if_path_is_moved part2 place: {:?}", place); + debug!("check_if_path_or_subpath_is_moved place: {:?}", place); if let Some(mpi) = self.move_path_for_place(place) { if let Some(child_mpi) = maybe_uninits.has_any_child_of(mpi) { self.report_use_of_moved_or_uninitialized( @@ -1445,7 +1482,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { (place, span): (&Place<'tcx>, Span), flow_state: &Flows<'cx, 'gcx, 'tcx>, ) { - // recur down place; dispatch to check_if_path_is_moved when necessary + debug!("check_if_assigned_path_is_moved place: {:?}", place); + // recur down place; dispatch to external checks when necessary let mut place = place; loop { match *place { @@ -1456,17 +1494,25 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Projection(ref proj) => { let Projection { ref base, ref elem } = **proj; match *elem { - ProjectionElem::Deref | - // assigning to *P requires `P` initialized. ProjectionElem::Index(_/*operand*/) | ProjectionElem::ConstantIndex { .. } | - // assigning to P[i] requires `P` initialized. + // assigning to P[i] requires P to be valid. ProjectionElem::Downcast(_/*adt_def*/, _/*variant_idx*/) => // assigning to (P->variant) is okay if assigning to `P` is okay // // FIXME: is this true even if P is a adt with a dtor? { } + // assigning to (*P) requires P to be initialized + ProjectionElem::Deref => { + self.check_if_full_path_is_moved( + context, InitializationRequiringAction::Use, + (base, span), flow_state); + // (base initialized; no need to + // recur further) + break; + } + ProjectionElem::Subslice { .. } => { panic!("we don't allow assignments to subslices, context: {:?}", context); @@ -1484,7 +1530,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // check_loans.rs first maps // `base` to its base_path. - self.check_if_path_is_moved( + self.check_if_path_or_subpath_is_moved( context, InitializationRequiringAction::Assignment, (base, span), flow_state); diff --git a/src/test/compile-fail/borrowck/borrowck-issue-48962.rs b/src/test/compile-fail/borrowck/borrowck-issue-48962.rs new file mode 100644 index 00000000000..e3bbfd9d5fe --- /dev/null +++ b/src/test/compile-fail/borrowck/borrowck-issue-48962.rs @@ -0,0 +1,38 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(nll)] + +struct Node { + elem: i32, + next: Option<Box<Node>>, +} + +fn a() { + let mut node = Node { + elem: 5, + next: None, + }; + + let mut src = &mut node; + {src}; + src.next = None; //~ ERROR use of moved value: `src` [E0382] +} + +fn b() { + let mut src = &mut (22, 44); + {src}; + src.0 = 66; //~ ERROR use of moved value: `src` [E0382] +} + +fn main() { + a(); + b(); +} diff --git a/src/test/run-pass/issue-48962.rs b/src/test/run-pass/issue-48962.rs new file mode 100644 index 00000000000..a603dfbfe5f --- /dev/null +++ b/src/test/run-pass/issue-48962.rs @@ -0,0 +1,43 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we are able to reinitilize box with moved referent +#![feature(nll)] +static mut ORDER: [usize; 3] = [0, 0, 0]; +static mut INDEX: usize = 0; + +struct Dropee (usize); + +impl Drop for Dropee { + fn drop(&mut self) { + unsafe { + ORDER[INDEX] = self.0; + INDEX = INDEX + 1; + } + } +} + +fn add_sentintel() { + unsafe { + ORDER[INDEX] = 2; + INDEX = INDEX + 1; + } +} + +fn main() { + let mut x = Box::new(Dropee(1)); + *x; // move out from `*x` + add_sentintel(); + *x = Dropee(3); // re-initialize `*x` + {x}; // drop value + unsafe { + assert_eq!(ORDER, [1, 2, 3]); + } +} |