Skip to content

Commit 49f851c

Browse files
committed
Fix type_of for enums to not lose data in some cases when immediate.
The previous implementation, when combined with small discriminants and immediate types, caused problems for types like `Either<u8, i16>` which are now small enough to be immediate and can have fields intersecting the highest-alignment variant's alignment padding (which LLVM doesn't preserve). So let's not do that.
1 parent ac4644d commit 49f851c

File tree

1 file changed

+32
-24
lines changed
  • src/librustc/middle/trans

1 file changed

+32
-24
lines changed

src/librustc/middle/trans/adt.rs

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -382,30 +382,38 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] {
382382
CEnum(ity, _, _) => ~[ll_inttype(cx, ity)],
383383
Univariant(ref st, _dtor) => struct_llfields(cx, st, sizing),
384384
NullablePointer{ nonnull: ref st, _ } => struct_llfields(cx, st, sizing),
385-
General(_ity, ref sts) => {
386-
// To get "the" type of a general enum, we pick the case
387-
// with the largest alignment (so it will always align
388-
// correctly in containing structures) and pad it out.
389-
assert!(sts.len() >= 1);
390-
let mut most_aligned = None;
391-
let mut largest_align = 0;
392-
let mut largest_size = 0;
393-
for st in sts.iter() {
394-
if largest_size < st.size {
395-
largest_size = st.size;
396-
}
397-
if largest_align < st.align {
398-
// Clang breaks ties by size; it is unclear if
399-
// that accomplishes anything important.
400-
largest_align = st.align;
401-
most_aligned = Some(st);
402-
}
403-
}
404-
let most_aligned = most_aligned.unwrap();
405-
let padding = largest_size - most_aligned.size;
406-
407-
struct_llfields(cx, most_aligned, sizing)
408-
+ &[Type::array(&Type::i8(), padding)]
385+
General(ity, ref sts) => {
386+
// We need a representation that has:
387+
// * The alignment of the most-aligned field
388+
// * The size of the largest variant (rounded up to that alignment)
389+
// * No alignment padding anywhere any variant has actual data
390+
// (currently matters only for enums small enough to be immediate)
391+
// * The discriminant in an obvious place.
392+
//
393+
// So we start with the discriminant, pad it up to the alignment with
394+
// more of its own type, then use alignment-sized ints to get the rest
395+
// of the size.
396+
//
397+
// Note: if/when we start exposing SIMD vector types (or f80, on some
398+
// platforms that have it), this will need some adjustment.
399+
let size = sts.iter().map(|st| st.size).max().unwrap();
400+
let most_aligned = sts.iter().max_by(|st| st.align).unwrap();
401+
let align = most_aligned.align;
402+
let discr_ty = ll_inttype(cx, ity);
403+
let discr_size = machine::llsize_of_alloc(cx, discr_ty) as u64;
404+
let pad_ty = match align {
405+
1 => Type::i8(),
406+
2 => Type::i16(),
407+
4 => Type::i32(),
408+
8 if machine::llalign_of_min(cx, Type::i64()) == 8 => Type::i64(),
409+
_ => fail!("Unsupported enum alignment: {:?}", align)
410+
};
411+
assert_eq!(machine::llalign_of_min(cx, pad_ty) as u64, align);
412+
let align_units = (size + align - 1) / align;
413+
assert_eq!(align % discr_size, 0);
414+
~[discr_ty,
415+
Type::array(&discr_ty, align / discr_size - 1),
416+
Type::array(&pad_ty, align_units - 1)]
409417
}
410418
}
411419
}

0 commit comments

Comments
 (0)