Auto merge of #49392 - retep007:nll-issue-48962, r=nikomatsakis

fixes move analysis

Fixed compiler error by correct checking when dereferencing

Fix #48962

r? @nikomatsakis

Author: bors

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(),

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,18 +1377,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
+        // 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(
@@ -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);
 

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();
+}

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]);
+    }
+}