diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 02c4b73efa146..2171b9a6c52b7 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -325,10 +325,6 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { offsets.len(), ty); } - if field.abi == Abi::Uninhabited { - return Ok(LayoutDetails::uninhabited(fields.len())); - } - if field.is_unsized() { sized = false; } @@ -451,6 +447,10 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { } } + if sized && fields.iter().any(|f| f.abi == Abi::Uninhabited) { + abi = Abi::Uninhabited; + } + Ok(LayoutDetails { variants: Variants::Single { index: 0 }, fields: FieldPlacement::Arbitrary { @@ -497,7 +497,13 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { // The never type. ty::TyNever => { - tcx.intern_layout(LayoutDetails::uninhabited(0)) + tcx.intern_layout(LayoutDetails { + variants: Variants::Single { index: 0 }, + fields: FieldPlacement::Union(0), + abi: Abi::Uninhabited, + align: dl.i8_align, + size: Size::from_bytes(0) + }) } // Potentially-fat pointers. @@ -711,27 +717,37 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { })); } - let (inh_first, inh_second) = { - let mut inh_variants = (0..variants.len()).filter(|&v| { - variants[v].iter().all(|f| f.abi != Abi::Uninhabited) + // A variant is absent if it's uninhabited and only has ZST fields. + // Present uninhabited variants only require space for their fields, + // but *not* an encoding of the discriminant (e.g. a tag value). + // See issue #49298 for more details on the need to leave space + // for non-ZST uninhabited data (mostly partial initialization). + let absent = |fields: &[TyLayout]| { + let uninhabited = fields.iter().any(|f| f.abi == Abi::Uninhabited); + let is_zst = fields.iter().all(|f| f.is_zst()); + uninhabited && is_zst + }; + let (present_first, present_second) = { + let mut present_variants = (0..variants.len()).filter(|&v| { + !absent(&variants[v]) }); - (inh_variants.next(), inh_variants.next()) + (present_variants.next(), present_variants.next()) }; - if inh_first.is_none() { - // Uninhabited because it has no variants, or only uninhabited ones. - return Ok(tcx.intern_layout(LayoutDetails::uninhabited(0))); + if present_first.is_none() { + // Uninhabited because it has no variants, or only absent ones. + return tcx.layout_raw(param_env.and(tcx.types.never)); } let is_struct = !def.is_enum() || - // Only one variant is inhabited. - (inh_second.is_none() && + // Only one variant is present. + (present_second.is_none() && // Representation optimizations are allowed. !def.repr.inhibit_enum_layout_opt()); if is_struct { // Struct, or univariant enum equivalent to a struct. // (Typechecking will reject discriminant-sizing attrs.) - let v = inh_first.unwrap(); + let v = present_first.unwrap(); let kind = if def.is_enum() || variants[v].len() == 0 { StructKind::AlwaysSized } else { @@ -773,7 +789,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { // Find one non-ZST variant. 'variants: for (v, fields) in variants.iter().enumerate() { - if fields.iter().any(|f| f.abi == Abi::Uninhabited) { + if absent(fields) { continue 'variants; } for f in fields { @@ -816,7 +832,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { let offset = st[i].fields.offset(field_index) + offset; let size = st[i].size; - let abi = match st[i].abi { + let mut abi = match st[i].abi { Abi::Scalar(_) => Abi::Scalar(niche.clone()), Abi::ScalarPair(ref first, ref second) => { // We need to use scalar_unit to reset the @@ -833,6 +849,10 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { _ => Abi::Aggregate { sized: true }, }; + if st.iter().all(|v| v.abi == Abi::Uninhabited) { + abi = Abi::Uninhabited; + } + return Ok(tcx.intern_layout(LayoutDetails { variants: Variants::NicheFilling { dataful_variant: i, @@ -959,9 +979,6 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { let old_ity_size = min_ity.size(); let new_ity_size = ity.size(); for variant in &mut layout_variants { - if variant.abi == Abi::Uninhabited { - continue; - } match variant.fields { FieldPlacement::Arbitrary { ref mut offsets, .. } => { for i in offsets { @@ -1055,6 +1072,11 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { } } } + + if layout_variants.iter().all(|v| v.abi == Abi::Uninhabited) { + abi = Abi::Uninhabited; + } + tcx.intern_layout(LayoutDetails { variants: Variants::Tagged { tag, @@ -1523,9 +1545,14 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx> ty::TyAdt(def, _) => def.variants[variant_index].fields.len(), _ => bug!() }; - let mut details = LayoutDetails::uninhabited(fields); - details.variants = Variants::Single { index: variant_index }; - cx.tcx().intern_layout(details) + let tcx = cx.tcx(); + tcx.intern_layout(LayoutDetails { + variants: Variants::Single { index: variant_index }, + fields: FieldPlacement::Union(fields), + abi: Abi::Uninhabited, + align: tcx.data_layout.i8_align, + size: Size::from_bytes(0) + }) } Variants::NicheFilling { ref variants, .. } | diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 7ae4d990c8a4e..6cd8e267ec5c1 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -761,17 +761,6 @@ impl LayoutDetails { align, } } - - pub fn uninhabited(field_count: usize) -> Self { - let align = Align::from_bytes(1, 1).unwrap(); - LayoutDetails { - variants: Variants::Single { index: 0 }, - fields: FieldPlacement::Union(field_count), - abi: Abi::Uninhabited, - align, - size: Size::from_bytes(0) - } - } } /// The details of the layout of a type, alongside the type itself. @@ -826,10 +815,10 @@ impl<'a, Ty> TyLayout<'a, Ty> { /// Returns true if the type is a ZST and not unsized. pub fn is_zst(&self) -> bool { match self.abi { - Abi::Uninhabited => true, Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } => false, + Abi::Uninhabited => self.size.bytes() == 0, Abi::Aggregate { sized } => sized && self.size.bytes() == 0 } } diff --git a/src/librustc_trans/mir/operand.rs b/src/librustc_trans/mir/operand.rs index 656ab95a28cf3..9d34f73ed2298 100644 --- a/src/librustc_trans/mir/operand.rs +++ b/src/librustc_trans/mir/operand.rs @@ -223,12 +223,9 @@ impl<'a, 'tcx> OperandRef<'tcx> { let offset = self.layout.fields.offset(i); let mut val = match (self.val, &self.layout.abi) { - // If we're uninhabited, or the field is ZST, it has no data. - _ if self.layout.abi == layout::Abi::Uninhabited || field.is_zst() => { - return OperandRef { - val: OperandValue::Immediate(C_undef(field.immediate_llvm_type(bx.cx))), - layout: field - }; + // If the field is ZST, it has no data. + _ if field.is_zst() => { + return OperandRef::new_zst(bx.cx, field); } // Newtype of a scalar, scalar pair or vector. diff --git a/src/librustc_trans/type_of.rs b/src/librustc_trans/type_of.rs index 5e6b276495764..a24e9a44efc21 100644 --- a/src/librustc_trans/type_of.rs +++ b/src/librustc_trans/type_of.rs @@ -213,10 +213,10 @@ pub trait LayoutLlvmExt<'tcx> { impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { fn is_llvm_immediate(&self) -> bool { match self.abi { - layout::Abi::Uninhabited | layout::Abi::Scalar(_) | layout::Abi::Vector { .. } => true, layout::Abi::ScalarPair(..) => false, + layout::Abi::Uninhabited | layout::Abi::Aggregate { .. } => self.is_zst() } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index db859e42057e9..073b0047f4ecf 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3275,7 +3275,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { span: Span, variant: &'tcx ty::VariantDef, ast_fields: &'gcx [hir::Field], - check_completeness: bool) { + check_completeness: bool) -> bool { let tcx = self.tcx; let adt_ty_hint = @@ -3377,6 +3377,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { truncated_fields_error)) .emit(); } + error_happened } fn check_struct_fields_on_error(&self, @@ -3475,24 +3476,29 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - self.check_expr_struct_fields(struct_ty, expected, expr.id, path_span, variant, fields, - base_expr.is_none()); + let error_happened = self.check_expr_struct_fields(struct_ty, expected, expr.id, path_span, + variant, fields, base_expr.is_none()); if let &Some(ref base_expr) = base_expr { - self.check_expr_has_type_or_error(base_expr, struct_ty); - match struct_ty.sty { - ty::TyAdt(adt, substs) if adt.is_struct() => { - let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| { - self.normalize_associated_types_in(expr.span, &f.ty(self.tcx, substs)) - }).collect(); - - self.tables - .borrow_mut() - .fru_field_types_mut() - .insert(expr.hir_id, fru_field_types); - } - _ => { - span_err!(self.tcx.sess, base_expr.span, E0436, - "functional record update syntax requires a struct"); + // If check_expr_struct_fields hit an error, do not attempt to populate + // the fields with the base_expr. This could cause us to hit errors later + // when certain fields are assumed to exist that in fact do not. + if !error_happened { + self.check_expr_has_type_or_error(base_expr, struct_ty); + match struct_ty.sty { + ty::TyAdt(adt, substs) if adt.is_struct() => { + let fru_field_types = adt.non_enum_variant().fields.iter().map(|f| { + self.normalize_associated_types_in(expr.span, &f.ty(self.tcx, substs)) + }).collect(); + + self.tables + .borrow_mut() + .fru_field_types_mut() + .insert(expr.hir_id, fru_field_types); + } + _ => { + span_err!(self.tcx.sess, base_expr.span, E0436, + "functional record update syntax requires a struct"); + } } } } diff --git a/src/test/run-pass/issue-46845.rs b/src/test/run-pass/issue-46845.rs index 235d3982a9c0c..92f68dcfc349d 100644 --- a/src/test/run-pass/issue-46845.rs +++ b/src/test/run-pass/issue-46845.rs @@ -24,6 +24,7 @@ union Foo { } // If all the variants are uninhabited, however, the union should be uninhabited. +// NOTE(#49298) the union being uninhabited shouldn't change its size. union Bar { _a: (Never, u64), _b: (u64, Never) @@ -31,7 +32,8 @@ union Bar { fn main() { assert_eq!(mem::size_of::(), 8); - assert_eq!(mem::size_of::(), 0); + // See the note on `Bar`'s definition for why this isn't `0`. + assert_eq!(mem::size_of::(), 8); let f = [Foo { a: 42 }, Foo { a: 10 }]; println!("{}", unsafe { f[0].a }); diff --git a/src/test/run-pass/issue-49298.rs b/src/test/run-pass/issue-49298.rs new file mode 100644 index 0000000000000..0b2169c9476cd --- /dev/null +++ b/src/test/run-pass/issue-49298.rs @@ -0,0 +1,34 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(test)] + +extern crate test; + +enum Void {} + +fn main() { + let mut x: (Void, usize); + let mut y = 42; + x.1 = 13; + + // Make sure `y` stays on the stack. + test::black_box(&mut y); + + // Check that the write to `x.1` did not overwrite `y`. + // Note that this doesn't fail with optimizations enabled, + // because we can't keep `x.1` on the stack, like we can `y`, + // as we can't borrow partially initialized variables. + assert_eq!(y.to_string(), "42"); + + // Check that `(Void, usize)` has space for the `usize` field. + assert_eq!(std::mem::size_of::<(Void, usize)>(), + std::mem::size_of::()); +} diff --git a/src/test/run-pass/issue-50442.rs b/src/test/run-pass/issue-50442.rs new file mode 100644 index 0000000000000..1e43bebf5c32c --- /dev/null +++ b/src/test/run-pass/issue-50442.rs @@ -0,0 +1,21 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +enum Void {} + +enum Foo { + A(i32), + B(Void), + C(i32) +} + +fn main() { + let _foo = Foo::A(0); +} diff --git a/src/test/run-pass/type-sizes.rs b/src/test/run-pass/type-sizes.rs index a47f082b9c3ee..8f4613d6c373a 100644 --- a/src/test/run-pass/type-sizes.rs +++ b/src/test/run-pass/type-sizes.rs @@ -68,9 +68,15 @@ enum EnumSingle5 { A = 42 as u8, } -enum NicheFilledEnumWithInhabitedVariant { +enum EnumWithMaybeUninhabitedVariant { A(&'static ()), - B(&'static (), !), + B(&'static (), T), + C, +} + +enum NicheFilledEnumWithAbsentVariant { + A(&'static ()), + B((), !), C, } @@ -107,5 +113,7 @@ pub fn main() { assert_eq!(size_of::(), 1); assert_eq!(size_of::(), 1); - assert_eq!(size_of::(), size_of::<&'static ()>()); + assert_eq!(size_of::>(), + size_of::>()); + assert_eq!(size_of::(), size_of::<&'static ()>()); } diff --git a/src/test/ui/issue-50618.rs b/src/test/ui/issue-50618.rs new file mode 100644 index 0000000000000..ed427c293df47 --- /dev/null +++ b/src/test/ui/issue-50618.rs @@ -0,0 +1,29 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct Point { + pub x: u64, + pub y: u64, +} + +const TEMPLATE: Point = Point { + x: 0, + y: 0 +}; + +fn main() { + let _ = || { + Point { + nonexistent: 0, + //~^ ERROR struct `Point` has no field named `nonexistent` + ..TEMPLATE + } + }; +} diff --git a/src/test/ui/issue-50618.stderr b/src/test/ui/issue-50618.stderr new file mode 100644 index 0000000000000..07cc5a1318aa4 --- /dev/null +++ b/src/test/ui/issue-50618.stderr @@ -0,0 +1,11 @@ +error[E0560]: struct `Point` has no field named `nonexistent` + --> $DIR/issue-50618.rs:24:13 + | +LL | nonexistent: 0, + | ^^^^^^^^^^^ `Point` does not have this field + | + = note: available fields are: `x`, `y` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0560`.