Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compute the correct layout for variants of uninhabited enums #69768

Merged
merged 3 commits into from
Mar 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
// 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 @@ -356,7 +356,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