-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes move analysis #49392
fixes move analysis #49392
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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), | ||
|
@@ -963,7 +967,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, | ||
|
@@ -1023,7 +1027,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), | ||
|
@@ -1051,7 +1055,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), | ||
|
@@ -1098,7 +1102,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), | ||
|
@@ -1116,7 +1120,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), | ||
|
@@ -1267,7 +1271,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. | ||
} | ||
|
@@ -1302,7 +1306,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, | ||
|
@@ -1320,18 +1324,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | |
// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe rename |
||
// 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 | ||
|
@@ -1350,9 +1353,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) { | ||
|
@@ -1372,18 +1375,52 @@ 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what code are you referring to exactly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly same code |
||
// 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. | ||
// | ||
// (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( | ||
|
@@ -1443,7 +1480,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 { | ||
|
@@ -1454,17 +1492,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 => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should have a comment // assigning to (*P) requires P to be initialized |
||
self.check_if_full_path_is_moved( | ||
context, InitializationRequiringAction::Use, | ||
(base, span), flow_state); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can add a Maybe in that case you would also not need the new error deduplication code. |
||
// (base initialized; no need to | ||
// recur further) | ||
break; | ||
} | ||
|
||
ProjectionElem::Subslice { .. } => { | ||
panic!("we don't allow assignments to subslices, context: {:?}", | ||
context); | ||
|
@@ -1482,7 +1528,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); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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]); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
english fix: error about
{:?}
suppressed.