Skip to content

Commit

Permalink
Rollup merge of rust-lang#69768 - oli-obk:union_field_ice, r=eddyb,Ra…
Browse files Browse the repository at this point in the history
…lfJung

Compute the correct layout for variants of uninhabited enums

r? @eddyb
cc @RalfJung

fixes rust-lang#69191
cc rust-lang#69763
  • Loading branch information
Dylan-DPC authored Mar 17, 2020
2 parents e24252a + 74608c7 commit dd0d904
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 13 deletions.
14 changes: 11 additions & 3 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
present_first @ Some(_) => present_first,
// Uninhabited because it has no variants, or only absent ones.
None if def.is_enum() => return tcx.layout_raw(param_env.and(tcx.types.never)),
// if it's a struct, still compute a layout so that we can still compute the
// field offsets
// If it's a struct, still compute a layout so that we can still compute the
// field offsets.
None => Some(VariantIdx::new(0)),
};

Expand Down Expand Up @@ -1990,7 +1990,15 @@ where
{
fn for_variant(this: TyLayout<'tcx>, cx: &C, variant_index: VariantIdx) -> TyLayout<'tcx> {
let details = match this.variants {
Variants::Single { index } if index == variant_index => this.details,
Variants::Single { index }
// If all variants but one are uninhabited, the variant layout is the enum layout.
if index == variant_index &&
// Don't confuse variants of uninhabited enums with the enum itself.
// For more details see https://github.com/rust-lang/rust/issues/69763.
this.fields != FieldPlacement::Union(0) =>
{
this.details
}

Variants::Single { index } => {
// Deny calling for_variant more than once for non-Single enums.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
let base = match op.try_as_mplace(self) {
Ok(mplace) => {
// The easy case
// We can reuse the mplace field computation logic for indirect operands.
let field = self.mplace_field(mplace, field)?;
return Ok(field.into());
}
Expand Down
8 changes: 0 additions & 8 deletions src/librustc_mir/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,14 +410,6 @@ where
stride * field
}
layout::FieldPlacement::Union(count) => {
// This is a narrow bug-fix for rust-lang/rust#69191: if we are
// trying to access absent field of uninhabited variant, then
// signal UB (but don't ICE the compiler).
// FIXME temporary hack to work around incoherence between
// layout computation and MIR building
if field >= count as u64 && base.layout.abi == layout::Abi::Uninhabited {
throw_ub!(Unreachable);
}
assert!(
field < count as u64,
"Tried to access field {} of union {:#?} with {} fields",
Expand Down
5 changes: 4 additions & 1 deletion src/librustc_target/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,10 @@ impl FieldPlacement {

pub fn offset(&self, i: usize) -> Size {
match *self {
FieldPlacement::Union(_) => Size::ZERO,
FieldPlacement::Union(count) => {
assert!(i < count, "tried to access field {} of union with {} fields", i, count);
Size::ZERO
}
FieldPlacement::Array { stride, count } => {
let i = i as u64;
assert!(i < count);
Expand Down

0 comments on commit dd0d904

Please sign in to comment.