From 478071ba9daabcdbc880db5638989dc16545537c Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 2 Jul 2023 14:06:56 +0200 Subject: [PATCH 1/3] clean up struct layout code --- compiler/rustc_abi/src/layout.rs | 8 ++-- compiler/rustc_ty_utils/src/layout.rs | 69 ++++++++++++++------------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index 73f9deb3143..f6875d895d3 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -134,7 +134,7 @@ pub trait LayoutCalculator { scalar_valid_range: (Bound, Bound), discr_range_of_repr: impl Fn(i128, i128) -> (Integer, bool), discriminants: impl Iterator, - niche_optimize_enum: bool, + dont_niche_optimize_enum: bool, always_sized: bool, ) -> Option { let dl = self.current_data_layout(); @@ -183,10 +183,10 @@ pub trait LayoutCalculator { // (Typechecking will reject discriminant-sizing attrs.) let v = present_first; - let kind = if is_enum || variants[v].is_empty() { + let kind = if is_enum || variants[v].is_empty() || always_sized { StructKind::AlwaysSized } else { - if !always_sized { StructKind::MaybeUnsized } else { StructKind::AlwaysSized } + StructKind::MaybeUnsized }; let mut st = self.univariant(dl, &variants[v], repr, kind)?; @@ -280,7 +280,7 @@ pub trait LayoutCalculator { } let calculate_niche_filling_layout = || -> Option { - if niche_optimize_enum { + if dont_niche_optimize_enum { return None; } diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 9ef9120e294..f6bc0dc330d 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -463,38 +463,43 @@ fn layout_of_uncached<'tcx>( )); } - tcx.mk_layout( - cx.layout_of_struct_or_enum( - &def.repr(), - &variants, - def.is_enum(), - def.is_unsafe_cell(), - tcx.layout_scalar_valid_range(def.did()), - |min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max), - def.is_enum() - .then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128))) - .into_iter() - .flatten(), - def.repr().inhibit_enum_layout_opt() - || def - .variants() - .iter_enumerated() - .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())), - { - let param_env = tcx.param_env(def.did()); - def.is_struct() - && match def.variants().iter().next().and_then(|x| x.fields.raw.last()) - { - Some(last_field) => tcx - .type_of(last_field.did) - .subst_identity() - .is_sized(tcx, param_env), - None => false, - } - }, - ) - .ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))?, - ) + let get_discriminant_type = + |min, max| Integer::repr_discr(tcx, ty, &def.repr(), min, max); + + let discriminants_iter = || { + def.is_enum() + .then(|| def.discriminants(tcx).map(|(v, d)| (v, d.val as i128))) + .into_iter() + .flatten() + }; + + let dont_niche_optimize_enum = def.repr().inhibit_enum_layout_opt() + || def + .variants() + .iter_enumerated() + .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())); + + let maybe_unsized = def.is_struct() + && def.non_enum_variant().fields.raw.last().is_some_and(|last_field| { + let param_env = tcx.param_env(def.did()); + !tcx.type_of(last_field.did).subst_identity().is_sized(tcx, param_env) + }); + + let Some(layout) = cx.layout_of_struct_or_enum( + &def.repr(), + &variants, + def.is_enum(), + def.is_unsafe_cell(), + tcx.layout_scalar_valid_range(def.did()), + get_discriminant_type, + discriminants_iter(), + dont_niche_optimize_enum, + !maybe_unsized, + ) else { + return Err(error(cx, LayoutError::SizeOverflow(ty))); + }; + + tcx.mk_layout(layout) } // Types with no meaningful known layout. From e3de14e46396694e48bfecf963fe9fa8e8a90d86 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Sun, 2 Jul 2023 14:07:08 +0200 Subject: [PATCH 2/3] sanity check field offsets in unsizeable structs --- compiler/rustc_ty_utils/src/layout.rs | 49 +++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index f6bc0dc330d..0dbe6265522 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -499,6 +499,55 @@ fn layout_of_uncached<'tcx>( return Err(error(cx, LayoutError::SizeOverflow(ty))); }; + // If the struct tail is sized and can be unsized, check that unsizing doesn't move the fields around. + if cfg!(debug_assertions) + && maybe_unsized + && def + .non_enum_variant() + .fields + .raw + .last() + .unwrap() + .ty(tcx, substs) + .is_sized(tcx, cx.param_env) + { + let mut variants = variants; + let tail_replacement = cx.layout_of(Ty::new_slice(tcx, tcx.types.u8)).unwrap(); + *variants[FIRST_VARIANT].raw.last_mut().unwrap() = tail_replacement.layout; + + let Some(unsized_layout) = cx.layout_of_struct_or_enum( + &def.repr(), + &variants, + def.is_enum(), + def.is_unsafe_cell(), + tcx.layout_scalar_valid_range(def.did()), + get_discriminant_type, + discriminants_iter(), + dont_niche_optimize_enum, + !maybe_unsized, + ) else { + bug!("failed to compute unsized layout of {ty:?}"); + }; + + let FieldsShape::Arbitrary { offsets: sized_offsets, .. } = &layout.fields else { + bug!("unexpected FieldsShape for sized layout of {ty:?}: {:?}", layout.fields); + }; + let FieldsShape::Arbitrary { offsets: unsized_offsets, .. } = &unsized_layout.fields else { + bug!("unexpected FieldsShape for unsized layout of {ty:?}: {:?}", unsized_layout.fields); + }; + + let (sized_tail, sized_fields) = sized_offsets.raw.split_last().unwrap(); + let (unsized_tail, unsized_fields) = unsized_offsets.raw.split_last().unwrap(); + + if sized_fields != unsized_fields { + bug!("unsizing {ty:?} changed field order!\n{layout:?}\n{unsized_layout:?}"); + } + + if sized_tail < unsized_tail { + bug!("unsizing {ty:?} moved tail backwards!\n{layout:?}\n{unsized_layout:?}"); + } + } + tcx.mk_layout(layout) } From 7aa5f39d3b55eb746d9d64244ed5343874807ac4 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Wed, 5 Jul 2023 20:44:24 +0200 Subject: [PATCH 3/3] add helper methods for accessing struct tail --- compiler/rustc_hir_analysis/src/check/wfcheck.rs | 2 +- compiler/rustc_hir_typeck/src/cast.rs | 14 ++++++-------- compiler/rustc_middle/src/ty/mod.rs | 16 ++++++++++++++++ compiler/rustc_middle/src/ty/util.rs | 4 ++-- .../src/solve/project_goals.rs | 2 +- .../src/solve/trait_goals.rs | 7 +------ .../src/traits/select/confirmation.rs | 7 +------ compiler/rustc_ty_utils/src/layout.rs | 11 ++--------- compiler/rustc_ty_utils/src/ty.rs | 2 +- 9 files changed, 31 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 7a625dd5932..d4748b7ef0b 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -981,7 +981,7 @@ fn check_type_defn<'tcx>(tcx: TyCtxt<'tcx>, item: &hir::Item<'tcx>, all_sized: b // intermediate types must be sized. let needs_drop_copy = || { packed && { - let ty = tcx.type_of(variant.fields.raw.last().unwrap().did).subst_identity(); + let ty = tcx.type_of(variant.tail().did).subst_identity(); let ty = tcx.erase_regions(ty); if ty.has_infer() { tcx.sess diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index 71799387458..633933317c0 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -103,15 +103,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Ok(match *t.kind() { ty::Slice(_) | ty::Str => Some(PointerKind::Length), ty::Dynamic(ref tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())), - ty::Adt(def, substs) if def.is_struct() => { - match def.non_enum_variant().fields.raw.last() { - None => Some(PointerKind::Thin), - Some(f) => { - let field_ty = self.field_ty(span, f, substs); - self.pointer_kind(field_ty, span)? - } + ty::Adt(def, substs) if def.is_struct() => match def.non_enum_variant().tail_opt() { + None => Some(PointerKind::Thin), + Some(f) => { + let field_ty = self.field_ty(span, f, substs); + self.pointer_kind(field_ty, span)? } - } + }, ty::Tuple(fields) => match fields.last() { None => Some(PointerKind::Thin), Some(&f) => self.pointer_kind(f, span)?, diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index c100c45b61a..9a28122535d 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2028,6 +2028,22 @@ impl VariantDef { &self.fields[FieldIdx::from_u32(0)] } + + /// Returns the last field in this variant, if present. + #[inline] + pub fn tail_opt(&self) -> Option<&FieldDef> { + self.fields.raw.last() + } + + /// Returns the last field in this variant. + /// + /// # Panics + /// + /// Panics, if the variant has no fields. + #[inline] + pub fn tail(&self) -> &FieldDef { + self.tail_opt().expect("expected unsized ADT to have a tail field") + } } impl PartialEq for VariantDef { diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 720d770eed4..e2e4a2dbdc8 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -230,7 +230,7 @@ impl<'tcx> TyCtxt<'tcx> { if !def.is_struct() { break; } - match def.non_enum_variant().fields.raw.last() { + match def.non_enum_variant().tail_opt() { Some(field) => { f(); ty = field.ty(self, substs); @@ -304,7 +304,7 @@ impl<'tcx> TyCtxt<'tcx> { (&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs)) if a_def == b_def && a_def.is_struct() => { - if let Some(f) = a_def.non_enum_variant().fields.raw.last() { + if let Some(f) = a_def.non_enum_variant().tail_opt() { a = f.ty(self, a_substs); b = f.ty(self, b_substs); } else { diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs index aa690719970..e53b784a756 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs @@ -366,7 +366,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { } ty::Adt(def, substs) if def.is_struct() => { - match def.non_enum_variant().fields.raw.last() { + match def.non_enum_variant().tail_opt() { None => tcx.types.unit, Some(field_def) => { let self_ty = field_def.ty(tcx, substs); diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 7ff47295e7c..bc6ae5471ad 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -398,12 +398,7 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { return Err(NoSolution); } - let tail_field = a_def - .non_enum_variant() - .fields - .raw - .last() - .expect("expected unsized ADT to have a tail field"); + let tail_field = a_def.non_enum_variant().tail(); let tail_field_ty = tcx.type_of(tail_field.did); let a_tail_ty = tail_field_ty.subst(tcx, a_substs); diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 21a223135ed..bc7dcf8e61d 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -1125,12 +1125,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Err(Unimplemented); } - let tail_field = def - .non_enum_variant() - .fields - .raw - .last() - .expect("expected unsized ADT to have a tail field"); + let tail_field = def.non_enum_variant().tail(); let tail_field_ty = tcx.type_of(tail_field.did); // Extract `TailField` and `TailField` from `Struct` and `Struct`, diff --git a/compiler/rustc_ty_utils/src/layout.rs b/compiler/rustc_ty_utils/src/layout.rs index 0dbe6265522..b67cd96a734 100644 --- a/compiler/rustc_ty_utils/src/layout.rs +++ b/compiler/rustc_ty_utils/src/layout.rs @@ -480,7 +480,7 @@ fn layout_of_uncached<'tcx>( .any(|(i, v)| v.discr != ty::VariantDiscr::Relative(i.as_u32())); let maybe_unsized = def.is_struct() - && def.non_enum_variant().fields.raw.last().is_some_and(|last_field| { + && def.non_enum_variant().tail_opt().is_some_and(|last_field| { let param_env = tcx.param_env(def.did()); !tcx.type_of(last_field.did).subst_identity().is_sized(tcx, param_env) }); @@ -502,14 +502,7 @@ fn layout_of_uncached<'tcx>( // If the struct tail is sized and can be unsized, check that unsizing doesn't move the fields around. if cfg!(debug_assertions) && maybe_unsized - && def - .non_enum_variant() - .fields - .raw - .last() - .unwrap() - .ty(tcx, substs) - .is_sized(tcx, cx.param_env) + && def.non_enum_variant().tail().ty(tcx, substs).is_sized(tcx, cx.param_env) { let mut variants = variants; let tail_replacement = cx.layout_of(Ty::new_slice(tcx, tcx.types.u8)).unwrap(); diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs index 466616cd22b..6e5c50492c6 100644 --- a/compiler/rustc_ty_utils/src/ty.rs +++ b/compiler/rustc_ty_utils/src/ty.rs @@ -103,7 +103,7 @@ fn adt_sized_constraint(tcx: TyCtxt<'_>, def_id: DefId) -> &[Ty<'_>] { let result = tcx.mk_type_list_from_iter( def.variants() .iter() - .filter_map(|v| v.fields.raw.last()) + .filter_map(|v| v.tail_opt()) .flat_map(|f| sized_constraint_for_ty(tcx, def, tcx.type_of(f.did).subst_identity())), );