Skip to content

Commit

Permalink
Auto merge of #50708 - pietroalbini:beta-backports, r=alexcrichton
Browse files Browse the repository at this point in the history
[beta] Process backports

* #50622: rustc: leave space for fields of uninhabited types to allow partial initialization.
* #50643: typeck: Fix ICE with struct update syntax

r? @alexcrichton
  • Loading branch information
bors committed May 13, 2018
2 parents ddf6aff + 7041b65 commit 5d261a0
Show file tree
Hide file tree
Showing 11 changed files with 188 additions and 64 deletions.
73 changes: 50 additions & 23 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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, .. } |
Expand Down
13 changes: 1 addition & 12 deletions src/librustc_target/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down
9 changes: 3 additions & 6 deletions src/librustc_trans/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down
42 changes: 24 additions & 18 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/test/run-pass/issue-46845.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ 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)
}

fn main() {
assert_eq!(mem::size_of::<Foo>(), 8);
assert_eq!(mem::size_of::<Bar>(), 0);
// See the note on `Bar`'s definition for why this isn't `0`.
assert_eq!(mem::size_of::<Bar>(), 8);

let f = [Foo { a: 42 }, Foo { a: 10 }];
println!("{}", unsafe { f[0].a });
Expand Down
34 changes: 34 additions & 0 deletions src/test/run-pass/issue-49298.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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::<usize>());
}
21 changes: 21 additions & 0 deletions src/test/run-pass/issue-50442.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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);
}
14 changes: 11 additions & 3 deletions src/test/run-pass/type-sizes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,15 @@ enum EnumSingle5 {
A = 42 as u8,
}

enum NicheFilledEnumWithInhabitedVariant {
enum EnumWithMaybeUninhabitedVariant<T> {
A(&'static ()),
B(&'static (), !),
B(&'static (), T),
C,
}

enum NicheFilledEnumWithAbsentVariant {
A(&'static ()),
B((), !),
C,
}

Expand Down Expand Up @@ -107,5 +113,7 @@ pub fn main() {
assert_eq!(size_of::<EnumSingle4>(), 1);
assert_eq!(size_of::<EnumSingle5>(), 1);

assert_eq!(size_of::<NicheFilledEnumWithInhabitedVariant>(), size_of::<&'static ()>());
assert_eq!(size_of::<EnumWithMaybeUninhabitedVariant<!>>(),
size_of::<EnumWithMaybeUninhabitedVariant<()>>());
assert_eq!(size_of::<NicheFilledEnumWithAbsentVariant>(), size_of::<&'static ()>());
}
Loading

0 comments on commit 5d261a0

Please sign in to comment.