]> git.lizzy.rs Git - rust.git/blobdiff - src/librustc_mir/borrow_check/mod.rs
Auto merge of #55134 - davidtwco:issue-55118, r=pnkfelix
[rust.git] / src / librustc_mir / borrow_check / mod.rs
index fe80447e4a84f4b9647f959899995a302283154e..98663270882af031482153f10840b5e83cf28b17 100644 (file)
@@ -36,9 +36,8 @@
 
 use syntax_pos::Span;
 
-use dataflow::indexes::BorrowIndex;
-use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex};
-use dataflow::move_paths::indexes::MoveOutIndex;
+use dataflow::indexes::{BorrowIndex, InitIndex, MoveOutIndex, MovePathIndex};
+use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError};
 use dataflow::Borrows;
 use dataflow::DataflowResultsConsumer;
 use dataflow::FlowAtLocation;
@@ -853,6 +852,7 @@ enum InitializationRequiringAction {
     MatchOn,
     Use,
     Assignment,
+    PartialAssignment,
 }
 
 struct RootPlace<'d, 'tcx: 'd> {
@@ -868,6 +868,7 @@ fn as_noun(self) -> &'static str {
             InitializationRequiringAction::MatchOn => "use", // no good noun
             InitializationRequiringAction::Use => "use",
             InitializationRequiringAction::Assignment => "assign",
+            InitializationRequiringAction::PartialAssignment => "assign to part",
         }
     }
 
@@ -878,6 +879,7 @@ fn as_verb_in_past_tense(self) -> &'static str {
             InitializationRequiringAction::MatchOn => "matched on",
             InitializationRequiringAction::Use => "used",
             InitializationRequiringAction::Assignment => "assigned",
+            InitializationRequiringAction::PartialAssignment => "partially assigned",
         }
     }
 }
@@ -1439,10 +1441,7 @@ fn check_if_reassignment_to_immutable_state(
         debug!("check_if_reassignment_to_immutable_state({:?})", local);
 
         // Check if any of the initializiations of `local` have happened yet:
-        let mpi = self.move_data.rev_lookup.find_local(local);
-        let init_indices = &self.move_data.init_path_map[mpi];
-        let first_init_index = init_indices.iter().find(|&ii| flow_state.ever_inits.contains(*ii));
-        if let Some(&init_index) = first_init_index {
+        if let Some(init_index) = self.is_local_ever_initialized(local, flow_state) {
             // And, if so, report an error.
             let init = &self.move_data.inits[init_index];
             let span = init.span(&self.mir);
@@ -1498,12 +1497,12 @@ fn check_if_full_path_is_moved(
 
         debug!("check_if_full_path_is_moved place: {:?}", place_span.0);
         match self.move_path_closest_to(place_span.0) {
-            Ok(mpi) => {
+            Ok((prefix, mpi)) => {
                 if maybe_uninits.contains(mpi) {
                     self.report_use_of_moved_or_uninitialized(
                         context,
                         desired_action,
-                        place_span,
+                        (prefix, place_span.0, place_span.1),
                         mpi,
                     );
                     return; // don't bother finding other problems.
@@ -1561,7 +1560,7 @@ fn check_if_path_or_subpath_is_moved(
                 self.report_use_of_moved_or_uninitialized(
                     context,
                     desired_action,
-                    place_span,
+                    (place_span.0, place_span.0, place_span.1),
                     child_mpi,
                 );
                 return; // don't bother finding other problems.
@@ -1579,14 +1578,14 @@ fn check_if_path_or_subpath_is_moved(
     /// An Err result includes a tag indicated why the search failed.
     /// Currently this can only occur if the place is built off of a
     /// static variable, as we do not track those in the MoveData.
-    fn move_path_closest_to(
+    fn move_path_closest_to<'a>(
         &mut self,
-        place: &Place<'tcx>,
-    ) -> Result<MovePathIndex, NoMovePathFound> {
+        place: &'a Place<'tcx>,
+    ) -> Result<(&'a Place<'tcx>, MovePathIndex), NoMovePathFound> where 'cx: 'a {
         let mut last_prefix = place;
         for prefix in self.prefixes(place, PrefixSet::All) {
             if let Some(mpi) = self.move_path_for_place(prefix) {
-                return Ok(mpi);
+                return Ok((prefix, mpi));
             }
             last_prefix = prefix;
         }
@@ -1667,6 +1666,26 @@ fn check_if_assigned_path_is_moved(
                                     // recur further)
                                     break;
                                 }
+
+
+                                // Once `let s; s.x = V; read(s.x);`,
+                                // is allowed, remove this match arm.
+                                ty::Adt(..) | ty::Tuple(..) => {
+                                    check_parent_of_field(self, context, base, span, flow_state);
+
+                                    if let Some(local) = place.base_local() {
+                                        // rust-lang/rust#21232,
+                                        // #54499, #54986: during
+                                        // period where we reject
+                                        // partial initialization, do
+                                        // not complain about
+                                        // unnecessary `mut` on an
+                                        // attempt to do a partial
+                                        // initialization.
+                                        self.used_mut.insert(local);
+                                    }
+                                }
+
                                 _ => {}
                             }
                         }
@@ -1677,8 +1696,73 @@ fn check_if_assigned_path_is_moved(
                 }
             }
         }
-    }
 
+        fn check_parent_of_field<'cx, 'gcx, 'tcx>(this: &mut MirBorrowckCtxt<'cx, 'gcx, 'tcx>,
+                                                  context: Context,
+                                                  base: &Place<'tcx>,
+                                                  span: Span,
+                                                  flow_state: &Flows<'cx, 'gcx, 'tcx>)
+        {
+            // rust-lang/rust#21232: Until Rust allows reads from the
+            // initialized parts of partially initialized structs, we
+            // will, starting with the 2018 edition, reject attempts
+            // to write to structs that are not fully initialized.
+            //
+            // In other words, *until* we allow this:
+            //
+            // 1. `let mut s; s.x = Val; read(s.x);`
+            //
+            // we will for now disallow this:
+            //
+            // 2. `let mut s; s.x = Val;`
+            //
+            // and also this:
+            //
+            // 3. `let mut s = ...; drop(s); s.x=Val;`
+            //
+            // This does not use check_if_path_or_subpath_is_moved,
+            // because we want to *allow* reinitializations of fields:
+            // e.g. want to allow
+            //
+            // `let mut s = ...; drop(s.x); s.x=Val;`
+            //
+            // This does not use check_if_full_path_is_moved on
+            // `base`, because that would report an error about the
+            // `base` as a whole, but in this scenario we *really*
+            // want to report an error about the actual thing that was
+            // moved, which may be some prefix of `base`.
+
+            // Shallow so that we'll stop at any dereference; we'll
+            // report errors about issues with such bases elsewhere.
+            let maybe_uninits = &flow_state.uninits;
+
+            // Find the shortest uninitialized prefix you can reach
+            // without going over a Deref.
+            let mut shortest_uninit_seen = None;
+            for prefix in this.prefixes(base, PrefixSet::Shallow) {
+                let mpi = match this.move_path_for_place(prefix) {
+                    Some(mpi) => mpi, None => continue,
+                };
+
+                if maybe_uninits.contains(mpi) {
+                    debug!("check_parent_of_field updating shortest_uninit_seen from {:?} to {:?}",
+                           shortest_uninit_seen, Some((prefix, mpi)));
+                    shortest_uninit_seen = Some((prefix, mpi));
+                } else {
+                    debug!("check_parent_of_field {:?} is definitely initialized", (prefix, mpi));
+                }
+            }
+
+            if let Some((prefix, mpi)) = shortest_uninit_seen {
+                this.report_use_of_moved_or_uninitialized(
+                    context,
+                    InitializationRequiringAction::PartialAssignment,
+                    (prefix, base, span),
+                    mpi,
+                );
+            }
+        }
+    }
 
     /// Check the permissions for the given place and read or write kind
     ///
@@ -1692,13 +1776,23 @@ fn check_access_permissions(
         location: Location,
     ) -> bool {
         debug!(
-            "check_access_permissions({:?}, {:?}, {:?})",
+            "check_access_permissions({:?}, {:?}, is_local_mutation_allowed: {:?})",
             place, kind, is_local_mutation_allowed
         );
 
         let error_access;
         let the_place_err;
 
+        // rust-lang/rust#21232, #54986: during period where we reject
+        // partial initialization, do not complain about mutability
+        // errors except for actual mutation (as opposed to an attempt
+        // to do a partial initialization).
+        let previously_initialized = if let Some(local) = place.base_local() {
+            self.is_local_ever_initialized(local, flow_state).is_some()
+        } else {
+            true
+        };
+
         match kind {
             Reservation(WriteKind::MutableBorrow(borrow_kind @ BorrowKind::Unique))
             | Reservation(WriteKind::MutableBorrow(borrow_kind @ BorrowKind::Mut { .. }))
@@ -1791,14 +1885,33 @@ fn check_access_permissions(
         }
 
         // at this point, we have set up the error reporting state.
-        self.report_mutability_error(
-            place,
-            span,
-            the_place_err,
-            error_access,
-            location,
-        );
-        return true;
+        if previously_initialized {
+            self.report_mutability_error(
+                place,
+                span,
+                the_place_err,
+                error_access,
+                location,
+            );
+            return true;
+        } else {
+            return false;
+        }
+    }
+
+    fn is_local_ever_initialized(&self,
+                                 local: Local,
+                                 flow_state: &Flows<'cx, 'gcx, 'tcx>)
+                                 -> Option<InitIndex>
+    {
+        let mpi = self.move_data.rev_lookup.find_local(local);
+        let ii = &self.move_data.init_path_map[mpi];
+        for &index in ii {
+            if flow_state.ever_inits.contains(index) {
+                return Some(index);
+            }
+        }
+        return None;
     }
 
     /// Adds the place into the used mutable variables set
@@ -1812,18 +1925,13 @@ fn add_used_mut<'d>(
                 place: Place::Local(local),
                 is_local_mutation_allowed,
             } => {
-                if is_local_mutation_allowed != LocalMutationIsAllowed::Yes {
-                    // If the local may be initialized, and it is now currently being
-                    // mutated, then it is justified to be annotated with the `mut`
-                    // keyword, since the mutation may be a possible reassignment.
-                    let mpi = self.move_data.rev_lookup.find_local(*local);
-                    let ii = &self.move_data.init_path_map[mpi];
-                    for &index in ii {
-                        if flow_state.ever_inits.contains(index) {
-                            self.used_mut.insert(*local);
-                            break;
-                        }
-                    }
+                // If the local may have been initialized, and it is now currently being
+                // mutated, then it is justified to be annotated with the `mut`
+                // keyword, since the mutation may be a possible reassignment.
+                if is_local_mutation_allowed != LocalMutationIsAllowed::Yes &&
+                    self.is_local_ever_initialized(*local, flow_state).is_some()
+                {
+                    self.used_mut.insert(*local);
                 }
             }
             RootPlace {
@@ -1849,7 +1957,7 @@ fn add_used_mut<'d>(
         }
     }
 
-    /// Whether this value be written or borrowed mutably.
+    /// Whether this value can be written or borrowed mutably.
     /// Returns the root place if the place passed in is a projection.
     fn is_mutable<'d>(
         &self,
@@ -1927,14 +2035,14 @@ fn is_mutable<'d>(
                             ty::RawPtr(tnm) => {
                                 match tnm.mutbl {
                                     // `*const` raw pointers are not mutable
-                                    hir::MutImmutable => return Err(place),
+                                    hir::MutImmutable => Err(place),
                                     // `*mut` raw pointers are always mutable, regardless of
                                     // context. The users have to check by themselves.
                                     hir::MutMutable => {
-                                        return Ok(RootPlace {
+                                        Ok(RootPlace {
                                             place,
                                             is_local_mutation_allowed,
-                                        });
+                                        })
                                     }
                                 }
                             }