Rollup merge of #147575 - beepster4096:movesubpath, r=oli-obk

Refactor move analysis subpath representation

Follow up to rust-lang/rust#147055

This PR does two things:
1. Document/validate move analysis's assumptions about `Subslice` projections
2. Decouple move paths from `ProjectionElem`, using a new enum `MoveSubPath` instead
    - This would be needed eventually when `ProjectionElem::Deref` is removed

I wanted to do even more abstraction, making `MovePathLookup::find` return an iterator to remove the special handling of subslices in borrowck, but that regressed diagnostics and just wasn't worth the complexity.
This commit is contained in:
Matthias Krüger 2025-10-19 14:04:53 +02:00 committed by GitHub
commit 3bfd71290e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 342 additions and 196 deletions

View file

@ -137,6 +137,7 @@ pub enum RuntimePhase {
/// And the following variants are allowed:
/// * [`StatementKind::Retag`]
/// * [`StatementKind::SetDiscriminant`]
/// * [`PlaceElem::ConstantIndex`] / [`PlaceElem::Subslice`] after [`PlaceElem::Subslice`]
///
/// Furthermore, `Copy` operands are allowed for non-`Copy` types.
Initial = 0,
@ -1246,6 +1247,9 @@ pub enum ProjectionElem<V, T> {
///
/// If `from_end` is true `slice[from..slice.len() - to]`.
/// Otherwise `array[from..to]`.
///
/// This projection cannot have `ConstantIndex` or additional `Subslice` projections after it
/// before runtime MIR.
Subslice {
from: u64,
to: u64,

View file

@ -9,7 +9,7 @@ use tracing::debug;
use super::{
Init, InitIndex, InitKind, InitLocation, LocationMap, LookupResult, MoveData, MoveOut,
MoveOutIndex, MovePath, MovePathIndex, MovePathLookup,
MoveOutIndex, MovePath, MovePathIndex, MovePathLookup, MoveSubPath, MoveSubPathResult,
};
struct MoveDataBuilder<'a, 'tcx, F> {
@ -94,26 +94,25 @@ fn new_move_path<'tcx>(
move_path
}
enum MovePathResult {
Path(MovePathIndex),
Union(MovePathIndex),
Error,
}
impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> {
/// This creates a MovePath for a given place, returning an `MovePathError`
/// if that place can't be moved from.
/// This creates a MovePath for a given place, calling `on_move`
/// if it can be moved from. If theres a union in the path, its
/// move place will be given to `on_move`. If there's a subslice
/// projection, `on_move` will be called for each element.
///
/// NOTE: places behind references *do not* get a move path, which is
/// problematic for borrowck.
///
/// Maybe we should have separate "borrowck" and "moveck" modes.
fn move_path_for(&mut self, place: Place<'tcx>) -> MovePathResult {
fn move_path_for<G>(&mut self, place: Place<'tcx>, mut on_move: G)
where
G: FnMut(&mut Self, MovePathIndex),
{
let data = &mut self.data;
debug!("lookup({:?})", place);
let Some(mut base) = data.rev_lookup.find_local(place.local) else {
return MovePathResult::Error;
return;
};
// The move path index of the first union that we find. Once this is
@ -123,144 +122,186 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> {
// from `*(u.f: &_)` isn't allowed.
let mut union_path = None;
for (place_ref, elem) in data.rev_lookup.un_derefer.iter_projections(place.as_ref()) {
let mut iter = data.rev_lookup.un_derefer.iter_projections(place.as_ref());
while let Some((place_ref, elem)) = iter.next() {
let body = self.body;
let tcx = self.tcx;
let place_ty = place_ref.ty(body, tcx).ty;
if place_ty.references_error() {
return MovePathResult::Error;
return;
}
match elem {
ProjectionElem::Deref => match place_ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
return MovePathResult::Error;
}
ty::Adt(adt, _) => {
if !adt.is_box() {
bug!("Adt should be a box type when Place is deref");
}
}
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Foreign(_)
| ty::Str
| ty::Array(_, _)
| ty::Pat(_, _)
| ty::Slice(_)
| ty::FnDef(_, _)
| ty::FnPtr(..)
| ty::Dynamic(_, _)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(_, _)
| ty::CoroutineWitness(..)
| ty::Never
| ty::Tuple(_)
| ty::UnsafeBinder(_)
| ty::Alias(_, _)
| ty::Param(_)
| ty::Bound(_, _)
| ty::Infer(_)
| ty::Error(_)
| ty::Placeholder(_) => {
bug!("When Place is Deref it's type shouldn't be {place_ty:#?}")
}
},
ProjectionElem::Field(_, _) => match place_ty.kind() {
ty::Adt(adt, _) => {
if adt.has_dtor(tcx) {
return MovePathResult::Error;
}
if adt.is_union() {
union_path.get_or_insert(base);
}
}
ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(_, _)
| ty::Tuple(_) => (),
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Foreign(_)
| ty::Str
| ty::Array(_, _)
| ty::Pat(_, _)
| ty::Slice(_)
| ty::RawPtr(_, _)
| ty::Ref(_, _, _)
| ty::FnDef(_, _)
| ty::FnPtr(..)
| ty::Dynamic(_, _)
| ty::CoroutineWitness(..)
| ty::Never
| ty::UnsafeBinder(_)
| ty::Alias(_, _)
| ty::Param(_)
| ty::Bound(_, _)
| ty::Infer(_)
| ty::Error(_)
| ty::Placeholder(_) => bug!(
"When Place contains ProjectionElem::Field its type shouldn't be {place_ty:#?}"
),
},
ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {
match place_ty.kind() {
ty::Slice(_) => {
return MovePathResult::Error;
}
ty::Array(_, _) => (),
_ => bug!("Unexpected type {:#?}", place_ty.is_array()),
}
let res = MoveSubPath::of(elem.kind());
let move_elem = match res {
MoveSubPathResult::One(move_elem) => {
match move_elem {
MoveSubPath::Deref => match place_ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => {
return;
}
ty::Adt(adt, _) => {
if !adt.is_box() {
bug!("Adt should be a box type when Place is deref");
}
}
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Foreign(_)
| ty::Str
| ty::Array(_, _)
| ty::Pat(_, _)
| ty::Slice(_)
| ty::FnDef(_, _)
| ty::FnPtr(..)
| ty::Dynamic(_, _)
| ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(_, _)
| ty::CoroutineWitness(..)
| ty::Never
| ty::Tuple(_)
| ty::UnsafeBinder(_)
| ty::Alias(_, _)
| ty::Param(_)
| ty::Bound(_, _)
| ty::Infer(_)
| ty::Error(_)
| ty::Placeholder(_) => {
bug!("When Place is Deref it's type shouldn't be {place_ty:#?}")
}
},
MoveSubPath::Field(_) => match place_ty.kind() {
ty::Adt(adt, _) => {
if adt.has_dtor(tcx) {
return;
}
if adt.is_union() {
union_path.get_or_insert(base);
}
}
ty::Closure(..)
| ty::CoroutineClosure(..)
| ty::Coroutine(_, _)
| ty::Tuple(_) => (),
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Foreign(_)
| ty::Str
| ty::Array(_, _)
| ty::Pat(_, _)
| ty::Slice(_)
| ty::RawPtr(_, _)
| ty::Ref(_, _, _)
| ty::FnDef(_, _)
| ty::FnPtr(..)
| ty::Dynamic(_, _)
| ty::CoroutineWitness(..)
| ty::Never
| ty::UnsafeBinder(_)
| ty::Alias(_, _)
| ty::Param(_)
| ty::Bound(_, _)
| ty::Infer(_)
| ty::Error(_)
| ty::Placeholder(_) => bug!(
"When Place contains ProjectionElem::Field its type shouldn't be {place_ty:#?}"
),
},
MoveSubPath::ConstantIndex(_) => match place_ty.kind() {
ty::Slice(_) => {
return;
}
ty::Array(_, _) => (),
_ => bug!("Unexpected type {:#?}", place_ty.is_array()),
},
MoveSubPath::Downcast(_) => (),
MoveSubPath::UnwrapUnsafeBinder => (),
};
move_elem
}
ProjectionElem::Index(_) => match place_ty.kind() {
ty::Array(..) | ty::Slice(_) => {
return MovePathResult::Error;
// Split `Subslice` patterns into the corresponding list of
// `ConstIndex` patterns. This is done to ensure that all move paths
// are disjoint, which is expected by drop elaboration.
MoveSubPathResult::Subslice { from, to } => {
assert!(
iter.all(
|(_, elem)| MoveSubPath::of(elem.kind()) == MoveSubPathResult::Skip
)
);
drop(iter); // drop for borrowck
let (&elem_ty, len) = match place_ty.kind() {
ty::Array(ty, size) => (
ty,
size.try_to_target_usize(self.tcx)
.expect("expected subslice projection on fixed-size array"),
),
_ => bug!("from_end: false slice pattern of non-array type"),
};
if !(self.filter)(elem_ty) {
return;
}
_ => bug!("Unexpected type {place_ty:#?}"),
},
ProjectionElem::UnwrapUnsafeBinder(_) => {}
// `OpaqueCast`:Only transmutes the type, so no moves there.
// `Downcast` :Only changes information about a `Place` without moving.
// So it's safe to skip these.
ProjectionElem::OpaqueCast(_) | ProjectionElem::Downcast(_, _) => (),
}
for offset in from..to {
let place_elem =
PlaceElem::ConstantIndex { offset, min_length: len, from_end: false };
let subpath_elem = MoveSubPath::ConstantIndex(offset);
let mpi = self.add_move_path(base, subpath_elem, |tcx| {
place_ref.project_deeper(&[place_elem], tcx)
});
on_move(self, mpi);
}
return;
}
MoveSubPathResult::Skip => continue,
MoveSubPathResult::Stop => return,
};
let elem_ty = PlaceTy::from_ty(place_ty).projection_ty(tcx, elem).ty;
if !(self.filter)(elem_ty) {
return MovePathResult::Error;
return;
}
if union_path.is_none() {
// inlined from add_move_path because of a borrowck conflict with the iterator
base =
*data.rev_lookup.projections.entry((base, elem.kind())).or_insert_with(|| {
new_move_path(
&mut data.move_paths,
&mut data.path_map,
&mut data.init_path_map,
Some(base),
place_ref.project_deeper(&[elem], tcx),
)
})
base = *data.rev_lookup.projections.entry((base, move_elem)).or_insert_with(|| {
new_move_path(
&mut data.move_paths,
&mut data.path_map,
&mut data.init_path_map,
Some(base),
place_ref.project_deeper(&[elem], tcx),
)
})
}
}
drop(iter); // drop for borrowck
if let Some(base) = union_path {
// Move out of union - always move the entire union.
MovePathResult::Union(base)
on_move(self, base);
} else {
MovePathResult::Path(base)
on_move(self, base);
}
}
fn add_move_path(
&mut self,
base: MovePathIndex,
elem: PlaceElem<'tcx>,
elem: MoveSubPath,
mk_place: impl FnOnce(TyCtxt<'tcx>) -> Place<'tcx>,
) -> MovePathIndex {
let MoveDataBuilder {
@ -268,7 +309,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> {
tcx,
..
} = self;
*rev_lookup.projections.entry((base, elem.kind())).or_insert_with(move || {
*rev_lookup.projections.entry((base, elem)).or_insert_with(move || {
new_move_path(move_paths, path_map, init_path_map, Some(base), mk_place(*tcx))
})
}
@ -276,7 +317,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> {
fn create_move_path(&mut self, place: Place<'tcx>) {
// This is an non-moving access (such as an overwrite or
// drop), so this not being a valid move path is OK.
let _ = self.move_path_for(place);
self.move_path_for(place, |_, _| ());
}
fn finalize(self) -> MoveData<'tcx> {
@ -525,46 +566,7 @@ impl<'a, 'tcx, F: Fn(Ty<'tcx>) -> bool> MoveDataBuilder<'a, 'tcx, F> {
fn gather_move(&mut self, place: Place<'tcx>) {
debug!("gather_move({:?}, {:?})", self.loc, place);
if let [ref base @ .., ProjectionElem::Subslice { from, to, from_end: false }] =
**place.projection
{
// Split `Subslice` patterns into the corresponding list of
// `ConstIndex` patterns. This is done to ensure that all move paths
// are disjoint, which is expected by drop elaboration.
let base_place =
Place { local: place.local, projection: self.tcx.mk_place_elems(base) };
let base_path = match self.move_path_for(base_place) {
MovePathResult::Path(path) => path,
MovePathResult::Union(path) => {
self.record_move(place, path);
return;
}
MovePathResult::Error => {
return;
}
};
let base_ty = base_place.ty(self.body, self.tcx).ty;
let len: u64 = match base_ty.kind() {
ty::Array(_, size) => size
.try_to_target_usize(self.tcx)
.expect("expected subslice projection on fixed-size array"),
_ => bug!("from_end: false slice pattern of non-array type"),
};
for offset in from..to {
let elem =
ProjectionElem::ConstantIndex { offset, min_length: len, from_end: false };
let path =
self.add_move_path(base_path, elem, |tcx| tcx.mk_place_elem(base_place, elem));
self.record_move(place, path);
}
} else {
match self.move_path_for(place) {
MovePathResult::Path(path) | MovePathResult::Union(path) => {
self.record_move(place, path)
}
MovePathResult::Error => {}
};
}
self.move_path_for(place, |this, mpi| this.record_move(place, mpi));
}
fn record_move(&mut self, place: Place<'tcx>, path: MovePathIndex) {

View file

@ -1,18 +1,9 @@
//! The move-analysis portion of borrowck needs to work in an abstract domain of lifted `Place`s.
//! Most of the `Place` variants fall into a one-to-one mapping between the concrete and abstract
//! (e.g., a field projection on a local variable, `x.field`, has the same meaning in both
//! domains). In other words, all field projections for the same field on the same local do not
//! have meaningfully different types if ever. Indexed projections are the exception: `a[x]` needs
//! to be treated as mapping to the same move path as `a[y]` as well as `a[13]`, etc. So we map
//! these `x`/`y` values to `()`.
//!
//! (In theory, the analysis could be extended to work with sets of paths, so that `a[0]` and
//! `a[13]` could be kept distinct, while `a[x]` would still overlap them both. But that is not
//! what this representation does today.)
//! [`MovePath`]s track the initialization state of places and their sub-paths.
use std::fmt;
use std::ops::{Index, IndexMut};
use rustc_abi::{FieldIdx, VariantIdx};
use rustc_data_structures::fx::FxHashMap;
use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::mir::*;
@ -309,7 +300,7 @@ pub struct MovePathLookup<'tcx> {
/// subsequent search so that it is solely relative to that
/// base-place). For the remaining lookup, we map the projection
/// elem to the associated MovePathIndex.
projections: FxHashMap<(MovePathIndex, ProjectionKind), MovePathIndex>,
projections: FxHashMap<(MovePathIndex, MoveSubPath), MovePathIndex>,
un_derefer: UnDerefer<'tcx>,
}
@ -333,7 +324,14 @@ impl<'tcx> MovePathLookup<'tcx> {
};
for (_, elem) in self.un_derefer.iter_projections(place) {
if let Some(&subpath) = self.projections.get(&(result, elem.kind())) {
let subpath = match MoveSubPath::of(elem.kind()) {
MoveSubPathResult::One(kind) => self.projections.get(&(result, kind)),
MoveSubPathResult::Subslice { .. } => None, // just use the parent MovePath
MoveSubPathResult::Skip => continue,
MoveSubPathResult::Stop => None,
};
if let Some(&subpath) = subpath {
result = subpath;
} else {
return LookupResult::Parent(Some(result));
@ -390,3 +388,58 @@ impl<'tcx> MoveData<'tcx> {
self.move_paths[root].find_descendant(&self.move_paths, pred)
}
}
/// A projection into a move path producing a child path
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum MoveSubPath {
Deref,
Field(FieldIdx),
ConstantIndex(u64),
Downcast(VariantIdx),
UnwrapUnsafeBinder,
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum MoveSubPathResult {
One(MoveSubPath),
Subslice { from: u64, to: u64 },
Skip,
Stop,
}
impl MoveSubPath {
pub fn of(elem: ProjectionKind) -> MoveSubPathResult {
let subpath = match elem {
// correspond to a MoveSubPath
ProjectionKind::Deref => MoveSubPath::Deref,
ProjectionKind::Field(idx, _) => MoveSubPath::Field(idx),
ProjectionKind::ConstantIndex { offset, min_length: _, from_end: false } => {
MoveSubPath::ConstantIndex(offset)
}
ProjectionKind::Downcast(_, idx) => MoveSubPath::Downcast(idx),
ProjectionKind::UnwrapUnsafeBinder(_) => MoveSubPath::UnwrapUnsafeBinder,
// this should be the same move path as its parent
// its fine to skip because it cannot have sibling move paths
// and it is not a user visible path
ProjectionKind::OpaqueCast(_) => {
return MoveSubPathResult::Skip;
}
// these cannot be moved through
ProjectionKind::Index(_)
| ProjectionKind::ConstantIndex { offset: _, min_length: _, from_end: true }
| ProjectionKind::Subslice { from: _, to: _, from_end: true } => {
return MoveSubPathResult::Stop;
}
// subslice is special.
// it needs to be split into individual move paths
ProjectionKind::Subslice { from, to, from_end: false } => {
return MoveSubPathResult::Subslice { from, to };
}
};
MoveSubPathResult::One(subpath)
}
}

View file

@ -655,20 +655,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
location: Location,
) {
match elem {
ProjectionElem::OpaqueCast(ty)
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) =>
{
self.fail(
location,
format!("explicit opaque type cast to `{ty}` after `PostAnalysisNormalize`"),
)
}
ProjectionElem::Index(index) => {
let index_ty = self.body.local_decls[index].ty;
if index_ty != self.tcx.types.usize {
self.fail(location, format!("bad index ({index_ty} != usize)"))
}
}
ProjectionElem::Deref
if self.body.phase >= MirPhase::Runtime(RuntimePhase::PostCleanup) =>
{
@ -815,6 +801,78 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
}
ProjectionElem::Index(index) => {
let indexed_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty;
match indexed_ty.kind() {
ty::Array(_, _) | ty::Slice(_) => {}
_ => self.fail(location, format!("{indexed_ty:?} cannot be indexed")),
}
let index_ty = self.body.local_decls[index].ty;
if index_ty != self.tcx.types.usize {
self.fail(location, format!("bad index ({index_ty} != usize)"))
}
}
ProjectionElem::ConstantIndex { offset, min_length, from_end } => {
let indexed_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty;
match indexed_ty.kind() {
ty::Array(_, _) => {
if from_end {
self.fail(location, "arrays should not be indexed from end");
}
}
ty::Slice(_) => {}
_ => self.fail(location, format!("{indexed_ty:?} cannot be indexed")),
}
if from_end {
if offset > min_length {
self.fail(
location,
format!(
"constant index with offset -{offset} out of bounds of min length {min_length}"
),
);
}
} else {
if offset >= min_length {
self.fail(
location,
format!(
"constant index with offset {offset} out of bounds of min length {min_length}"
),
);
}
}
}
ProjectionElem::Subslice { from, to, from_end } => {
let indexed_ty = place_ref.ty(&self.body.local_decls, self.tcx).ty;
match indexed_ty.kind() {
ty::Array(_, _) => {
if from_end {
self.fail(location, "arrays should not be subsliced from end");
}
}
ty::Slice(_) => {
if !from_end {
self.fail(location, "slices should be subsliced from end");
}
}
_ => self.fail(location, format!("{indexed_ty:?} cannot be indexed")),
}
if !from_end && from > to {
self.fail(location, "backwards subslice {from}..{to}");
}
}
ProjectionElem::OpaqueCast(ty)
if self.body.phase >= MirPhase::Runtime(RuntimePhase::Initial) =>
{
self.fail(
location,
format!("explicit opaque type cast to `{ty}` after `PostAnalysisNormalize`"),
)
}
ProjectionElem::UnwrapUnsafeBinder(unwrapped_ty) => {
let binder_ty = place_ref.ty(&self.body.local_decls, self.tcx);
let ty::UnsafeBinder(binder_ty) = *binder_ty.ty.kind() else {
@ -921,6 +979,25 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
}
if self.body.phase < MirPhase::Runtime(RuntimePhase::Initial)
&& let Some(i) = place
.projection
.iter()
.position(|elem| matches!(elem, ProjectionElem::Subslice { .. }))
&& let Some(tail) = place.projection.get(i + 1..)
&& tail.iter().any(|elem| {
matches!(
elem,
ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. }
)
})
{
self.fail(
location,
format!("place {place:?} has `ConstantIndex` or `Subslice` after `Subslice`"),
);
}
self.super_place(place, cntxt, location);
}

View file

@ -67,8 +67,6 @@ We don't actually create a move-path for **every** [`Place`] that gets
used. In particular, if it is illegal to move from a [`Place`], then
there is no need for a [`MovePathIndex`]. Some examples:
- You cannot move from a static variable, so we do not create a [`MovePathIndex`]
for static variables.
- You cannot move an individual element of an array, so if we have e.g. `foo: [String; 3]`,
there would be no move-path for `foo[1]`.
- You cannot move from inside of a borrowed reference, so if we have e.g. `foo: &String`,
@ -82,6 +80,18 @@ initialized (which lowers overhead).
[`move_path_for`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir_dataflow/move_paths/builder/struct.MoveDataBuilder.html#method.move_path_for
## Projections
Instead of using [`PlaceElem`], projections in move paths are stored as [`MoveSubPath`]s.
Projections that can't be moved out of and projections that can be skipped are not represented.
Subslice projections of arrays (produced by slice patterns) are special; they're turned into
multiple [`ConstantIndex`] subpaths, one for each element in the subslice.
[`PlaceElem`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/type.PlaceElem.html
[`MoveSubPath`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir_dataflow/move_paths/enum.MoveSubPath.html
[`ConstantIndex`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir_dataflow/move_paths/enum.MoveSubPath.html#variant.ConstantIndex
## Looking up a move-path
If you have a [`Place`] and you would like to convert it to a [`MovePathIndex`], you