Auto merge of rust-lang#80624 - RalfJung:place-ref, r=oli-obk

use PlaceRef more consistently instead of loosely coupled local+projection

Instead of working directly with the `projections` array, use `iter_projections` and `last_projection`. This avoids having to construct new `PlaceRef` from the pieces everywhere.

I only did this for a few files, to see how people think about this. If y'all are happy with this, I'll open an E-mentor issue to complete this. I grepped for `Place::ty_from` to find the places that need adjusting -- this could miss some, but I am not sure what else to grep for.

Author: bors

compiler/rustc_codegen_ssa/src/mir/analyze.rs

@@ -104,7 +104,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
     ) {
         let cx = self.fx.cx;
 
-        if let &[ref proj_base @ .., elem] = place_ref.projection {
+        if let Some((place_base, elem)) = place_ref.last_projection() {
             let mut base_context = if context.is_mutating_use() {
                 PlaceContext::MutatingUse(MutatingUseContext::Projection)
             } else {
@@ -119,8 +119,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
                 )
             );
             if is_consume {
-                let base_ty =
-                    mir::Place::ty_from(place_ref.local, proj_base, self.fx.mir, cx.tcx());
+                let base_ty = mir::PlaceRef::ty(&place_base, self.fx.mir, cx.tcx());
                 let base_ty = self.fx.monomorphize(base_ty);
 
                 // ZSTs don't require any actual memory access.
@@ -175,11 +174,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
                 base_context = context;
             }
 
-            self.process_place(
-                &mir::PlaceRef { local: place_ref.local, projection: proj_base },
-                base_context,
-                location,
-            );
+            self.process_place(&place_base, base_context, location);
             // HACK(eddyb) this emulates the old `visit_projection_elem`, this
             // entire `visit_place`-like `process_place` method should be rewritten,
             // now that we have moved to the "slice of projections" representation.

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -514,7 +514,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
 
     pub fn monomorphized_place_ty(&self, place_ref: mir::PlaceRef<'tcx>) -> Ty<'tcx> {
         let tcx = self.cx.tcx();
-        let place_ty = mir::Place::ty_from(place_ref.local, place_ref.projection, self.mir, tcx);
+        let place_ty = mir::PlaceRef::ty(&place_ref, self.mir, tcx);
         self.monomorphize(place_ty.ty)
     }
 }

compiler/rustc_middle/src/mir/mod.rs

@@ -1745,18 +1745,14 @@ impl<'tcx> Place<'tcx> {
 
     /// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or
     /// a single deref of a local.
-    //
-    // FIXME: can we safely swap the semantics of `fn base_local` below in here instead?
+    #[inline(always)]
     pub fn local_or_deref_local(&self) -> Option<Local> {
-        match self.as_ref() {
-            PlaceRef { local, projection: [] }
-            | PlaceRef { local, projection: [ProjectionElem::Deref] } => Some(local),
-            _ => None,
-        }
+        self.as_ref().local_or_deref_local()
     }
 
     /// If this place represents a local variable like `_X` with no
     /// projections, return `Some(_X)`.
+    #[inline(always)]
     pub fn as_local(&self) -> Option<Local> {
         self.as_ref().as_local()
     }
@@ -1770,6 +1766,7 @@ impl<'tcx> Place<'tcx> {
     /// As a concrete example, given the place a.b.c, this would yield:
     /// - (a, .b)
     /// - (a.b, .c)
+    ///
     /// Given a place without projections, the iterator is empty.
     pub fn iter_projections(
         self,
@@ -1790,8 +1787,6 @@ impl From<Local> for Place<'_> {
 impl<'tcx> PlaceRef<'tcx> {
     /// Finds the innermost `Local` from this `Place`, *if* it is either a local itself or
     /// a single deref of a local.
-    //
-    // FIXME: can we safely swap the semantics of `fn base_local` below in here instead?
     pub fn local_or_deref_local(&self) -> Option<Local> {
         match *self {
             PlaceRef { local, projection: [] }
@@ -1808,6 +1803,14 @@ impl<'tcx> PlaceRef<'tcx> {
             _ => None,
         }
     }
+
+    pub fn last_projection(&self) -> Option<(PlaceRef<'tcx>, PlaceElem<'tcx>)> {
+        if let &[ref proj_base @ .., elem] = self.projection {
+            Some((PlaceRef { local: self.local, projection: proj_base }, elem))
+        } else {
+            None
+        }
+    }
 }
 
 impl Debug for Place<'_> {

compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs

@@ -293,9 +293,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 );
             }
 
-            let ty =
-                Place::ty_from(used_place.local, used_place.projection, self.body, self.infcx.tcx)
-                    .ty;
+            let ty = PlaceRef::ty(&used_place, self.body, self.infcx.tcx).ty;
             let needs_note = match ty.kind() {
                 ty::Closure(id, _) => {
                     let tables = self.infcx.tcx.typeck(id.expect_local());
@@ -732,8 +730,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     ) -> (String, String, String, String) {
         // Define a small closure that we can use to check if the type of a place
         // is a union.
-        let union_ty = |place_base, place_projection| {
-            let ty = Place::ty_from(place_base, place_projection, self.body, self.infcx.tcx).ty;
+        let union_ty = |place_base| {
+            let ty = PlaceRef::ty(&place_base, self.body, self.infcx.tcx).ty;
             ty.ty_adt_def().filter(|adt| adt.is_union()).map(|_| ty)
         };
 
@@ -751,15 +749,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 // field access to a union. If we find that, then we will keep the place of the
                 // union being accessed and the field that was being accessed so we can check the
                 // second borrowed place for the same union and a access to a different field.
-                let Place { local, projection } = first_borrowed_place;
-
-                let mut cursor = projection.as_ref();
-                while let [proj_base @ .., elem] = cursor {
-                    cursor = proj_base;
-
+                for (place_base, elem) in first_borrowed_place.iter_projections().rev() {
                     match elem {
-                        ProjectionElem::Field(field, _) if union_ty(local, proj_base).is_some() => {
-                            return Some((PlaceRef { local, projection: proj_base }, field));
+                        ProjectionElem::Field(field, _) if union_ty(place_base).is_some() => {
+                            return Some((place_base, field));
                         }
                         _ => {}
                     }
@@ -769,23 +762,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             .and_then(|(target_base, target_field)| {
                 // With the place of a union and a field access into it, we traverse the second
                 // borrowed place and look for a access to a different field of the same union.
-                let Place { local, ref projection } = second_borrowed_place;
-
-                let mut cursor = &projection[..];
-                while let [proj_base @ .., elem] = cursor {
-                    cursor = proj_base;
-
+                for (place_base, elem) in second_borrowed_place.iter_projections().rev() {
                     if let ProjectionElem::Field(field, _) = elem {
-                        if let Some(union_ty) = union_ty(local, proj_base) {
-                            if field != target_field
-                                && local == target_base.local
-                                && proj_base == target_base.projection
-                            {
+                        if let Some(union_ty) = union_ty(place_base) {
+                            if field != target_field && place_base == target_base {
                                 return Some((
-                                    self.describe_any_place(PlaceRef {
-                                        local,
-                                        projection: proj_base,
-                                    }),
+                                    self.describe_any_place(place_base),
                                     self.describe_any_place(first_borrowed_place.as_ref()),
                                     self.describe_any_place(second_borrowed_place.as_ref()),
                                     union_ty.to_string(),

compiler/rustc_mir/src/borrow_check/mod.rs

@@ -1740,20 +1740,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
 
         self.check_if_full_path_is_moved(location, desired_action, place_span, flow_state);
 
-        if let [base_proj @ .., ProjectionElem::Subslice { from, to, from_end: false }] =
-            place_span.0.projection
+        if let Some((place_base, ProjectionElem::Subslice { from, to, from_end: false })) =
+            place_span.0.last_projection()
         {
-            let place_ty =
-                Place::ty_from(place_span.0.local, base_proj, self.body(), self.infcx.tcx);
+            let place_ty = PlaceRef::ty(&place_base, self.body(), self.infcx.tcx);
             if let ty::Array(..) = place_ty.ty.kind() {
-                let array_place = PlaceRef { local: place_span.0.local, projection: base_proj };
                 self.check_if_subslice_element_is_moved(
                     location,
                     desired_action,
-                    (array_place, place_span.1),
+                    (place_base, place_span.1),
                     maybe_uninits,
-                    *from,
-                    *to,
+                    from,
+                    to,
                 );
                 return;
             }
@@ -1825,10 +1823,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         debug!("check_if_assigned_path_is_moved place: {:?}", place);
 
         // None case => assigning to `x` does not require `x` be initialized.
-        let mut cursor = &*place.projection.as_ref();
-        while let [proj_base @ .., elem] = cursor {
-            cursor = proj_base;
-
+        for (place_base, elem) in place.iter_projections().rev() {
             match elem {
                 ProjectionElem::Index(_/*operand*/) |
                 ProjectionElem::ConstantIndex { .. } |
@@ -1843,10 +1838,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 ProjectionElem::Deref => {
                     self.check_if_full_path_is_moved(
                         location, InitializationRequiringAction::Use,
-                        (PlaceRef {
-                            local: place.local,
-                            projection: proj_base,
-                        }, span), flow_state);
+                        (place_base, span), flow_state);
                     // (base initialized; no need to
                     // recur further)
                     break;
@@ -1862,15 +1854,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     // assigning to `P.f` requires `P` itself
                     // be already initialized
                     let tcx = self.infcx.tcx;
-                    let base_ty = Place::ty_from(place.local, proj_base, self.body(), tcx).ty;
+                    let base_ty = PlaceRef::ty(&place_base, self.body(), tcx).ty;
                     match base_ty.kind() {
                         ty::Adt(def, _) if def.has_dtor(tcx) => {
                             self.check_if_path_or_subpath_is_moved(
                                 location, InitializationRequiringAction::Assignment,
-                                (PlaceRef {
-                                    local: place.local,
-                                    projection: proj_base,
-                                }, span), flow_state);
+                                (place_base, span), flow_state);
 
                             // (base initialized; no need to
                             // recur further)
@@ -1880,10 +1869,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         // Once `let s; s.x = V; read(s.x);`,
                         // is allowed, remove this match arm.
                         ty::Adt(..) | ty::Tuple(..) => {
-                            check_parent_of_field(self, location, PlaceRef {
-                                local: place.local,
-                                projection: proj_base,
-                            }, span, flow_state);
+                            check_parent_of_field(self, location, place_base, span, flow_state);
 
                             // rust-lang/rust#21232, #54499, #54986: during period where we reject
                             // partial initialization, do not complain about unnecessary `mut` on
@@ -1965,9 +1951,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 // no move out from an earlier location) then this is an attempt at initialization
                 // of the union - we should error in that case.
                 let tcx = this.infcx.tcx;
-                if let ty::Adt(def, _) =
-                    Place::ty_from(base.local, base.projection, this.body(), tcx).ty.kind()
-                {
+                if let ty::Adt(def, _) = PlaceRef::ty(&base, this.body(), tcx).ty.kind() {
                     if def.is_union() {
                         if this.move_data.path_map[mpi].iter().any(|moi| {
                             this.move_data.moves[*moi].source.is_predecessor_of(location, this.body)
@@ -2162,9 +2146,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
         place: PlaceRef<'tcx>,
         is_local_mutation_allowed: LocalMutationIsAllowed,
     ) -> Result<RootPlace<'tcx>, PlaceRef<'tcx>> {
-        match place {
-            PlaceRef { local, projection: [] } => {
-                let local = &self.body.local_decls[local];
+        match place.last_projection() {
+            None => {
+                let local = &self.body.local_decls[place.local];
                 match local.mutability {
                     Mutability::Not => match is_local_mutation_allowed {
                         LocalMutationIsAllowed::Yes => Ok(RootPlace {
@@ -2186,11 +2170,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                     }),
                 }
             }
-            PlaceRef { local: _, projection: [proj_base @ .., elem] } => {
+            Some((place_base, elem)) => {
                 match elem {
                     ProjectionElem::Deref => {
-                        let base_ty =
-                            Place::ty_from(place.local, proj_base, self.body(), self.infcx.tcx).ty;
+                        let base_ty = PlaceRef::ty(&place_base, self.body(), self.infcx.tcx).ty;
 
                         // Check the kind of deref to decide
                         match base_ty.kind() {
@@ -2208,10 +2191,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                             _ => LocalMutationIsAllowed::Yes,
                                         };
 
-                                        self.is_mutable(
-                                            PlaceRef { local: place.local, projection: proj_base },
-                                            mode,
-                                        )
+                                        self.is_mutable(place_base, mode)
                                     }
                                 }
                             }
@@ -2229,10 +2209,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 }
                             }
                             // `Box<T>` owns its content, so mutable if its location is mutable
-                            _ if base_ty.is_box() => self.is_mutable(
-                                PlaceRef { local: place.local, projection: proj_base },
-                                is_local_mutation_allowed,
-                            ),
+                            _ if base_ty.is_box() => {
+                                self.is_mutable(place_base, is_local_mutation_allowed)
+                            }
                             // Deref should only be for reference, pointers or boxes
                             _ => bug!("Deref of unexpected type: {:?}", base_ty),
                         }
@@ -2286,10 +2265,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                     //     });
                                     // }
                                     // ```
-                                    let _ = self.is_mutable(
-                                        PlaceRef { local: place.local, projection: proj_base },
-                                        is_local_mutation_allowed,
-                                    )?;
+                                    let _ =
+                                        self.is_mutable(place_base, is_local_mutation_allowed)?;
                                     Ok(RootPlace {
                                         place_local: place.local,
                                         place_projection: place.projection,
@@ -2298,10 +2275,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 }
                             }
                         } else {
-                            self.is_mutable(
-                                PlaceRef { local: place.local, projection: proj_base },
-                                is_local_mutation_allowed,
-                            )
+                            self.is_mutable(place_base, is_local_mutation_allowed)
                         }
                     }
                 }