Skip to content

Commit de0abf7

Browse files
borsbors
committed
Auto merge of #66650 - matthewjasper:nonuniform-array-move, r=pnkfelix
Remove uniform array move MIR passes This PR fixes a number of bugs caused by limitations of this pass * Projections from constant indexes weren't being canonicalized * Constant indexes from the start weren't being canonicalized (they could have different min_lengths) * It didn't apply to non-moves This PR makes the following changes to support removing this pass: * ConstantIndex of arrays are now generated in a canonical form (from the start, min_length is the actual length). * Subslices are now split when generating move paths and when checking subslices have been moved. Additionally * The parent move path of a projection from an array element is now calculated correctly closes #66502
2 parents 27d6f55 + d96485d commit de0abf7

32 files changed

+1131
-631
lines changed

‎src/librustc/mir/mod.rs

+17-7
Original file line numberDiff line numberDiff line change
@@ -1714,18 +1714,25 @@ pub enum ProjectionElem<V, T> {
17141714
ConstantIndex {
17151715
/// index or -index (in Python terms), depending on from_end
17161716
offset: u32,
1717-
/// thing being indexed must be at least this long
1717+
/// The thing being indexed must be at least this long. For arrays this
1718+
/// is always the exact length.
17181719
min_length: u32,
1719-
/// counting backwards from end?
1720+
/// Counting backwards from end? This is always false when indexing an
1721+
/// array.
17201722
from_end: bool,
17211723
},
17221724

17231725
/// These indices are generated by slice patterns.
17241726
///
1725-
/// slice[from:-to] in Python terms.
1727+
/// If `from_end` is true `slice[from..slice.len() - to]`.
1728+
/// Otherwise `array[from..to]`.
17261729
Subslice {
17271730
from: u32,
17281731
to: u32,
1732+
/// Whether `to` counts from the start or end of the array/slice.
1733+
/// For `PlaceElem`s this is `true` if and only if the base is a slice.
1734+
/// For `ProjectionKind`, this can also be `true` for arrays.
1735+
from_end: bool,
17291736
},
17301737

17311738
/// "Downcast" to a variant of an ADT. Currently, we only introduce
@@ -1914,15 +1921,18 @@ impl Debug for Place<'_> {
19141921
ProjectionElem::ConstantIndex { offset, min_length, from_end: true } => {
19151922
write!(fmt, "[-{:?} of {:?}]", offset, min_length)?;
19161923
}
1917-
ProjectionElem::Subslice { from, to } if *to == 0 => {
1924+
ProjectionElem::Subslice { from, to, from_end: true } if *to == 0 => {
19181925
write!(fmt, "[{:?}:]", from)?;
19191926
}
1920-
ProjectionElem::Subslice { from, to } if *from == 0 => {
1927+
ProjectionElem::Subslice { from, to, from_end: true } if *from == 0 => {
19211928
write!(fmt, "[:-{:?}]", to)?;
19221929
}
1923-
ProjectionElem::Subslice { from, to } => {
1930+
ProjectionElem::Subslice { from, to, from_end: true } => {
19241931
write!(fmt, "[{:?}:-{:?}]", from, to)?;
19251932
}
1933+
ProjectionElem::Subslice { from, to, from_end: false } => {
1934+
write!(fmt, "[{:?}..{:?}]", from, to)?;
1935+
}
19261936
}
19271937
}
19281938

@@ -2456,7 +2466,7 @@ impl UserTypeProjection {
24562466
}
24572467

24582468
pub(crate) fn subslice(mut self, from: u32, to: u32) -> Self {
2459-
self.projs.push(ProjectionElem::Subslice { from, to });
2469+
self.projs.push(ProjectionElem::Subslice { from, to, from_end: true });
24602470
self
24612471
}
24622472

‎src/librustc/mir/tcx.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,17 @@ impl<'tcx> PlaceTy<'tcx> {
8888
}
8989
ProjectionElem::Index(_) | ProjectionElem::ConstantIndex { .. } =>
9090
PlaceTy::from_ty(self.ty.builtin_index().unwrap()),
91-
ProjectionElem::Subslice { from, to } => {
91+
ProjectionElem::Subslice { from, to, from_end } => {
9292
PlaceTy::from_ty(match self.ty.kind {
93-
ty::Array(inner, size) => {
93+
ty::Slice(..) => self.ty,
94+
ty::Array(inner, _) if !from_end => {
95+
tcx.mk_array(inner, (to - from) as u64)
96+
}
97+
ty::Array(inner, size) if from_end => {
9498
let size = size.eval_usize(tcx, param_env);
9599
let len = size - (from as u64) - (to as u64);
96100
tcx.mk_array(inner, len)
97101
}
98-
ty::Slice(..) => self.ty,
99102
_ => {
100103
bug!("cannot subslice non-array type: `{:?}`", self)
101104
}

‎src/librustc/mir/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -954,7 +954,7 @@ macro_rules! visit_place_fns {
954954
);
955955
}
956956
ProjectionElem::Deref |
957-
ProjectionElem::Subslice { from: _, to: _ } |
957+
ProjectionElem::Subslice { from: _, to: _, from_end: _ } |
958958
ProjectionElem::ConstantIndex { offset: _,
959959
min_length: _,
960960
from_end: _ } |

‎src/librustc_codegen_ssa/mir/place.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -565,14 +565,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
565565
let llindex = bx.sub(lllen, lloffset);
566566
cg_base.project_index(bx, llindex)
567567
}
568-
mir::ProjectionElem::Subslice { from, to } => {
568+
mir::ProjectionElem::Subslice { from, to, from_end } => {
569569
let mut subslice = cg_base.project_index(bx,
570570
bx.cx().const_usize(*from as u64));
571571
let projected_ty = PlaceTy::from_ty(cg_base.layout.ty)
572572
.projection_ty(tcx, elem).ty;
573573
subslice.layout = bx.cx().layout_of(self.monomorphize(&projected_ty));
574574

575575
if subslice.layout.is_unsized() {
576+
assert!(from_end, "slice subslices should be `from_end`");
576577
subslice.llextra = Some(bx.sub(cg_base.llextra.unwrap(),
577578
bx.cx().const_usize((*from as u64) + (*to as u64))));
578579
}

‎src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
7878
.collect();
7979

8080
if move_out_indices.is_empty() {
81-
let root_place = self
82-
.prefixes(used_place, PrefixSet::All)
83-
.last()
84-
.unwrap();
81+
let root_place = PlaceRef { projection: &[], ..used_place };
8582

8683
if !self.uninitialized_error_reported.insert(root_place) {
8784
debug!(

‎src/librustc_mir/borrow_check/mod.rs

+71-17
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ fn do_mir_borrowck<'a, 'tcx>(
174174

175175
let mut errors_buffer = Vec::new();
176176
let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<(Place<'tcx>, MoveError<'tcx>)>>) =
177-
match MoveData::gather_moves(&body, tcx) {
177+
match MoveData::gather_moves(&body, tcx, param_env) {
178178
Ok(move_data) => (move_data, None),
179179
Err((move_data, move_errors)) => (move_data, Some(move_errors)),
180180
};
@@ -1600,7 +1600,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
16001600
(prefix, place_span.0, place_span.1),
16011601
mpi,
16021602
);
1603-
return; // don't bother finding other problems.
16041603
}
16051604
}
16061605
Err(NoMovePathFound::ReachedStatic) => {
@@ -1614,6 +1613,46 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
16141613
}
16151614
}
16161615

1616+
/// Subslices correspond to multiple move paths, so we iterate through the
1617+
/// elements of the base array. For each element we check
1618+
///
1619+
/// * Does this element overlap with our slice.
1620+
/// * Is any part of it uninitialized.
1621+
fn check_if_subslice_element_is_moved(
1622+
&mut self,
1623+
location: Location,
1624+
desired_action: InitializationRequiringAction,
1625+
place_span: (PlaceRef<'cx, 'tcx>, Span),
1626+
maybe_uninits: &FlowAtLocation<'tcx, MaybeUninitializedPlaces<'cx, 'tcx>>,
1627+
from: u32,
1628+
to: u32,
1629+
) {
1630+
if let Some(mpi) = self.move_path_for_place(place_span.0) {
1631+
let mut child = self.move_data.move_paths[mpi].first_child;
1632+
while let Some(child_mpi) = child {
1633+
let child_move_place = &self.move_data.move_paths[child_mpi];
1634+
let child_place = &child_move_place.place;
1635+
let last_proj = child_place.projection.last().unwrap();
1636+
if let ProjectionElem::ConstantIndex { offset, from_end, .. } = last_proj {
1637+
debug_assert!(!from_end, "Array constant indexing shouldn't be `from_end`.");
1638+
1639+
if (from..to).contains(offset) {
1640+
if let Some(uninit_child) = maybe_uninits.has_any_child_of(child_mpi) {
1641+
self.report_use_of_moved_or_uninitialized(
1642+
location,
1643+
desired_action,
1644+
(place_span.0, place_span.0, place_span.1),
1645+
uninit_child,
1646+
);
1647+
return; // don't bother finding other problems.
1648+
}
1649+
}
1650+
}
1651+
child = child_move_place.next_sibling;
1652+
}
1653+
}
1654+
}
1655+
16171656
fn check_if_path_or_subpath_is_moved(
16181657
&mut self,
16191658
location: Location,
@@ -1640,6 +1679,30 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
16401679

16411680
self.check_if_full_path_is_moved(location, desired_action, place_span, flow_state);
16421681

1682+
if let [
1683+
base_proj @ ..,
1684+
ProjectionElem::Subslice { from, to, from_end: false },
1685+
] = place_span.0.projection {
1686+
let place_ty = Place::ty_from(
1687+
place_span.0.base,
1688+
base_proj,
1689+
self.body(),
1690+
self.infcx.tcx,
1691+
);
1692+
if let ty::Array(..) = place_ty.ty.kind {
1693+
let array_place = PlaceRef { base: place_span.0.base, projection: base_proj };
1694+
self.check_if_subslice_element_is_moved(
1695+
location,
1696+
desired_action,
1697+
(array_place, place_span.1),
1698+
maybe_uninits,
1699+
*from,
1700+
*to,
1701+
);
1702+
return;
1703+
}
1704+
}
1705+
16431706
// A move of any shallow suffix of `place` also interferes
16441707
// with an attempt to use `place`. This is scenario 3 above.
16451708
//
@@ -1675,25 +1738,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
16751738
/// static variable, as we do not track those in the MoveData.
16761739
fn move_path_closest_to(
16771740
&mut self,
1678-
place: PlaceRef<'cx, 'tcx>,
1741+
place: PlaceRef<'_, 'tcx>,
16791742
) -> Result<(PlaceRef<'cx, 'tcx>, MovePathIndex), NoMovePathFound> {
1680-
let mut last_prefix = place.base;
1681-
1682-
for prefix in self.prefixes(place, PrefixSet::All) {
1683-
if let Some(mpi) = self.move_path_for_place(prefix) {
1684-
return Ok((prefix, mpi));
1685-
}
1686-
1687-
last_prefix = prefix.base;
1688-
}
1689-
1690-
match last_prefix {
1691-
PlaceBase::Local(_) => panic!("should have move path for every Local"),
1692-
PlaceBase::Static(_) => Err(NoMovePathFound::ReachedStatic),
1743+
match self.move_data.rev_lookup.find(place) {
1744+
LookupResult::Parent(Some(mpi))
1745+
| LookupResult::Exact(mpi) => Ok((self.move_data.move_paths[mpi].place.as_ref(), mpi)),
1746+
LookupResult::Parent(None) => Err(NoMovePathFound::ReachedStatic),
16931747
}
16941748
}
16951749

1696-
fn move_path_for_place(&mut self, place: PlaceRef<'cx, 'tcx>) -> Option<MovePathIndex> {
1750+
fn move_path_for_place(&mut self, place: PlaceRef<'_, 'tcx>) -> Option<MovePathIndex> {
16971751
// If returns None, then there is no move path corresponding
16981752
// to a direct owner of `place` (which means there is nothing
16991753
// that borrowck tracks for its analysis).

‎src/librustc_mir/borrow_check/nll/type_check/mod.rs

+8-15
Original file line numberDiff line numberDiff line change
@@ -675,23 +675,16 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
675675
}),
676676
)
677677
}
678-
ProjectionElem::Subslice { from, to } => PlaceTy::from_ty(
678+
ProjectionElem::Subslice { from, to, from_end } => PlaceTy::from_ty(
679679
match base_ty.kind {
680-
ty::Array(inner, size) => {
681-
let size = size.eval_usize(tcx, self.cx.param_env);
682-
let min_size = (from as u64) + (to as u64);
683-
if let Some(rest_size) = size.checked_sub(min_size) {
684-
tcx.mk_array(inner, rest_size)
685-
} else {
686-
span_mirbug_and_err!(
687-
self,
688-
place,
689-
"taking too-small slice of {:?}",
690-
base_ty
691-
)
692-
}
680+
ty::Array(inner, _) => {
681+
assert!(!from_end, "array subslices should not use from_end");
682+
tcx.mk_array(inner, (to - from) as u64)
693683
}
694-
ty::Slice(..) => base_ty,
684+
ty::Slice(..) => {
685+
assert!(from_end, "slice subslices should use from_end");
686+
base_ty
687+
},
695688
_ => span_mirbug_and_err!(self, place, "slice of non-array {:?}", base_ty),
696689
},
697690
),

‎src/librustc_mir/borrow_check/places_conflict.rs

+35-7
Original file line numberDiff line numberDiff line change
@@ -503,34 +503,62 @@ fn place_projection_conflict<'tcx>(
503503
Overlap::Disjoint
504504
}
505505
}
506+
(
507+
ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false },
508+
ProjectionElem::Subslice { from, to, from_end: false }
509+
)
510+
| (
511+
ProjectionElem::Subslice { from, to, from_end: false },
512+
ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }
513+
) => {
514+
if (from..to).contains(&offset) {
515+
debug!("place_element_conflict: DISJOINT-OR-EQ-ARRAY-CONSTANT-INDEX-SUBSLICE");
516+
Overlap::EqualOrDisjoint
517+
} else {
518+
debug!("place_element_conflict: DISJOINT-ARRAY-CONSTANT-INDEX-SUBSLICE");
519+
Overlap::Disjoint
520+
}
521+
}
506522
(ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false },
507523
ProjectionElem::Subslice {from, .. })
508524
| (ProjectionElem::Subslice {from, .. },
509525
ProjectionElem::ConstantIndex { offset, min_length: _, from_end: false }) => {
510526
if offset >= from {
511527
debug!(
512-
"place_element_conflict: DISJOINT-OR-EQ-ARRAY-CONSTANT-INDEX-SUBSLICE");
528+
"place_element_conflict: DISJOINT-OR-EQ-SLICE-CONSTANT-INDEX-SUBSLICE");
513529
Overlap::EqualOrDisjoint
514530
} else {
515-
debug!("place_element_conflict: DISJOINT-ARRAY-CONSTANT-INDEX-SUBSLICE");
531+
debug!("place_element_conflict: DISJOINT-SLICE-CONSTANT-INDEX-SUBSLICE");
516532
Overlap::Disjoint
517533
}
518534
}
519535
(ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true },
520-
ProjectionElem::Subslice {from: _, to })
521-
| (ProjectionElem::Subslice {from: _, to },
536+
ProjectionElem::Subslice { to, from_end: true, .. })
537+
| (ProjectionElem::Subslice { to, from_end: true, .. },
522538
ProjectionElem::ConstantIndex { offset, min_length: _, from_end: true }) => {
523539
if offset > to {
524540
debug!("place_element_conflict: \
525-
DISJOINT-OR-EQ-ARRAY-CONSTANT-INDEX-SUBSLICE-FE");
541+
DISJOINT-OR-EQ-SLICE-CONSTANT-INDEX-SUBSLICE-FE");
526542
Overlap::EqualOrDisjoint
527543
} else {
528-
debug!("place_element_conflict: DISJOINT-ARRAY-CONSTANT-INDEX-SUBSLICE-FE");
544+
debug!("place_element_conflict: DISJOINT-SLICE-CONSTANT-INDEX-SUBSLICE-FE");
545+
Overlap::Disjoint
546+
}
547+
}
548+
(
549+
ProjectionElem::Subslice { from: f1, to: t1, from_end: false },
550+
ProjectionElem::Subslice { from: f2, to: t2, from_end: false }
551+
) => {
552+
if f2 >= t1 || f1 >= t2 {
553+
debug!("place_element_conflict: DISJOINT-ARRAY-SUBSLICES");
529554
Overlap::Disjoint
555+
} else {
556+
debug!("place_element_conflict: DISJOINT-OR-EQ-ARRAY-SUBSLICES");
557+
Overlap::EqualOrDisjoint
530558
}
531559
}
532560
(ProjectionElem::Subslice { .. }, ProjectionElem::Subslice { .. }) => {
533-
debug!("place_element_conflict: DISJOINT-OR-EQ-ARRAY-SUBSLICES");
561+
debug!("place_element_conflict: DISJOINT-OR-EQ-SLICE-SUBSLICES");
534562
Overlap::EqualOrDisjoint
535563
}
536564
(ProjectionElem::Deref, _)

0 commit comments

Comments
 (0)