Fix bugs to optimizing enums:

- The discriminant must be first in all variants.
- The loop responsible for patching enum variants when the discriminant is enlarged was nonfunctional.
This commit is contained in:
Austin Hicks 2016-11-16 17:00:35 -05:00
parent cae94e8ec0
commit 8cfbffea3b
4 changed files with 15 additions and 6 deletions

View File

@ -550,6 +550,7 @@ impl<'a, 'gcx, 'tcx> Struct {
-> Result<(), LayoutError<'gcx>>
where I: Iterator<Item=Result<&'a Layout, LayoutError<'gcx>>> {
let fields = fields.collect::<Result<Vec<_>, LayoutError<'gcx>>>()?;
if is_enum_variant { assert!(fields.len() >= 1, "Enum variants must have at least a discriminant field.") }
if fields.len() == 0 {return Ok(())};
self.offsets = vec![Size::from_bytes(0); fields.len()];
@ -566,6 +567,7 @@ impl<'a, 'gcx, 'tcx> Struct {
let optimizing = &mut inverse_gep_index[start..end];
optimizing.sort_by_key(|&x| fields[x as usize].align(dl).abi());
}
if is_enum_variant { assert_eq!(inverse_gep_index[0], 0, "Enums must have field 0 as the field with lowest offset.") }
}
// At this point, inverse_gep_index holds field indices by increasing offset.
@ -1053,7 +1055,7 @@ impl<'a, 'gcx, 'tcx> Layout {
// Tuples and closures.
ty::TyClosure(def_id, ref substs) => {
let tys = substs.upvar_tys(def_id, tcx);
let mut st = Struct::new(dl,
let st = Struct::new(dl,
tys.map(|ty| ty.layout(infcx)),
attr::ReprAny,
false, ty)?;
@ -1228,7 +1230,8 @@ impl<'a, 'gcx, 'tcx> Layout {
hint, false, ty)?;
// Find the first field we can't move later
// to make room for a larger discriminant.
for i in st.field_index_by_increasing_offset() {
// It is important to skip the first field.
for i in st.field_index_by_increasing_offset().skip(1) {
let field = fields[i].unwrap();
let field_align = field.align(dl);
if field.size(dl).bytes() != 0 || field_align.abi() != 1 {
@ -1270,7 +1273,9 @@ impl<'a, 'gcx, 'tcx> Layout {
let new_ity_size = Int(ity).size(dl);
for variant in &mut variants {
for i in variant.offsets.iter_mut() {
if *i <= old_ity_size {
// The first field is the discrimminant, at offset 0.
// These aren't in order, and we need to skip it.
if *i <= old_ity_size && *i > Size::from_bytes(0){
*i = new_ity_size;
}
}

View File

@ -750,7 +750,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences {
if let Layout::General { ref variants, ref size, discr, .. } = *layout {
let discr_size = Primitive::Int(discr).size(&cx.tcx.data_layout).bytes();
debug!("enum `{}` is {} bytes large", t, size.bytes());
debug!("enum `{}` is {} bytes large with layout:\n{:#?}", t, size.bytes(), layout);
let (largest, slargest, largest_index) = enum_definition.variants
.iter()

View File

@ -11,6 +11,9 @@
#![warn(variant_size_differences)]
#![allow(dead_code)]
// Note that the following test only works because all fields of the enum variants are of the same size.
// If this test is modified so that the reordering logic in librustc/ty/layout.rs kicks in, it will fail.
enum Enum1 { }
enum Enum2 { A, B, C }

View File

@ -26,8 +26,9 @@ fn main() {
assert_eq!(size_of::<E>(), 1);
assert_eq!(size_of::<Option<E>>(), 1);
assert_eq!(size_of::<Result<E, ()>>(), 1);
assert_eq!(size_of::<S>(), 4);
assert_eq!(size_of::<Option<S>>(), 4);
// The next asserts are correct given the currently dumb field reordering algorithm, which actually makes this struct larger.
assert_eq!(size_of::<S>(), 6);
assert_eq!(size_of::<Option<S>>(), 6);
let enone = None::<E>;
let esome = Some(E::A);
if let Some(..) = enone {