From 22b1acb45500f34bb100f7da4d5e90fcc37bca92 Mon Sep 17 00:00:00 2001 From: ouz-a Date: Tue, 12 Sep 2023 10:28:37 +0300 Subject: [PATCH 1/5] match on elem first --- .../src/move_paths/builder.rs | 78 +++++++++++-------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index 2e3b9577b5030..e5c627ade8e35 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -115,44 +115,56 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { let body = self.builder.body; let tcx = self.builder.tcx; let place_ty = place_ref.ty(body, tcx).ty; - match place_ty.kind() { - ty::Ref(..) | ty::RawPtr(..) => { - return Err(MoveError::cannot_move_out_of( - self.loc, - BorrowedContent { target_place: place_ref.project_deeper(&[elem], tcx) }, - )); - } - ty::Adt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => { - return Err(MoveError::cannot_move_out_of( - self.loc, - InteriorOfTypeWithDestructor { container_ty: place_ty }, - )); - } - ty::Adt(adt, _) if adt.is_union() => { - union_path.get_or_insert(base); - } - ty::Slice(_) => { - return Err(MoveError::cannot_move_out_of( - self.loc, - InteriorOfSliceOrArray { - ty: place_ty, - is_index: matches!(elem, ProjectionElem::Index(..)), - }, - )); - } - - ty::Array(..) => { - if let ProjectionElem::Index(..) = elem { + match elem { + ProjectionElem::Deref => match place_ty.kind() { + ty::Ref(..) | ty::RawPtr(..) => { return Err(MoveError::cannot_move_out_of( self.loc, - InteriorOfSliceOrArray { ty: place_ty, is_index: true }, + BorrowedContent { + target_place: place_ref.project_deeper(&[elem], tcx), + }, )); } - } - - _ => {} - }; + _ => (), + }, + ProjectionElem::Field(_, _) + | ProjectionElem::OpaqueCast(_) + | ProjectionElem::Downcast(_, _) => match place_ty.kind() { + ty::Adt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfTypeWithDestructor { container_ty: place_ty }, + )); + } + ty::Adt(adt, _) if adt.is_union() => { + union_path.get_or_insert(base); + } + _ => (), + }, + ProjectionElem::ConstantIndex { .. } + | ProjectionElem::Index(_) + | ProjectionElem::Subslice { .. } => match place_ty.kind() { + ty::Slice(_) => { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { + ty: place_ty, + is_index: matches!(elem, ProjectionElem::Index(..)), + }, + )); + } + ty::Array(..) => { + if let ProjectionElem::Index(..) = elem { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { ty: place_ty, is_index: true }, + )); + } + } + _ => (), + }, + } if union_path.is_none() { // inlined from add_move_path because of a borrowck conflict with the iterator base = From 0cb22a66eb577254b8a507ef75b7f10728b396ca Mon Sep 17 00:00:00 2001 From: ouz-a Date: Tue, 12 Sep 2023 21:09:29 +0300 Subject: [PATCH 2/5] very verbose error handling --- .../src/move_paths/builder.rs | 106 ++++++++++++++---- 1 file changed, 86 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index e5c627ade8e35..31efa26a6aae4 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -125,35 +125,88 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { }, )); } - _ => (), + ty::Adt(adt, _) => { + if !adt.is_box() { + bug!("Adt should be a box type"); + } + } + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Foreign(_) + | ty::Str + | ty::Array(_, _) + | ty::Slice(_) + | ty::FnDef(_, _) + | ty::FnPtr(_) + | ty::Dynamic(_, _, _) + | ty::Closure(_, _) + | ty::Generator(_, _, _) + | ty::GeneratorWitness(_) + | ty::GeneratorWitnessMIR(_, _) + | ty::Never + | ty::Tuple(_) + | ty::Alias(_, _) + | ty::Param(_) + | ty::Bound(_, _) + | ty::Infer(_) + | ty::Error(_) + | ty::Placeholder(_) => bug!("Place has a wrong type {place_ty:#?}"), }, - ProjectionElem::Field(_, _) - | ProjectionElem::OpaqueCast(_) - | ProjectionElem::Downcast(_, _) => match place_ty.kind() { - ty::Adt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => { + ProjectionElem::Field(_, _) => match place_ty.kind() { + ty::Adt(adt, _) if adt.has_dtor(tcx) => { return Err(MoveError::cannot_move_out_of( self.loc, InteriorOfTypeWithDestructor { container_ty: place_ty }, )); } - ty::Adt(adt, _) if adt.is_union() => { - union_path.get_or_insert(base); + ty::Adt(adt, _) => { + if adt.is_union() { + union_path.get_or_insert(base); + } } - - _ => (), + ty::Closure(_, _) | ty::Generator(_, _, _) | ty::Tuple(_) => (), + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Foreign(_) + | ty::Str + | ty::Array(_, _) + | ty::Slice(_) + | ty::RawPtr(_) + | ty::Ref(_, _, _) + | ty::FnDef(_, _) + | ty::FnPtr(_) + | ty::Dynamic(_, _, _) + | ty::GeneratorWitness(_) + | ty::GeneratorWitnessMIR(_, _) + | ty::Never + | ty::Alias(_, _) + | ty::Param(_) + | ty::Bound(_, _) + | ty::Infer(_) + | ty::Error(_) + | ty::Placeholder(_) => bug!("Place has a wrong type {place_ty:#?}"), }, - ProjectionElem::ConstantIndex { .. } - | ProjectionElem::Index(_) - | ProjectionElem::Subslice { .. } => match place_ty.kind() { - ty::Slice(_) => { - return Err(MoveError::cannot_move_out_of( - self.loc, - InteriorOfSliceOrArray { - ty: place_ty, - is_index: matches!(elem, ProjectionElem::Index(..)), - }, - )); + ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => { + match place_ty.kind() { + ty::Slice(_) => { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { + ty: place_ty, + is_index: matches!(elem, ProjectionElem::Index(..)), + }, + )); + } + _ => (), } + } + ProjectionElem::Index(_) => match place_ty.kind() { ty::Array(..) => { if let ProjectionElem::Index(..) = elem { return Err(MoveError::cannot_move_out_of( @@ -162,8 +215,21 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { )); } } + ty::Slice(_) => { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { + ty: place_ty, + is_index: matches!(elem, ProjectionElem::Index(..)), + }, + )); + } _ => (), }, + // `OpaqueCast` only transmutes the type, so no moves there and + // `Downcast` only changes information about a `Place` without moving + // So it's safe to skip these. + ProjectionElem::OpaqueCast(_) | ProjectionElem::Downcast(_, _) => (), } if union_path.is_none() { // inlined from add_move_path because of a borrowck conflict with the iterator From d6efedcaf578e67f07293349d2128c06043a239f Mon Sep 17 00:00:00 2001 From: ouz-a Date: Fri, 22 Sep 2023 11:21:55 +0300 Subject: [PATCH 3/5] remove inner match --- compiler/rustc_mir_dataflow/src/move_paths/builder.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index 31efa26a6aae4..d19b1a902f44c 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -208,12 +208,10 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { } ProjectionElem::Index(_) => match place_ty.kind() { ty::Array(..) => { - if let ProjectionElem::Index(..) = elem { - return Err(MoveError::cannot_move_out_of( - self.loc, - InteriorOfSliceOrArray { ty: place_ty, is_index: true }, - )); - } + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfSliceOrArray { ty: place_ty, is_index: true }, + )); } ty::Slice(_) => { return Err(MoveError::cannot_move_out_of( From 442c87a0b0168e644a40ef8ecd874c53fe7e04b8 Mon Sep 17 00:00:00 2001 From: ouz-a Date: Fri, 22 Sep 2023 11:31:14 +0300 Subject: [PATCH 4/5] better bug message --- compiler/rustc_mir_dataflow/src/move_paths/builder.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index d19b1a902f44c..8732489418ae0 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -127,7 +127,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { } ty::Adt(adt, _) => { if !adt.is_box() { - bug!("Adt should be a box type"); + bug!("Adt should be a box type when Place is deref"); } } ty::Bool @@ -153,7 +153,9 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | ty::Bound(_, _) | ty::Infer(_) | ty::Error(_) - | ty::Placeholder(_) => bug!("Place has a wrong type {place_ty:#?}"), + | 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) => { @@ -190,7 +192,9 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | ty::Bound(_, _) | ty::Infer(_) | ty::Error(_) - | ty::Placeholder(_) => bug!("Place has a wrong type {place_ty:#?}"), + | ty::Placeholder(_) => bug!( + "When Place contains ProjectionElem::Field it's type shouldn't be {place_ty:#?}" + ), }, ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => { match place_ty.kind() { From 63df126f59622f280dd34489eb7b3a03bdd7417d Mon Sep 17 00:00:00 2001 From: ouz-a Date: Fri, 22 Sep 2023 16:28:45 +0300 Subject: [PATCH 5/5] match array for constantindex and subslice --- .../src/move_paths/builder.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index 8732489418ae0..213b81eaac9a4 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -158,13 +158,13 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { } }, ProjectionElem::Field(_, _) => match place_ty.kind() { - ty::Adt(adt, _) if adt.has_dtor(tcx) => { - return Err(MoveError::cannot_move_out_of( - self.loc, - InteriorOfTypeWithDestructor { container_ty: place_ty }, - )); - } ty::Adt(adt, _) => { + if adt.has_dtor(tcx) { + return Err(MoveError::cannot_move_out_of( + self.loc, + InteriorOfTypeWithDestructor { container_ty: place_ty }, + )); + } if adt.is_union() { union_path.get_or_insert(base); } @@ -207,7 +207,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { }, )); } - _ => (), + ty::Array(_, _) => (), + _ => bug!("Unexpected type {:#?}", place_ty.is_array()), } } ProjectionElem::Index(_) => match place_ty.kind() { @@ -226,7 +227,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { }, )); } - _ => (), + _ => bug!("Unexpected type {place_ty:#?}"), }, // `OpaqueCast` only transmutes the type, so no moves there and // `Downcast` only changes information about a `Place` without moving