From b7c8cc4b778b2fcf4193a3f7fd9cf8c323ebcf5c Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 11 Apr 2018 17:25:18 +0200 Subject: [PATCH 1/6] Properly look for uninhabitedness when handling discriminants --- src/librustc_mir/interpret/eval_context.rs | 3 +++ src/librustc_trans/mir/place.rs | 5 ++++- src/librustc_trans/mir/rvalue.rs | 10 ++++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index b98ab218de5cb..478f45e841d1c 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -850,6 +850,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M ) -> EvalResult<'tcx, u128> { let layout = self.layout_of(ty)?; trace!("read_discriminant_value {:#?}", layout); + if layout.abi == layout::Abi::Uninhabited { + return Ok(0); + } match layout.variants { layout::Variants::Single { index } => { diff --git a/src/librustc_trans/mir/place.rs b/src/librustc_trans/mir/place.rs index b340d91b02708..3cadaef47bb66 100644 --- a/src/librustc_trans/mir/place.rs +++ b/src/librustc_trans/mir/place.rs @@ -16,7 +16,7 @@ use rustc::mir::tcx::PlaceTy; use rustc_data_structures::indexed_vec::Idx; use base; use builder::Builder; -use common::{CodegenCx, C_usize, C_u8, C_u32, C_uint, C_int, C_null, C_uint_big}; +use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_int, C_null, C_uint_big}; use consts; use type_of::LayoutLlvmExt; use type_::Type; @@ -264,6 +264,9 @@ impl<'a, 'tcx> PlaceRef<'tcx> { /// Obtain the actual discriminant of a value. pub fn trans_get_discr(self, bx: &Builder<'a, 'tcx>, cast_to: Ty<'tcx>) -> ValueRef { let cast_to = bx.cx.layout_of(cast_to).immediate_llvm_type(bx.cx); + if self.layout.abi == layout::Abi::Uninhabited { + return C_undef(cast_to); + } match self.layout.variants { layout::Variants::Single { index } => { return C_uint(cast_to, index as u64); diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 93702bfbbf3b1..245f3ec11c96d 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -22,7 +22,7 @@ use base; use builder::Builder; use callee; use common::{self, val_ty}; -use common::{C_bool, C_u8, C_i32, C_u32, C_u64, C_null, C_usize, C_uint, C_uint_big}; +use common::{C_bool, C_u8, C_i32, C_u32, C_u64, C_undef, C_null, C_usize, C_uint, C_uint_big}; use consts; use monomorphize; use type_::Type; @@ -267,11 +267,17 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> { } mir::CastKind::Misc => { assert!(cast.is_llvm_immediate()); + let ll_t_out = cast.immediate_llvm_type(bx.cx); + if operand.layout.abi == layout::Abi::Uninhabited { + return (bx, OperandRef { + val: OperandValue::Immediate(C_undef(ll_t_out)), + layout: cast, + }); + } let r_t_in = CastTy::from_ty(operand.layout.ty) .expect("bad input type for cast"); let r_t_out = CastTy::from_ty(cast.ty).expect("bad output type for cast"); let ll_t_in = operand.layout.immediate_llvm_type(bx.cx); - let ll_t_out = cast.immediate_llvm_type(bx.cx); let llval = operand.immediate(); let mut signed = false; From 1839ec5ef885c04c585be4b3af7a3a9c269efddb Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 11 Apr 2018 17:30:49 +0200 Subject: [PATCH 2/6] Consistently use C_uint_big for discriminants --- src/librustc_trans/mir/place.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_trans/mir/place.rs b/src/librustc_trans/mir/place.rs index 3cadaef47bb66..c39612e3ab093 100644 --- a/src/librustc_trans/mir/place.rs +++ b/src/librustc_trans/mir/place.rs @@ -16,7 +16,7 @@ use rustc::mir::tcx::PlaceTy; use rustc_data_structures::indexed_vec::Idx; use base; use builder::Builder; -use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_int, C_null, C_uint_big}; +use common::{CodegenCx, C_undef, C_usize, C_u8, C_u32, C_uint, C_null, C_uint_big}; use consts; use type_of::LayoutLlvmExt; use type_::Type; @@ -331,9 +331,11 @@ impl<'a, 'tcx> PlaceRef<'tcx> { let ptr = self.project_field(bx, 0); let to = self.layout.ty.ty_adt_def().unwrap() .discriminant_for_variant(bx.tcx(), variant_index) - .val as u64; - bx.store(C_int(ptr.layout.llvm_type(bx.cx), to as i64), - ptr.llval, ptr.align); + .val; + bx.store( + C_uint_big(ptr.layout.llvm_type(bx.cx), to), + ptr.llval, + ptr.align); } layout::Variants::NicheFilling { dataful_variant, From 2807f4f773ac3e56849474085a8203261b49c556 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Sat, 31 Mar 2018 23:06:26 +0200 Subject: [PATCH 3/6] Properly evaluate zst enum --- src/librustc_mir/interpret/eval_context.rs | 9 +++++++++ src/librustc_mir/interpret/place.rs | 1 + 2 files changed, 10 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 478f45e841d1c..a7cd044cec03a 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -1319,6 +1319,15 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M pub fn try_read_value(&self, ptr: Pointer, ptr_align: Align, ty: Ty<'tcx>) -> EvalResult<'tcx, Option> { use syntax::ast::FloatTy; + let layout = self.layout_of(ty)?; + // do the strongest layout check of the two + let align = layout.align.max(ptr_align); + self.memory.check_align(ptr, align)?; + + if layout.size.bytes() == 0 { + return Ok(Some(Value::ByVal(PrimVal::Undef))); + } + let ptr = ptr.to_ptr()?; let val = match ty.sty { ty::TyBool => { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 456f5fd75db09..42cb149d68221 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -136,6 +136,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { let val = [a, b][field_index]; Ok(Some((Value::ByVal(val), field.ty))) }, + // FIXME(oli-obk): figure out whether we should be calling `try_read_value` here _ => Ok(None), } } From 7bfe3ae00a36e8162e2f8f5b47eb7127562e1d17 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 12 Apr 2018 15:11:14 +0200 Subject: [PATCH 4/6] Add a test for casts of univariant C-like enums --- src/test/run-pass/issue-23304-2.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/test/run-pass/issue-23304-2.rs b/src/test/run-pass/issue-23304-2.rs index 79712f7c25e12..5989b7e9c6ad8 100644 --- a/src/test/run-pass/issue-23304-2.rs +++ b/src/test/run-pass/issue-23304-2.rs @@ -10,8 +10,13 @@ #![allow(dead_code)] -enum X { A = 0 as isize } +enum X { A = 42 as isize } enum Y { A = X::A as isize } -fn main() { } +fn main() { + let x = X::A; + let x = x as isize; + assert_eq!(x, 42); + assert_eq!(Y::A as isize, 42); +} From 8f36804c00ca11ca9b4ed111dc4b4066b940508e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Fri, 30 Mar 2018 15:49:56 +0200 Subject: [PATCH 5/6] Treat repr(Rust) univariant fieldless enums as a ZST (fixes #15747) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes all those enums be represented the same way: ```rust enum A1 { B1 } enum A2 { B2 = 0 } enum A3 { B3, C3(!) } ``` --- src/librustc/ty/layout.rs | 9 +++--- src/librustc_mir/interpret/eval_context.rs | 22 ++++++++++++++- src/librustc_trans/mir/place.rs | 5 +++- src/librustc_trans/mir/rvalue.rs | 16 +++++++++++ src/test/run-pass/type-sizes.rs | 33 ++++++++++++++++++++++ 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 77e2e9447f1ed..7dec180c5a732 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -1451,11 +1451,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { // Only one variant is inhabited. (inh_second.is_none() && // Representation optimizations are allowed. - !def.repr.inhibit_enum_layout_opt() && - // Inhabited variant either has data ... - (!variants[inh_first.unwrap()].is_empty() || - // ... or there other, uninhabited, variants. - variants.len() > 1)); + !def.repr.inhibit_enum_layout_opt()); if is_struct { // Struct, or univariant enum equivalent to a struct. // (Typechecking will reject discriminant-sizing attrs.) @@ -1489,6 +1485,9 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { return Ok(tcx.intern_layout(st)); } + // The current code for niche-filling relies on variant indices + // instead of actual discriminants, so dataful enums with + // explicit discriminants (RFC #2363) would misbehave. let no_explicit_discriminants = def.variants.iter().enumerate() .all(|(i, v)| v.discr == ty::VariantDiscr::Relative(i)); diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index a7cd044cec03a..00f90573682c6 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -669,6 +669,23 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M (Value::ByVal(_), _) => bug!("expected fat ptr"), } } else { + let src_layout = self.layout_of(src.ty)?; + match src_layout.variants { + layout::Variants::Single { index } => { + if let Some(def) = src.ty.ty_adt_def() { + let discr_val = def + .discriminant_for_variant(*self.tcx, index) + .val; + return self.write_primval( + dest, + PrimVal::Bytes(discr_val), + dest_ty); + } + } + layout::Variants::Tagged { .. } | + layout::Variants::NicheFilling { .. } => {}, + } + let src_val = self.value_to_primval(src)?; let dest_val = self.cast_primval(src_val, src.ty, dest_ty)?; let valty = ValTy { @@ -856,7 +873,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M match layout.variants { layout::Variants::Single { index } => { - return Ok(index as u128); + let discr_val = ty.ty_adt_def().map_or( + index as u128, + |def| def.discriminant_for_variant(*self.tcx, index).val); + return Ok(discr_val); } layout::Variants::Tagged { .. } | layout::Variants::NicheFilling { .. } => {}, diff --git a/src/librustc_trans/mir/place.rs b/src/librustc_trans/mir/place.rs index c39612e3ab093..b8b0c019ca606 100644 --- a/src/librustc_trans/mir/place.rs +++ b/src/librustc_trans/mir/place.rs @@ -269,7 +269,10 @@ impl<'a, 'tcx> PlaceRef<'tcx> { } match self.layout.variants { layout::Variants::Single { index } => { - return C_uint(cast_to, index as u64); + let discr_val = self.layout.ty.ty_adt_def().map_or( + index as u128, + |def| def.discriminant_for_variant(bx.cx.tcx, index).val); + return C_uint_big(cast_to, discr_val); } layout::Variants::Tagged { .. } | layout::Variants::NicheFilling { .. } => {}, diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 245f3ec11c96d..c932777402eb8 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -278,6 +278,22 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> { .expect("bad input type for cast"); let r_t_out = CastTy::from_ty(cast.ty).expect("bad output type for cast"); let ll_t_in = operand.layout.immediate_llvm_type(bx.cx); + match operand.layout.variants { + layout::Variants::Single { index } => { + if let Some(def) = operand.layout.ty.ty_adt_def() { + let discr_val = def + .discriminant_for_variant(bx.cx.tcx, index) + .val; + let discr = C_uint_big(ll_t_out, discr_val); + return (bx, OperandRef { + val: OperandValue::Immediate(discr), + layout: cast, + }); + } + } + layout::Variants::Tagged { .. } | + layout::Variants::NicheFilling { .. } => {}, + } let llval = operand.immediate(); let mut signed = false; diff --git a/src/test/run-pass/type-sizes.rs b/src/test/run-pass/type-sizes.rs index 7bd9a1703ee94..a47f082b9c3ee 100644 --- a/src/test/run-pass/type-sizes.rs +++ b/src/test/run-pass/type-sizes.rs @@ -43,6 +43,31 @@ enum ReorderedEnum { B(u8, u16, u8), } +enum EnumEmpty {} + +enum EnumSingle1 { + A, +} + +enum EnumSingle2 { + A = 42 as isize, +} + +enum EnumSingle3 { + A, + B(!), +} + +#[repr(u8)] +enum EnumSingle4 { + A, +} + +#[repr(u8)] +enum EnumSingle5 { + A = 42 as u8, +} + enum NicheFilledEnumWithInhabitedVariant { A(&'static ()), B(&'static (), !), @@ -74,5 +99,13 @@ pub fn main() { assert_eq!(size_of::(), 4 as usize); assert_eq!(size_of::(), 4); assert_eq!(size_of::(), 6); + + assert_eq!(size_of::(), 0); + assert_eq!(size_of::(), 0); + assert_eq!(size_of::(), 0); + assert_eq!(size_of::(), 0); + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), size_of::<&'static ()>()); } From 1c09977c9a1aa344c52ddf44ebd42bacd876274b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 25 Apr 2018 10:33:02 +0200 Subject: [PATCH 6/6] Mark SingleVariant as repr(u8) in c-style-enum I should rather properly fix debuginfo but I have no clue how to do that. --- src/test/debuginfo/c-style-enum.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/debuginfo/c-style-enum.rs b/src/test/debuginfo/c-style-enum.rs index 1f1f42e2dec2e..2dbac8e3d9eac 100644 --- a/src/test/debuginfo/c-style-enum.rs +++ b/src/test/debuginfo/c-style-enum.rs @@ -151,6 +151,7 @@ enum ManualDiscriminant { } #[derive(Copy, Clone)] +#[repr(u8)] enum SingleVariant { TheOnlyVariant }