From f1124a2f55406be8e758488352c59ae9deef17b3 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Fri, 24 May 2013 18:08:45 -0400 Subject: [PATCH 01/17] Add parser for `#[repr(...)]`; nothing uses it yet. Also export enum attrs into metadata, and add a convenient interface for obtaining the repr hint from either a local or remote definition. --- src/librustc/metadata/encoder.rs | 1 + src/librustc/middle/ty.rs | 48 ++++++++++++---- src/libsyntax/attr.rs | 96 +++++++++++++++++++++++++++++++- 3 files changed, 133 insertions(+), 12 deletions(-) diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index ade5bfe85e4..86c4ca0158f 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -989,6 +989,7 @@ fn encode_info_for_item(ecx: &EncodeContext, encode_family(ebml_w, 't'); encode_bounds_and_type(ebml_w, ecx, &lookup_item_type(tcx, def_id)); encode_name(ecx, ebml_w, item.ident); + encode_attributes(ebml_w, item.attrs); for v in (*enum_definition).variants.iter() { encode_variant_id(ebml_w, local_def(v.node.id)); } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 39aa6c5e396..3945fcd80ae 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -38,6 +38,7 @@ use syntax::ast::*; use syntax::ast_util::is_local; use syntax::ast_util; use syntax::attr; +use syntax::attr::AttrMetaMethods; use syntax::codemap::Span; use syntax::codemap; use syntax::parse::token; @@ -4101,27 +4102,42 @@ pub fn lookup_trait_def(cx: ctxt, did: ast::DefId) -> @ty::TraitDef { } } -/// Determine whether an item is annotated with an attribute -pub fn has_attr(tcx: ctxt, did: DefId, attr: &str) -> bool { +/// Iterate over meta_items of a definition. +// (This should really be an iterator, but that would require csearch and +// decoder to use iterators instead of higher-order functions.) +pub fn each_attr(tcx: ctxt, did: DefId, f: &fn(@MetaItem) -> bool) -> bool { if is_local(did) { match tcx.items.find(&did.node) { - Some( - &ast_map::node_item(@ast::item { - attrs: ref attrs, - _ - }, _)) => attr::contains_name(*attrs, attr), + Some(&ast_map::node_item(@ast::item {attrs: ref attrs, _}, _)) => + attrs.iter().advance(|attr| f(attr.node.value)), _ => tcx.sess.bug(format!("has_attr: {:?} is not an item", - did)) + did)) } } else { - let mut ret = false; + let mut cont = true; do csearch::get_item_attrs(tcx.cstore, did) |meta_items| { - ret = ret || attr::contains_name(meta_items, attr); + if cont { + cont = meta_items.iter().advance(|ptrptr| f(*ptrptr)); + } } - ret + return cont; } } +/// Determine whether an item is annotated with an attribute +pub fn has_attr(tcx: ctxt, did: DefId, attr: &str) -> bool { + let mut found = false; + each_attr(tcx, did, |item| { + if attr == item.name() { + found = true; + false + } else { + true + } + }); + return found; +} + /// Determine whether an item is annotated with `#[packed]` pub fn lookup_packed(tcx: ctxt, did: DefId) -> bool { has_attr(tcx, did, "packed") @@ -4132,6 +4148,16 @@ pub fn lookup_simd(tcx: ctxt, did: DefId) -> bool { has_attr(tcx, did, "simd") } +// Obtain the the representation annotation for a definition. +pub fn lookup_repr_hint(tcx: ctxt, did: DefId) -> attr::ReprAttr { + let mut acc = attr::ReprAny; + ty::each_attr(tcx, did, |meta| { + acc = attr::find_repr_attr(tcx.sess.diagnostic(), meta, acc); + true + }); + return acc; +} + // Look up a field ID, whether or not it's local // Takes a list of type substs in case the struct is generic pub fn lookup_field_type(tcx: ctxt, diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index 40b7ff29e24..e538e09c056 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -14,7 +14,7 @@ use extra; use ast; use ast::{Attribute, Attribute_, MetaItem, MetaWord, MetaNameValue, MetaList}; -use codemap::{Spanned, spanned, dummy_spanned}; +use codemap::{Span, Spanned, spanned, dummy_spanned}; use codemap::BytePos; use diagnostic::span_handler; use parse::comments::{doc_comment_style, strip_doc_comment_decoration}; @@ -363,3 +363,97 @@ pub fn require_unique_names(diagnostic: @mut span_handler, } } } + + +/** + * Fold this over attributes to parse #[repr(...)] forms. + * + * Valid repr contents: any of the primitive integral type names (see + * `int_type_of_word`, below) to specify the discriminant type; and `C`, to use + * the same discriminant size that the corresponding C enum would. These are + * not allowed on univariant or zero-variant enums, which have no discriminant. + * + * If a discriminant type is so specified, then the discriminant will be + * present (before fields, if any) with that type; reprensentation + * optimizations which would remove it will not be done. + */ +pub fn find_repr_attr(diagnostic: @mut span_handler, attr: @ast::MetaItem, acc: ReprAttr) + -> ReprAttr { + let mut acc = acc; + match attr.node { + ast::MetaList(s, ref items) if "repr" == s => { + for item in items.iter() { + match item.node { + ast::MetaWord(word) => { + let word: &str = word; + let hint = match word { + // Can't use "extern" because it's not a lexical identifier. + "C" => ReprExtern, + _ => match int_type_of_word(word) { + Some(ity) => ReprInt(item.span, ity), + None => { + // Not a word we recognize + diagnostic.span_err(item.span, + "unrecognized representation hint"); + ReprAny + } + } + }; + if hint != ReprAny { + if acc == ReprAny { + acc = hint; + } else if acc != hint { + diagnostic.span_warn(item.span, + "conflicting representation hint ignored") + } + } + } + // Not a word: + _ => diagnostic.span_err(item.span, "unrecognized representation hint") + } + } + } + // Not a "repr" hint: ignore. + _ => { } + } + return acc; +} + +fn int_type_of_word(s: &str) -> Option { + match s { + "i8" => Some(SignedInt(ast::ty_i8)), + "u8" => Some(UnsignedInt(ast::ty_u8)), + "i16" => Some(SignedInt(ast::ty_i16)), + "u16" => Some(UnsignedInt(ast::ty_u16)), + "i32" => Some(SignedInt(ast::ty_i32)), + "u32" => Some(UnsignedInt(ast::ty_u32)), + "i64" => Some(SignedInt(ast::ty_i64)), + "u64" => Some(UnsignedInt(ast::ty_u64)), + "int" => Some(SignedInt(ast::ty_i)), + "uint" => Some(UnsignedInt(ast::ty_u)), + _ => None + } +} + +#[deriving(Eq)] +pub enum ReprAttr { + ReprAny, + ReprInt(Span, IntType), + ReprExtern +} + +#[deriving(Eq)] +pub enum IntType { + SignedInt(ast::int_ty), + UnsignedInt(ast::uint_ty) +} + +impl IntType { + #[inline] + pub fn is_signed(self) -> bool { + match self { + SignedInt(*) => true, + UnsignedInt(*) => false + } + } +} From 01740acd5ab243e7b8c0e1e076ed56edcb9bf4d0 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 30 Jun 2013 22:42:30 -0700 Subject: [PATCH 02/17] Initial implementation of enum discrimnant sizing. Allows an enum with a discriminant to use any of the primitive integer types to store it. By default the smallest usable type is chosen, but this can be overridden with an attribute: `#[repr(int)]` etc., or `#[repr(C)]` to match the target's C ABI for the equivalent C enum. This commit breaks a few things, due to transmutes that now no longer match in size, or u8 enums being passed to C that expects int, or reflection; later commits on this branch fix them. --- src/librustc/middle/trans/adt.rs | 243 ++++++++++++++++----- src/librustc/middle/trans/debuginfo.rs | 50 +++-- src/librustc/middle/trans/expr.rs | 2 +- src/librustc/middle/trans/reflect.rs | 4 +- src/test/run-pass/small-enum-range-edge.rs | 32 +++ 5 files changed, 253 insertions(+), 78 deletions(-) create mode 100644 src/test/run-pass/small-enum-range-edge.rs diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 5e64dc5c2e2..f1b280338aa 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -14,7 +14,8 @@ * This module determines how to represent enums, structs, and tuples * based on their monomorphized types; it is responsible both for * choosing a representation and translating basic operations on - * values of those types. + * values of those types. (Note: exporting the representations for + * debuggers is handled in debuginfo.rs, not here.) * * Note that the interface treats everything as a general case of an * enum, so structs/tuples/etc. have one pseudo-variant with @@ -29,8 +30,6 @@ * that might contain one and adjust GEP indices accordingly. See * issue #4578. * - * - Using smaller integer types for discriminants. - * * - Store nested enums' discriminants in the same word. Rather, if * some variants start with enums, and those enums representations * have unused alignment padding between discriminant and body, the @@ -56,16 +55,21 @@ use middle::trans::machine; use middle::trans::type_of; use middle::ty; use middle::ty::Disr; +use syntax::abi::{X86, X86_64, Arm, Mips}; use syntax::ast; +use syntax::attr; +use syntax::attr::IntType; use util::ppaux::ty_to_str; use middle::trans::type_::Type; +type Hint = attr::ReprAttr; + /// Representations. pub enum Repr { /// C-like enums; basically an int. - CEnum(Disr, Disr), // discriminant range + CEnum(IntType, Disr, Disr), // discriminant range (signedness based on the IntType) /** * Single-case variants, and structs/tuples/records. * @@ -78,7 +82,7 @@ pub enum Repr { * General-case enums: for each case there is a struct, and they * all start with a field for the discriminant. */ - General(~[Struct]), + General(IntType, ~[Struct]), /** * Two cases distinguished by a nullable pointer: the case with discriminant * `nndiscr` is represented by the struct `nonnull`, where the `ptrfield`th @@ -166,11 +170,19 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { if cases.iter().all(|c| c.tys.len() == 0) { // All bodies empty -> intlike let discrs = cases.map(|c| c.discr); - return CEnum(*discrs.iter().min().unwrap(), *discrs.iter().max().unwrap()); + let hint = ty::lookup_repr_hint(cx.tcx, def_id); + let bounds = IntBounds { + ulo: *discrs.iter().min().unwrap(), + uhi: *discrs.iter().max().unwrap(), + slo: discrs.iter().map(|n| *n as i64).min().unwrap(), + shi: discrs.iter().map(|n| *n as i64).max().unwrap() + }; + return mk_cenum(cx, hint, &bounds); } if cases.len() == 1 { // Equivalent to a struct/tuple/newtype. + // FIXME: should this conflict with a discriminant size hint? assert_eq!(cases[0].discr, 0); return Univariant(mk_struct(cx, cases[0].tys, false), false) } @@ -185,6 +197,7 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { } if cases.len() == 2 { + // FIXME: disable if size hint present? let mut discr = 0; while discr < 2 { if cases[1 - discr].is_zerolen(cx) { @@ -207,8 +220,13 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { } // The general case. - let discr = ~[ty::mk_uint()]; - return General(cases.map(|c| mk_struct(cx, discr + c.tys, false))) + let hint = ty::lookup_repr_hint(cx.tcx, def_id); + assert!((cases.len() - 1) as i64 >= 0); + let bounds = IntBounds { ulo: 0, uhi: (cases.len() - 1) as u64, + slo: 0, shi: (cases.len() - 1) as i64 }; + let ity = range_to_inttype(cx, hint, &bounds); + let discr = ~[ty_of_inttype(ity)]; + return General(ity, cases.map(|c| mk_struct(cx, discr + c.tys, false))) } _ => cx.sess.bug("adt::represent_type called on non-ADT type") } @@ -225,6 +243,93 @@ fn mk_struct(cx: &mut CrateContext, tys: &[ty::t], packed: bool) -> Struct { } } +struct IntBounds { + slo: i64, + shi: i64, + ulo: u64, + uhi: u64 +} + +fn mk_cenum(cx: &mut CrateContext, hint: Hint, bounds: &IntBounds) -> Repr { + let it = range_to_inttype(cx, hint, bounds); + match it { + attr::SignedInt(_) => CEnum(it, bounds.slo as Disr, bounds.shi as Disr), + attr::UnsignedInt(_) => CEnum(it, bounds.ulo, bounds.uhi) + } +} + +fn range_to_inttype(cx: &mut CrateContext, hint: Hint, bounds: &IntBounds) -> IntType { + debug!("range_to_inttype: {:?} {:?}", hint, bounds); + // Lists of sizes to try. u64 is always allowed as a fallback. + static choose_shortest: &'static[IntType] = &[ + attr::UnsignedInt(ast::ty_u8), attr::SignedInt(ast::ty_i8), + attr::UnsignedInt(ast::ty_u16), attr::SignedInt(ast::ty_i16), + attr::UnsignedInt(ast::ty_u32), attr::SignedInt(ast::ty_i32)]; + static at_least_32: &'static[IntType] = &[ + attr::UnsignedInt(ast::ty_u32), attr::SignedInt(ast::ty_i32)]; + + let attempts; + match hint { + attr::ReprInt(span, ity) => { + if !bounds_usable(cx, ity, bounds) { + cx.sess.span_err(span, "representation hint insufficient for discriminant range") + } + return ity; + } + attr::ReprExtern => { + attempts = match cx.sess.targ_cfg.arch { + X86 | X86_64 => at_least_32, + // WARNING: the ARM EABI has two variants; the one corresponding to `at_least_32` + // appears to be used on Linux and NetBSD, but some systems may use the variant + // corresponding to `choose_shortest`. However, we don't run on those yet...? + Arm => at_least_32, + Mips => at_least_32, + } + } + attr::ReprAny => { + attempts = choose_shortest; + } + } + let mut best = attr::UnsignedInt(ast::ty_u64); + for &ity in attempts.iter() { + if bounds_usable(cx, ity, bounds) { + best = ity; + break; + } + } + return best; +} + +pub fn ll_inttype(cx: &mut CrateContext, ity: IntType) -> Type { + match ity { + attr::SignedInt(t) => Type::int_from_ty(cx, t), + attr::UnsignedInt(t) => Type::uint_from_ty(cx, t) + } +} + +fn bounds_usable(cx: &mut CrateContext, ity: IntType, bounds: &IntBounds) -> bool { + debug!("bounds_usable: {:?} {:?}", ity, bounds); + match ity { + attr::SignedInt(_) => { + let lllo = C_integral(ll_inttype(cx, ity), bounds.slo as u64, true); + let llhi = C_integral(ll_inttype(cx, ity), bounds.shi as u64, true); + bounds.slo == const_to_int(lllo) as i64 && bounds.shi == const_to_int(llhi) as i64 + } + attr::UnsignedInt(_) => { + let lllo = C_integral(ll_inttype(cx, ity), bounds.ulo, false); + let llhi = C_integral(ll_inttype(cx, ity), bounds.uhi, false); + bounds.ulo == const_to_uint(lllo) as u64 && bounds.uhi == const_to_uint(llhi) as u64 + } + } +} + +fn ty_of_inttype(ity: IntType) -> ty::t { + match ity { + attr::SignedInt(t) => ty::mk_mach_int(t), + attr::UnsignedInt(t) => ty::mk_mach_uint(t) + } +} + /** * Returns the fields of a struct for the given representation. * All nominal types are LLVM structs, in order to be able to use @@ -239,10 +344,10 @@ pub fn sizing_fields_of(cx: &mut CrateContext, r: &Repr) -> ~[Type] { } fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] { match *r { - CEnum(*) => ~[Type::enum_discrim(cx)], + CEnum(ity, _, _) => ~[ll_inttype(cx, ity)], Univariant(ref st, _dtor) => struct_llfields(cx, st, sizing), NullablePointer{ nonnull: ref st, _ } => struct_llfields(cx, st, sizing), - General(ref sts) => { + General(_ity, ref sts) => { // To get "the" type of a general enum, we pick the case // with the largest alignment (so it will always align // correctly in containing structures) and pad it out. @@ -288,7 +393,7 @@ pub fn trans_switch(bcx: @mut Block, r: &Repr, scrutinee: ValueRef) -> (_match::branch_kind, Option) { match *r { CEnum(*) | General(*) => { - (_match::switch, Some(trans_get_discr(bcx, r, scrutinee))) + (_match::switch, Some(trans_get_discr(bcx, r, scrutinee, None))) } NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => { (_match::switch, Some(nullable_bitdiscr(bcx, nonnull, nndiscr, ptrfield, scrutinee))) @@ -302,16 +407,31 @@ pub fn trans_switch(bcx: @mut Block, r: &Repr, scrutinee: ValueRef) /// Obtain the actual discriminant of a value. -pub fn trans_get_discr(bcx: @mut Block, r: &Repr, scrutinee: ValueRef) +pub fn trans_get_discr(bcx: @mut Block, r: &Repr, scrutinee: ValueRef, cast_to: Option) -> ValueRef { + let signed; + let val; match *r { - CEnum(min, max) => load_discr(bcx, scrutinee, min, max), - Univariant(*) => C_disr(bcx.ccx(), 0), - General(ref cases) => load_discr(bcx, scrutinee, 0, (cases.len() - 1) as Disr), - NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => { - ZExt(bcx, nullable_bitdiscr(bcx, nonnull, nndiscr, ptrfield, scrutinee), - Type::enum_discrim(bcx.ccx())) + CEnum(ity, min, max) => { + val = load_discr(bcx, ity, scrutinee, min, max); + signed = ity.is_signed(); } + General(ity, ref cases) => { + val = load_discr(bcx, ity, scrutinee, 0, (cases.len() - 1) as Disr); + signed = ity.is_signed(); + } + Univariant(*) => { + val = C_u8(0); + signed = false; + } + NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => { + val = nullable_bitdiscr(bcx, nonnull, nndiscr, ptrfield, scrutinee); + signed = false; + } + } + match cast_to { + None => val, + Some(llty) => if signed { SExt(bcx, val, llty) } else { ZExt(bcx, val, llty) } } } @@ -324,10 +444,15 @@ fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: Disr, ptrfield: } /// Helper for cases where the discriminant is simply loaded. -fn load_discr(bcx: @mut Block, scrutinee: ValueRef, min: Disr, max: Disr) +fn load_discr(bcx: @mut Block, ity: IntType, scrutinee: ValueRef, min: Disr, max: Disr) -> ValueRef { let ptr = GEPi(bcx, scrutinee, [0, 0]); - if max + 1 == min { + let llty = ll_inttype(bcx.ccx(), ity); + assert_eq!(val_ty(ptr), llty.ptr_to()); + let bits = machine::llbitsize_of_real(bcx.ccx(), llty); + assert!(bits <= 64); + let mask = (-1u64 >> (64 - bits)) as Disr; + if (max + 1) & mask == min & mask { // i.e., if the range is everything. The lo==hi case would be // rejected by the LLVM verifier (it would mean either an // empty set, which is impossible, or the entire range of the @@ -350,15 +475,17 @@ fn load_discr(bcx: @mut Block, scrutinee: ValueRef, min: Disr, max: Disr) */ pub fn trans_case(bcx: @mut Block, r: &Repr, discr: Disr) -> _match::opt_result { match *r { - CEnum(*) => { - _match::single_result(rslt(bcx, C_disr(bcx.ccx(), discr))) + CEnum(ity, _, _) => { + _match::single_result(rslt(bcx, C_integral(ll_inttype(bcx.ccx(), ity), + discr as u64, true))) + } + General(ity, _) => { + _match::single_result(rslt(bcx, C_integral(ll_inttype(bcx.ccx(), ity), + discr as u64, true))) } Univariant(*) => { bcx.ccx().sess.bug("no cases for univariants or structs") } - General(*) => { - _match::single_result(rslt(bcx, C_disr(bcx.ccx(), discr))) - } NullablePointer{ _ } => { assert!(discr == 0 || discr == 1); _match::single_result(rslt(bcx, C_i1(discr != 0))) @@ -373,9 +500,14 @@ pub fn trans_case(bcx: @mut Block, r: &Repr, discr: Disr) -> _match::opt_result */ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr) { match *r { - CEnum(min, max) => { - assert!(min <= discr && discr <= max); - Store(bcx, C_disr(bcx.ccx(), discr), GEPi(bcx, val, [0, 0])) + CEnum(ity, min, max) => { + assert_discr_in_range(ity, min, max, discr); + Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true), + GEPi(bcx, val, [0, 0])) + } + General(ity, _) => { + Store(bcx, C_integral(ll_inttype(bcx.ccx(), ity), discr as u64, true), + GEPi(bcx, val, [0, 0])) } Univariant(ref st, true) => { assert_eq!(discr, 0); @@ -385,9 +517,6 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr) { Univariant(*) => { assert_eq!(discr, 0); } - General(*) => { - Store(bcx, C_disr(bcx.ccx(), discr), GEPi(bcx, val, [0, 0])) - } NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => { if discr != nndiscr { let llptrptr = GEPi(bcx, val, [0, ptrfield]); @@ -398,6 +527,13 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr) { } } +fn assert_discr_in_range(ity: IntType, min: Disr, max: Disr, discr: Disr) { + match ity { + attr::UnsignedInt(_) => assert!(min <= discr && discr <= max), + attr::SignedInt(_) => assert!(min as i64 <= discr as i64 && discr as i64 <= max as i64) + } +} + /** * The number of fields in a given case; for use when obtaining this * information from the type or definition is less convenient. @@ -409,7 +545,7 @@ pub fn num_args(r: &Repr, discr: Disr) -> uint { assert_eq!(discr, 0); st.fields.len() - (if dtor { 1 } else { 0 }) } - General(ref cases) => cases[discr].fields.len() - 1, + General(_, ref cases) => cases[discr].fields.len() - 1, NullablePointer{ nonnull: ref nonnull, nndiscr, nullfields: ref nullfields, _ } => { if discr == nndiscr { nonnull.fields.len() } else { nullfields.len() } } @@ -430,7 +566,7 @@ pub fn trans_field_ptr(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr, assert_eq!(discr, 0); struct_field_ptr(bcx, st, val, ix, false) } - General(ref cases) => { + General(_, ref cases) => { struct_field_ptr(bcx, &cases[discr], val, ix + 1, true) } NullablePointer{ nonnull: ref nonnull, nullfields: ref nullfields, nndiscr, _ } => { @@ -498,24 +634,23 @@ pub fn trans_drop_flag_ptr(bcx: @mut Block, r: &Repr, val: ValueRef) -> ValueRef pub fn trans_const(ccx: &mut CrateContext, r: &Repr, discr: Disr, vals: &[ValueRef]) -> ValueRef { match *r { - CEnum(min, max) => { + CEnum(ity, min, max) => { assert_eq!(vals.len(), 0); - assert!(min <= discr && discr <= max); - C_disr(ccx, discr) + assert_discr_in_range(ity, min, max, discr); + C_integral(ll_inttype(ccx, ity), discr as u64, true) } - Univariant(ref st, _dro) => { - assert_eq!(discr, 0); - let contents = build_const_struct(ccx, st, vals); - C_struct(contents, st.packed) - } - General(ref cases) => { + General(ity, ref cases) => { let case = &cases[discr]; let max_sz = cases.iter().map(|x| x.size).max().unwrap(); - let discr_ty = C_disr(ccx, discr); - let contents = build_const_struct(ccx, case, - ~[discr_ty] + vals); + let lldiscr = C_integral(ll_inttype(ccx, ity), discr as u64, true); + let contents = build_const_struct(ccx, case, ~[lldiscr] + vals); C_struct(contents + &[padding(max_sz - case.size)], false) } + Univariant(ref st, _dro) => { + assert!(discr == 0); + let contents = build_const_struct(ccx, st, vals); + C_struct(contents, st.packed) + } NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => { if discr == nndiscr { C_struct(build_const_struct(ccx, nonnull, vals), false) @@ -585,9 +720,19 @@ fn roundup(x: u64, a: u64) -> u64 { ((x + (a - 1)) / a) * a } pub fn const_get_discrim(ccx: &mut CrateContext, r: &Repr, val: ValueRef) -> Disr { match *r { - CEnum(*) => const_to_uint(val) as Disr, + CEnum(ity, _, _) => { + match ity { + attr::SignedInt(*) => const_to_int(val) as Disr, + attr::UnsignedInt(*) => const_to_uint(val) as Disr + } + } + General(ity, _) => { + match ity { + attr::SignedInt(*) => const_to_int(const_get_elt(ccx, val, [0])) as Disr, + attr::UnsignedInt(*) => const_to_uint(const_get_elt(ccx, val, [0])) as Disr + } + } Univariant(*) => 0, - General(*) => const_to_uint(const_get_elt(ccx, val, [0])) as Disr, NullablePointer{ nndiscr, ptrfield, _ } => { if is_null(const_struct_field(ccx, val, ptrfield)) { /* subtraction as uint is ok because nndiscr is either 0 or 1 */ @@ -646,7 +791,3 @@ pub fn is_newtypeish(r: &Repr) -> bool { _ => false } } - -fn C_disr(cx: &CrateContext, i: Disr) -> ValueRef { - return C_integral(cx.int_type, i, false); -} diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 2138afd2e9b..616af475609 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -109,6 +109,7 @@ use std::libc::{c_uint, c_ulonglong, c_longlong}; use std::ptr; use std::unstable::atomics; use std::vec; +use syntax::attr; use syntax::codemap::{Span, Pos}; use syntax::{ast, codemap, ast_util, ast_map, opt_vec}; use syntax::parse::token; @@ -1250,7 +1251,7 @@ impl MemberDescriptionFactory for GeneralMemberDescriptionFactory { -> ~[MemberDescription] { // Capture type_rep, so we don't have to copy the struct_defs array let struct_defs = match *self.type_rep { - adt::General(ref struct_defs) => struct_defs, + adt::General(_, ref struct_defs) => struct_defs, _ => cx.sess.bug("unreachable") }; @@ -1399,14 +1400,6 @@ fn prepare_enum_metadata(cx: &mut CrateContext, return FinalMetadata(empty_type_metadata); } - // Prepare some data (llvm type, size, align, etc) about the discriminant. This data will be - // needed in all of the following cases. - let discriminant_llvm_type = Type::enum_discrim(cx); - let (discriminant_size, discriminant_align) = size_and_align_of(cx, discriminant_llvm_type); - - assert!(Type::enum_discrim(cx) == cx.int_type); - let discriminant_base_type_metadata = type_metadata(cx, ty::mk_int(), codemap::dummy_sp()); - let variants = ty::enum_variants(cx.tcx, enum_def_id); let enumerators_metadata: ~[DIDescriptor] = variants @@ -1426,26 +1419,34 @@ fn prepare_enum_metadata(cx: &mut CrateContext, }) .collect(); - let discriminant_type_metadata = do enum_name.with_c_str |enum_name| { - unsafe { - llvm::LLVMDIBuilderCreateEnumerationType( - DIB(cx), - containing_scope, - enum_name, - file_metadata, - loc.line as c_uint, - bytes_to_bits(discriminant_size), - bytes_to_bits(discriminant_align), - create_DIArray(DIB(cx), enumerators_metadata), - discriminant_base_type_metadata) + let discriminant_type_metadata = |inttype| { + let discriminant_llvm_type = adt::ll_inttype(cx, inttype); + let (discriminant_size, discriminant_align) = size_and_align_of(cx, discriminant_llvm_type); + let discriminant_base_type_metadata = type_metadata(cx, match inttype { + attr::SignedInt(t) => ty::mk_mach_int(t), + attr::UnsignedInt(t) => ty::mk_mach_uint(t) + }, codemap::dummy_sp()); + do enum_name.with_c_str |enum_name| { + unsafe { + llvm::LLVMDIBuilderCreateEnumerationType( + DIB(cx), + containing_scope, + enum_name, + file_metadata, + loc.line as c_uint, + bytes_to_bits(discriminant_size), + bytes_to_bits(discriminant_align), + create_DIArray(DIB(cx), enumerators_metadata), + discriminant_base_type_metadata) + } } }; let type_rep = adt::represent_type(cx, enum_type); return match *type_rep { - adt::CEnum(*) => { - FinalMetadata(discriminant_type_metadata) + adt::CEnum(inttype, _, _) => { + FinalMetadata(discriminant_type_metadata(inttype)) } adt::Univariant(ref struct_def, _) => { assert!(variants.len() == 1); @@ -1466,7 +1467,8 @@ fn prepare_enum_metadata(cx: &mut CrateContext, member_description_factory: member_description_factory } } - adt::General(_) => { + adt::General(inttype, _) => { + let discriminant_type_metadata = discriminant_type_metadata(inttype); let enum_llvm_type = type_of::type_of(cx, enum_type); let (enum_type_size, enum_type_align) = size_and_align_of(cx, enum_llvm_type); diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 1c856f04b06..e1de6615206 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1728,7 +1728,7 @@ fn trans_imm_cast(bcx: @mut Block, expr: &ast::Expr, let repr = adt::represent_type(ccx, t_in); let slot = Alloca(bcx, ll_t_in, ""); Store(bcx, llexpr, slot); - let lldiscrim_a = adt::trans_get_discr(bcx, repr, slot); + let lldiscrim_a = adt::trans_get_discr(bcx, repr, slot, Some(Type::i64())); match k_out { cast_integral => int_cast(bcx, ll_t_out, val_ty(lldiscrim_a), diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index 54ecc15c44f..1b27e06dca8 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -25,7 +25,7 @@ use middle::ty; use util::ppaux::ty_to_str; use std::libc::c_uint; -use std::option::None; +use std::option::{Some,None}; use std::vec; use syntax::ast::DefId; use syntax::ast; @@ -308,7 +308,7 @@ impl Reflector { }; let mut bcx = fcx.entry_bcx.unwrap(); let arg = BitCast(bcx, arg, llptrty); - let ret = adt::trans_get_discr(bcx, repr, arg); + let ret = adt::trans_get_discr(bcx, repr, arg, Some(ccx.int_type)); Store(bcx, ret, fcx.llretptr.unwrap()); match fcx.llreturn { Some(llreturn) => cleanup_and_Br(bcx, bcx, llreturn), diff --git a/src/test/run-pass/small-enum-range-edge.rs b/src/test/run-pass/small-enum-range-edge.rs new file mode 100644 index 00000000000..a9fc13437ab --- /dev/null +++ b/src/test/run-pass/small-enum-range-edge.rs @@ -0,0 +1,32 @@ +// Copyright 2013 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. + +/*! + * Tests the range assertion wraparound case in trans::middle::adt::load_discr. + */ + +#[repr(u8)] +enum Eu { Lu = 0, Hu = 255 } +static CLu: Eu = Lu; +static CHu: Eu = Hu; + +#[repr(i8)] +enum Es { Ls = -128, Hs = 127 } +static CLs: Es = Ls; +static CHs: Es = Hs; + +pub fn main() { + assert_eq!((Hu as u8) + 1, Lu as u8); + assert_eq!((Hs as i8) + 1, Ls as i8); + assert_eq!(CLu as u8, Lu as u8); + assert_eq!(CHu as u8, Hu as u8); + assert_eq!(CLs as i8, Ls as i8); + assert_eq!(CHs as i8, Hs as i8); +} From 25f953437d2bc3983b03c0d17cf56de40acec45b Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 2 Jun 2013 13:03:35 -0700 Subject: [PATCH 03/17] Lint non-FFI-safe enums. --- src/librustc/middle/lint.rs | 9 +++++ src/librustc/middle/trans/adt.rs | 69 ++++++++++++++++++++++++-------- src/libsyntax/attr.rs | 19 +++++++++ 3 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 1ac14dfc4b1..e8f9dad6585 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -34,6 +34,7 @@ //! Context itself, span_lint should be used instead of add_lint. use driver::session; +use middle::trans::adt; // for `adt::is_ffi_safe` use middle::ty; use middle::pat_util; use metadata::csearch; @@ -627,6 +628,14 @@ fn check_item_ctypes(cx: &Context, it: &ast::item) { "found rust type `uint` in foreign module, while \ libc::c_uint or libc::c_ulong should be used"); } + ast::DefTy(def_id) => { + if !adt::is_ffi_safe(cx.tcx, def_id) { + cx.span_lint(ctypes, ty.span, + "found enum type without foreign-function-safe \ + representation annotation in foreign module"); + // NOTE this message could be more helpful + } + } _ => () } } diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index f1b280338aa..10b7655ae45 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -145,22 +145,8 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { return Univariant(mk_struct(cx, ftys, packed), dtor) } ty::ty_enum(def_id, ref substs) => { - struct Case { discr: Disr, tys: ~[ty::t] }; - impl Case { - fn is_zerolen(&self, cx: &mut CrateContext) -> bool { - mk_struct(cx, self.tys, false).size == 0 - } - fn find_ptr(&self) -> Option { - self.tys.iter().position(|&ty| mono_data_classify(ty) == MonoNonNull) - } - } - - let cases = do ty::enum_variants(cx.tcx, def_id).map |vi| { - let arg_tys = do vi.args.map |&raw_ty| { - ty::subst(cx.tcx, substs, raw_ty) - }; - Case { discr: vi.disr_val, tys: arg_tys } - }; + let cases = get_cases(cx.tcx, def_id, substs); + let hint = ty::lookup_repr_hint(cx.tcx, def_id); if cases.len() == 0 { // Uninhabitable; represent as unit @@ -170,7 +156,6 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { if cases.iter().all(|c| c.tys.len() == 0) { // All bodies empty -> intlike let discrs = cases.map(|c| c.discr); - let hint = ty::lookup_repr_hint(cx.tcx, def_id); let bounds = IntBounds { ulo: *discrs.iter().min().unwrap(), uhi: *discrs.iter().max().unwrap(), @@ -232,6 +217,56 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { } } +/// Determine, without doing translation, whether an ADT must be FFI-safe. +/// For use in lint or similar, where being sound but slightly incomplete is acceptable. +pub fn is_ffi_safe(tcx: ty::ctxt, def_id: ast::DefId) -> bool { + match ty::get(ty::lookup_item_type(tcx, def_id).ty).sty { + ty::ty_enum(def_id, ref substs) => { + let cases = get_cases(tcx, def_id, substs); + // Univariant => like struct/tuple. + if cases.len() <= 2 { + return true; + } + let hint = ty::lookup_repr_hint(tcx, def_id); + // Appropriate representation explicitly selected? + if hint.is_ffi_safe() { + return true; + } + // Conservative approximation of nullable pointers, for Option<~T> etc. + if cases.len() == 2 && hint == attr::ReprAny && + (cases[0].tys.is_empty() && cases[1].find_ptr().is_some() || + cases[1].tys.is_empty() && cases[0].find_ptr().is_some()) { + return true; + } + false + } + // struct, tuple, etc. + // (is this right in the present of typedefs?) + _ => true + } +} + +// NOTE this should probably all be in ty +struct Case { discr: Disr, tys: ~[ty::t] } +impl Case { + fn is_zerolen(&self, cx: &mut CrateContext) -> bool { + mk_struct(cx, self.tys, false).size == 0 + } + fn find_ptr(&self) -> Option { + self.tys.iter().position(|&ty| mono_data_classify(ty) == MonoNonNull) + } +} + +fn get_cases(tcx: ty::ctxt, def_id: ast::DefId, substs: &ty::substs) -> ~[Case] { + do ty::enum_variants(tcx, def_id).map |vi| { + let arg_tys = do vi.args.map |&raw_ty| { + ty::subst(tcx, substs, raw_ty) + }; + Case { discr: vi.disr_val, tys: arg_tys } + } +} + + fn mk_struct(cx: &mut CrateContext, tys: &[ty::t], packed: bool) -> Struct { let lltys = tys.map(|&ty| type_of::sizing_type_of(cx, ty)); let llty_rec = Type::struct_(lltys, packed); diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index e538e09c056..222ca61a791 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -442,6 +442,16 @@ pub enum ReprAttr { ReprExtern } +impl ReprAttr { + pub fn is_ffi_safe(&self) -> bool { + match *self { + ReprAny => false, + ReprInt(_sp, ity) => ity.is_ffi_safe(), + ReprExtern => true + } + } +} + #[deriving(Eq)] pub enum IntType { SignedInt(ast::int_ty), @@ -456,4 +466,13 @@ impl IntType { UnsignedInt(*) => false } } + fn is_ffi_safe(self) -> bool { + match self { + SignedInt(ast::ty_i8) | UnsignedInt(ast::ty_u8) | + SignedInt(ast::ty_i16) | UnsignedInt(ast::ty_u16) | + SignedInt(ast::ty_i32) | UnsignedInt(ast::ty_u32) | + SignedInt(ast::ty_i64) | UnsignedInt(ast::ty_u64) => true, + _ => false + } + } } From ac311ecaab5d5a92fa86e9b201971bf92bcfbf99 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 3 Oct 2013 10:11:24 -0700 Subject: [PATCH 04/17] Fix multiple mistakes in adt::is_ffi_safe --- src/librustc/middle/trans/adt.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 10b7655ae45..26f8ad6bd47 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -221,10 +221,10 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { /// For use in lint or similar, where being sound but slightly incomplete is acceptable. pub fn is_ffi_safe(tcx: ty::ctxt, def_id: ast::DefId) -> bool { match ty::get(ty::lookup_item_type(tcx, def_id).ty).sty { - ty::ty_enum(def_id, ref substs) => { - let cases = get_cases(tcx, def_id, substs); + ty::ty_enum(def_id, _) => { + let variants = ty::enum_variants(tcx, def_id); // Univariant => like struct/tuple. - if cases.len() <= 2 { + if variants.len() <= 1 { return true; } let hint = ty::lookup_repr_hint(tcx, def_id); @@ -232,10 +232,10 @@ pub fn is_ffi_safe(tcx: ty::ctxt, def_id: ast::DefId) -> bool { if hint.is_ffi_safe() { return true; } - // Conservative approximation of nullable pointers, for Option<~T> etc. - if cases.len() == 2 && hint == attr::ReprAny && - (cases[0].tys.is_empty() && cases[1].find_ptr().is_some() || - cases[1].tys.is_empty() && cases[0].find_ptr().is_some()) { + // Option<~T> and similar are used in FFI. Rather than try to resolve type parameters + // and recognize this case exactly, this overapproximates -- assuming that if a + // non-C-like enum is being used in FFI then the user knows what they're doing. + if variants.iter().any(|vi| !vi.args.is_empty()) { return true; } false From c8c08763ec12b0e693f400390957249c1f6583b9 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Wed, 28 Aug 2013 18:21:04 -0700 Subject: [PATCH 05/17] Add repr attributes in various places that need them. --- src/libextra/enum_set.rs | 2 +- src/librustc/lib/llvm.rs | 6 ++++++ src/librustc/metadata/common.rs | 4 ++-- src/librustc/middle/ty.rs | 2 +- src/libstd/rt/io/signal.rs | 1 + src/test/run-pass/issue-2718.rs | 2 +- 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libextra/enum_set.rs b/src/libextra/enum_set.rs index da9e0a225ba..e0b1e694a49 100644 --- a/src/libextra/enum_set.rs +++ b/src/libextra/enum_set.rs @@ -139,7 +139,7 @@ mod test { use enum_set::*; - #[deriving(Eq)] + #[deriving(Eq)] #[repr(uint)] enum Foo { A, B, C } diff --git a/src/librustc/lib/llvm.rs b/src/librustc/lib/llvm.rs index 64c6678ec5f..4eb8a0b8fa6 100644 --- a/src/librustc/lib/llvm.rs +++ b/src/librustc/lib/llvm.rs @@ -147,6 +147,7 @@ pub static Vector: TypeKind = 13; pub static Metadata: TypeKind = 14; pub static X86_MMX: TypeKind = 15; +#[repr(C)] pub enum AtomicBinOp { Xchg = 0, Add = 1, @@ -161,6 +162,7 @@ pub enum AtomicBinOp { UMin = 10, } +#[repr(C)] pub enum AtomicOrdering { NotAtomic = 0, Unordered = 1, @@ -173,6 +175,7 @@ pub enum AtomicOrdering { } // Consts for the LLVMCodeGenFileType type (in include/llvm/c/TargetMachine.h) +#[repr(C)] pub enum FileType { AssemblyFile = 0, ObjectFile = 1 @@ -194,6 +197,7 @@ pub enum AsmDialect { } #[deriving(Eq)] +#[repr(C)] pub enum CodeGenOptLevel { CodeGenLevelNone = 0, CodeGenLevelLess = 1, @@ -201,6 +205,7 @@ pub enum CodeGenOptLevel { CodeGenLevelAggressive = 3, } +#[repr(C)] pub enum RelocMode { RelocDefault = 0, RelocStatic = 1, @@ -208,6 +213,7 @@ pub enum RelocMode { RelocDynamicNoPic = 3, } +#[repr(C)] pub enum CodeGenModel { CodeModelDefault = 0, CodeModelJITDefault = 1, diff --git a/src/librustc/metadata/common.rs b/src/librustc/metadata/common.rs index c3bc3e0fe25..3692d571a53 100644 --- a/src/librustc/metadata/common.rs +++ b/src/librustc/metadata/common.rs @@ -111,7 +111,7 @@ pub static tag_items_data_item_reexport_def_id: uint = 0x4e; pub static tag_items_data_item_reexport_name: uint = 0x4f; // used to encode crate_ctxt side tables -#[deriving(Eq)] +#[deriving(Eq)] #[repr(uint)] pub enum astencode_tag { // Reserves 0x50 -- 0x6f tag_ast = 0x50, @@ -143,7 +143,7 @@ impl astencode_tag { pub fn from_uint(value : uint) -> Option { let is_a_tag = first_astencode_tag <= value && value <= last_astencode_tag; if !is_a_tag { None } else { - Some(unsafe { cast::transmute(value as int) }) + Some(unsafe { cast::transmute(value) }) } } } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 3945fcd80ae..7d4fafb29f6 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -715,7 +715,7 @@ pub struct ParamBounds { pub type BuiltinBounds = EnumSet; -#[deriving(Clone, Eq, IterBytes, ToStr)] +#[deriving(Clone, Eq, IterBytes, ToStr)] #[repr(uint)] pub enum BuiltinBound { BoundStatic, BoundSend, diff --git a/src/libstd/rt/io/signal.rs b/src/libstd/rt/io/signal.rs index 4c6c675df03..b782d713950 100644 --- a/src/libstd/rt/io/signal.rs +++ b/src/libstd/rt/io/signal.rs @@ -26,6 +26,7 @@ use result::{Err, Ok}; use rt::io::io_error; use rt::rtio::{IoFactory, RtioSignal, with_local_io}; +#[repr(int)] #[deriving(Eq, IterBytes)] pub enum Signum { /// Equivalent to SIGBREAK, delivered when the user presses Ctrl-Break. diff --git a/src/test/run-pass/issue-2718.rs b/src/test/run-pass/issue-2718.rs index 7cebfa13223..c2c8213517c 100644 --- a/src/test/run-pass/issue-2718.rs +++ b/src/test/run-pass/issue-2718.rs @@ -26,7 +26,7 @@ pub mod pipes { payload: Option } - #[deriving(Eq)] + #[deriving(Eq)] #[repr(int)] pub enum state { empty, full, From a027f164bcda6f99d0b44f79ca9f676ecc12a50a Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 29 Aug 2013 23:45:06 -0700 Subject: [PATCH 06/17] Check repr attribute consistency at check time, not translation. Note that raising an error during trans doesn't stop the compile or cause rustc to exit with a failure status, currently, so this is of more than cosmetic importance. --- src/librustc/middle/trans/adt.rs | 24 +++++---- src/librustc/middle/typeck/check/mod.rs | 54 +++++++++++++++++-- src/test/compile-fail/tag-variant-disr-dup.rs | 2 +- 3 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 26f8ad6bd47..687af6e6c8c 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -150,6 +150,8 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { if cases.len() == 0 { // Uninhabitable; represent as unit + // (Typechecking will reject discriminant-sizing attrs.) + assert_eq!(hint, attr::ReprAny); return Univariant(mk_struct(cx, [], false), false); } @@ -165,13 +167,6 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { return mk_cenum(cx, hint, &bounds); } - if cases.len() == 1 { - // Equivalent to a struct/tuple/newtype. - // FIXME: should this conflict with a discriminant size hint? - assert_eq!(cases[0].discr, 0); - return Univariant(mk_struct(cx, cases[0].tys, false), false) - } - // Since there's at least one // non-empty body, explicit discriminants should have // been rejected by a checker before this point. @@ -181,8 +176,15 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { ty::item_path_str(cx.tcx, def_id))) } - if cases.len() == 2 { - // FIXME: disable if size hint present? + if cases.len() == 1 { + // Equivalent to a struct/tuple/newtype. + // (Typechecking will reject discriminant-sizing attrs.) + assert_eq!(hint, attr::ReprAny); + return Univariant(mk_struct(cx, cases[0].tys, false), false) + } + + if cases.len() == 2 && hint == attr::ReprAny { + // Nullable pointer optimization let mut discr = 0; while discr < 2 { if cases[1 - discr].is_zerolen(cx) { @@ -205,7 +207,6 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { } // The general case. - let hint = ty::lookup_repr_hint(cx.tcx, def_id); assert!((cases.len() - 1) as i64 >= 0); let bounds = IntBounds { ulo: 0, uhi: (cases.len() - 1) as u64, slo: 0, shi: (cases.len() - 1) as i64 }; @@ -307,7 +308,7 @@ fn range_to_inttype(cx: &mut CrateContext, hint: Hint, bounds: &IntBounds) -> In match hint { attr::ReprInt(span, ity) => { if !bounds_usable(cx, ity, bounds) { - cx.sess.span_err(span, "representation hint insufficient for discriminant range") + cx.sess.span_bug(span, "representation hint insufficient for discriminant range") } return ity; } @@ -365,6 +366,7 @@ fn ty_of_inttype(ity: IntType) -> ty::t { } } + /** * Returns the fields of a struct for the given representation. * All nominal types are LLVM structs, in order to be able to use diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index d71aa776689..15660618f8e 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -121,6 +121,7 @@ use syntax::ast; use syntax::ast_map; use syntax::ast_util::local_def; use syntax::ast_util; +use syntax::attr; use syntax::codemap::Span; use syntax::codemap; use syntax::opt_vec::OptVec; @@ -3159,9 +3160,38 @@ pub fn check_enum_variants(ccx: @mut CrateCtxt, sp: Span, vs: &[ast::variant], id: ast::NodeId) { + + fn disr_in_range(ccx: @mut CrateCtxt, + ty: attr::IntType, + disr: ty::Disr) -> bool { + fn uint_in_range(ccx: @mut CrateCtxt, ty: ast::uint_ty, disr: ty::Disr) -> bool { + match ty { + ast::ty_u8 => disr as u8 as Disr == disr, + ast::ty_u16 => disr as u16 as Disr == disr, + ast::ty_u32 => disr as u32 as Disr == disr, + ast::ty_u64 => disr as u64 as Disr == disr, + ast::ty_u => uint_in_range(ccx, ccx.tcx.sess.targ_cfg.uint_type, disr) + } + } + fn int_in_range(ccx: @mut CrateCtxt, ty: ast::int_ty, disr: ty::Disr) -> bool { + match ty { + ast::ty_i8 => disr as i8 as Disr == disr, + ast::ty_i16 => disr as i16 as Disr == disr, + ast::ty_i32 => disr as i32 as Disr == disr, + ast::ty_i64 => disr as i64 as Disr == disr, + ast::ty_i => int_in_range(ccx, ccx.tcx.sess.targ_cfg.int_type, disr) + } + } + match ty { + attr::UnsignedInt(ty) => uint_in_range(ccx, ty, disr), + attr::SignedInt(ty) => int_in_range(ccx, ty, disr) + } + } + fn do_check(ccx: @mut CrateCtxt, vs: &[ast::variant], - id: ast::NodeId) + id: ast::NodeId, + hint: attr::ReprAttr) -> ~[@ty::VariantInfo] { let rty = ty::node_id_to_type(ccx.tcx, id); @@ -3203,9 +3233,20 @@ pub fn check_enum_variants(ccx: @mut CrateCtxt, None => () }; - // Check for duplicate discriminator values + // Check for duplicate discriminant values if disr_vals.contains(¤t_disr_val) { - ccx.tcx.sess.span_err(v.span, "discriminator value already exists"); + ccx.tcx.sess.span_err(v.span, "discriminant value already exists"); + } + // Check for unrepresentable discriminant values + match hint { + attr::ReprAny | attr::ReprExtern => (), + attr::ReprInt(sp, ity) => { + if !disr_in_range(ccx, ity, current_disr_val) { + ccx.tcx.sess.span_err(v.span, + "discriminant value outside specified type"); + ccx.tcx.sess.span_note(sp, "discriminant type specified here"); + } + } } disr_vals.push(current_disr_val); @@ -3219,8 +3260,13 @@ pub fn check_enum_variants(ccx: @mut CrateCtxt, } let rty = ty::node_id_to_type(ccx.tcx, id); + let hint = ty::lookup_repr_hint(ccx.tcx, ast::DefId { crate: ast::LOCAL_CRATE, node: id }); + if hint != attr::ReprAny && vs.len() <= 1 { + ccx.tcx.sess.span_err(sp, format!("unsupported representation for {}variant enum", + if vs.len() == 1 { "uni" } else { "zero-" })) + } - let variants = do_check(ccx, vs, id); + let variants = do_check(ccx, vs, id, hint); // cache so that ty::enum_variants won't repeat this work ccx.tcx.enum_var_cache.insert(local_def(id), @variants); diff --git a/src/test/compile-fail/tag-variant-disr-dup.rs b/src/test/compile-fail/tag-variant-disr-dup.rs index a5f85a685e6..d0608ec4c19 100644 --- a/src/test/compile-fail/tag-variant-disr-dup.rs +++ b/src/test/compile-fail/tag-variant-disr-dup.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//error-pattern:discriminator value already exists +//error-pattern:discriminant value already exists // black and white have the same discriminator value ... From 01097cbab00f64b922bcf5b91fb26a68f6d0d9cf Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sat, 31 Aug 2013 14:41:35 -0700 Subject: [PATCH 07/17] Unbreak the debuginfo tests. The variant used in debug-info/method-on-enum.rs had its layout changed by the smaller discriminant, so that the `u32` no longer overlaps both of the `u16`s, and thus the debugger is printing partially uninitialized data when it prints the wrong variant. Thus, the test runner is modified to accept wildcards (using a string that should be unlikely to occur literally), to allow for this. --- src/compiletest/runtest.rs | 27 +++++++++++++++++++++++++-- src/test/debug-info/method-on-enum.rs | 16 ++++++++-------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/compiletest/runtest.rs b/src/compiletest/runtest.rs index e7e1b110289..13c4c7948b8 100644 --- a/src/compiletest/runtest.rs +++ b/src/compiletest/runtest.rs @@ -281,7 +281,7 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) { }; let config = &mut config; let cmds = props.debugger_cmds.connect("\n"); - let check_lines = props.check_lines.clone(); + let check_lines = &props.check_lines; // compile test file (it shoud have 'compile-flags:-g' in the header) let mut ProcRes = compile_test(config, props, testfile); @@ -315,11 +315,34 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) { let num_check_lines = check_lines.len(); if num_check_lines > 0 { + // Allow check lines to leave parts unspecified (e.g., uninitialized + // bits in the wrong case of an enum) with the notation "[...]". + let check_fragments: ~[~[&str]] = check_lines.map(|s| s.split_str_iter("[...]").collect()); // check if each line in props.check_lines appears in the // output (in order) let mut i = 0u; for line in ProcRes.stdout.line_iter() { - if check_lines[i].trim() == line.trim() { + let mut rest = line.trim(); + let mut first = true; + let mut failed = false; + for &frag in check_fragments[i].iter() { + let found = if first { + if rest.starts_with(frag) { Some(0) } else { None } + } else { + rest.find_str(frag) + }; + match found { + None => { + failed = true; + break; + } + Some(i) => { + rest = rest.slice_from(i + frag.len()); + } + } + first = false; + } + if !failed && rest.len() == 0 { i += 1u; } if i == num_check_lines { diff --git a/src/test/debug-info/method-on-enum.rs b/src/test/debug-info/method-on-enum.rs index 2f02844fdc5..a6aacdf8e66 100644 --- a/src/test/debug-info/method-on-enum.rs +++ b/src/test/debug-info/method-on-enum.rs @@ -15,7 +15,7 @@ // STACK BY REF // debugger:finish // debugger:print *self -// check:$1 = {{Variant2, x = 1799, y = 1799}, {Variant2, 117901063}} +// check:$1 = {{Variant2, [...]}, {Variant2, 117901063}} // debugger:print arg1 // check:$2 = -1 // debugger:print arg2 @@ -25,7 +25,7 @@ // STACK BY VAL // debugger:finish // d ebugger:print self -- ignored for now because of issue #8512 -// c heck:$X = {{Variant2, x = 1799, y = 1799}, {Variant2, 117901063}} +// c heck:$X = {{Variant2, [...]}, {Variant2, 117901063}} // debugger:print arg1 // check:$4 = -3 // debugger:print arg2 @@ -35,7 +35,7 @@ // OWNED BY REF // debugger:finish // debugger:print *self -// check:$6 = {{Variant1, x = 1799, y = 1799}, {Variant1, 117901063}} +// check:$6 = {{Variant1, x = 1799, y = 1799}, {Variant1, [...]}} // debugger:print arg1 // check:$7 = -5 // debugger:print arg2 @@ -45,7 +45,7 @@ // OWNED BY VAL // debugger:finish // d ebugger:print self -- ignored for now because of issue #8512 -// c heck:$X = {{Variant1, x = 1799, y = 1799}, {Variant1, 117901063}} +// c heck:$X = {{Variant1, x = 1799, y = 1799}, {Variant1, [...]}} // debugger:print arg1 // check:$9 = -7 // debugger:print arg2 @@ -55,7 +55,7 @@ // OWNED MOVED // debugger:finish // debugger:print *self -// check:$11 = {{Variant1, x = 1799, y = 1799}, {Variant1, 117901063}} +// check:$11 = {{Variant1, x = 1799, y = 1799}, {Variant1, [...]}} // debugger:print arg1 // check:$12 = -9 // debugger:print arg2 @@ -65,7 +65,7 @@ // MANAGED BY REF // debugger:finish // debugger:print *self -// check:$14 = {{Variant2, x = 1799, y = 1799}, {Variant2, 117901063}} +// check:$14 = {{Variant2, [...]}, {Variant2, 117901063}} // debugger:print arg1 // check:$15 = -11 // debugger:print arg2 @@ -75,7 +75,7 @@ // MANAGED BY VAL // debugger:finish // d ebugger:print self -- ignored for now because of issue #8512 -// c heck:$X = {{Variant2, x = 1799, y = 1799}, {Variant2, 117901063}} +// c heck:$X = {{Variant2, [...]}, {Variant2, 117901063}} // debugger:print arg1 // check:$17 = -13 // debugger:print arg2 @@ -85,7 +85,7 @@ // MANAGED SELF // debugger:finish // debugger:print self->val -// check:$19 = {{Variant2, x = 1799, y = 1799}, {Variant2, 117901063}} +// check:$19 = {{Variant2, [...]}, {Variant2, 117901063}} // debugger:print arg1 // check:$20 = -15 // debugger:print arg2 From 727731f89ed5c341237705026f50f5f6a566336d Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Wed, 25 Sep 2013 09:41:10 -0700 Subject: [PATCH 08/17] Assorted cleanups suggested by reviewers. --- src/libextra/enum_set.rs | 3 ++- src/librustc/metadata/common.rs | 3 ++- src/librustc/middle/trans/adt.rs | 2 +- src/librustc/middle/trans/debuginfo.rs | 7 ++----- src/librustc/middle/ty.rs | 3 ++- src/libsyntax/attr.rs | 3 +-- src/test/run-pass/issue-2718.rs | 3 ++- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libextra/enum_set.rs b/src/libextra/enum_set.rs index e0b1e694a49..75611f88b63 100644 --- a/src/libextra/enum_set.rs +++ b/src/libextra/enum_set.rs @@ -139,7 +139,8 @@ mod test { use enum_set::*; - #[deriving(Eq)] #[repr(uint)] + #[deriving(Eq)] + #[repr(uint)] enum Foo { A, B, C } diff --git a/src/librustc/metadata/common.rs b/src/librustc/metadata/common.rs index 3692d571a53..6294b6cb6e3 100644 --- a/src/librustc/metadata/common.rs +++ b/src/librustc/metadata/common.rs @@ -111,7 +111,8 @@ pub static tag_items_data_item_reexport_def_id: uint = 0x4e; pub static tag_items_data_item_reexport_name: uint = 0x4f; // used to encode crate_ctxt side tables -#[deriving(Eq)] #[repr(uint)] +#[deriving(Eq)] +#[repr(uint)] pub enum astencode_tag { // Reserves 0x50 -- 0x6f tag_ast = 0x50, diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 687af6e6c8c..6e77a0cf720 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -359,7 +359,7 @@ fn bounds_usable(cx: &mut CrateContext, ity: IntType, bounds: &IntBounds) -> boo } } -fn ty_of_inttype(ity: IntType) -> ty::t { +pub fn ty_of_inttype(ity: IntType) -> ty::t { match ity { attr::SignedInt(t) => ty::mk_mach_int(t), attr::UnsignedInt(t) => ty::mk_mach_uint(t) diff --git a/src/librustc/middle/trans/debuginfo.rs b/src/librustc/middle/trans/debuginfo.rs index 616af475609..3c112b6fbf1 100644 --- a/src/librustc/middle/trans/debuginfo.rs +++ b/src/librustc/middle/trans/debuginfo.rs @@ -109,7 +109,6 @@ use std::libc::{c_uint, c_ulonglong, c_longlong}; use std::ptr; use std::unstable::atomics; use std::vec; -use syntax::attr; use syntax::codemap::{Span, Pos}; use syntax::{ast, codemap, ast_util, ast_map, opt_vec}; use syntax::parse::token; @@ -1422,10 +1421,8 @@ fn prepare_enum_metadata(cx: &mut CrateContext, let discriminant_type_metadata = |inttype| { let discriminant_llvm_type = adt::ll_inttype(cx, inttype); let (discriminant_size, discriminant_align) = size_and_align_of(cx, discriminant_llvm_type); - let discriminant_base_type_metadata = type_metadata(cx, match inttype { - attr::SignedInt(t) => ty::mk_mach_int(t), - attr::UnsignedInt(t) => ty::mk_mach_uint(t) - }, codemap::dummy_sp()); + let discriminant_base_type_metadata = type_metadata(cx, adt::ty_of_inttype(inttype), + codemap::dummy_sp()); do enum_name.with_c_str |enum_name| { unsafe { llvm::LLVMDIBuilderCreateEnumerationType( diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 7d4fafb29f6..ad25213098b 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -715,7 +715,8 @@ pub struct ParamBounds { pub type BuiltinBounds = EnumSet; -#[deriving(Clone, Eq, IterBytes, ToStr)] #[repr(uint)] +#[deriving(Clone, Eq, IterBytes, ToStr)] +#[repr(uint)] pub enum BuiltinBound { BoundStatic, BoundSend, diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index 222ca61a791..7a5a326add4 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -385,8 +385,7 @@ pub fn find_repr_attr(diagnostic: @mut span_handler, attr: @ast::MetaItem, acc: for item in items.iter() { match item.node { ast::MetaWord(word) => { - let word: &str = word; - let hint = match word { + let hint = match word.as_slice() { // Can't use "extern" because it's not a lexical identifier. "C" => ReprExtern, _ => match int_type_of_word(word) { diff --git a/src/test/run-pass/issue-2718.rs b/src/test/run-pass/issue-2718.rs index c2c8213517c..3bedbfa27b7 100644 --- a/src/test/run-pass/issue-2718.rs +++ b/src/test/run-pass/issue-2718.rs @@ -26,7 +26,8 @@ pub mod pipes { payload: Option } - #[deriving(Eq)] #[repr(int)] + #[deriving(Eq)] + #[repr(int)] pub enum state { empty, full, From 92109b12029b21e27f9f12a2b4faf4abc0627b64 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 3 Oct 2013 13:58:01 -0700 Subject: [PATCH 09/17] Yet more neatening --- src/librustc/middle/trans/adt.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 6e77a0cf720..c3bda97e910 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -326,14 +326,12 @@ fn range_to_inttype(cx: &mut CrateContext, hint: Hint, bounds: &IntBounds) -> In attempts = choose_shortest; } } - let mut best = attr::UnsignedInt(ast::ty_u64); for &ity in attempts.iter() { if bounds_usable(cx, ity, bounds) { - best = ity; - break; + return ity; } } - return best; + return attr::UnsignedInt(ast::ty_u64); } pub fn ll_inttype(cx: &mut CrateContext, ity: IntType) -> Type { From fcfbfde0b71d2470cb544d3d216eaf61ceb348bf Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 29 Sep 2013 02:20:11 -0700 Subject: [PATCH 10/17] Adjust reflection for the possibility of discriminants larger than int. Not only can discriminants be smaller than int now, but they can be larger than int on 32-bit targets. This has obvious implications for the reflection interface. Without this change, things fail with LLVM assertions when we try to "extend" i64 to i32. --- src/librustc/middle/trans/common.rs | 4 ++++ src/librustc/middle/trans/reflect.rs | 8 ++++---- src/libstd/reflect.rs | 10 +++++----- src/libstd/repr.rs | 12 ++++++------ src/libstd/unstable/intrinsics.rs | 13 +++++++++---- src/test/run-pass/enum-discrim-width-stuff.rs | 9 ++------- src/test/run-pass/reflect-visit-data.rs | 18 +++++++++--------- src/test/run-pass/reflect-visit-type.rs | 10 +++++----- 8 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 560cfa4be6b..c002584a7ff 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -868,6 +868,10 @@ pub fn C_i64(i: i64) -> ValueRef { return C_integral(Type::i64(), i as u64, true); } +pub fn C_u64(i: u64) -> ValueRef { + return C_integral(Type::i64(), i, false); +} + pub fn C_int(cx: &CrateContext, i: int) -> ValueRef { return C_integral(cx.int_type, i as u64, true); } diff --git a/src/librustc/middle/trans/reflect.rs b/src/librustc/middle/trans/reflect.rs index 1b27e06dca8..7e875243fd0 100644 --- a/src/librustc/middle/trans/reflect.rs +++ b/src/librustc/middle/trans/reflect.rs @@ -292,11 +292,11 @@ impl Reflector { sub_path, "get_disr"); - let llfdecl = decl_internal_rust_fn(ccx, [opaqueptrty], ty::mk_int(), sym); + let llfdecl = decl_internal_rust_fn(ccx, [opaqueptrty], ty::mk_u64(), sym); let fcx = new_fn_ctxt(ccx, ~[], llfdecl, - ty::mk_uint(), + ty::mk_u64(), None); let arg = unsafe { // @@ -308,7 +308,7 @@ impl Reflector { }; let mut bcx = fcx.entry_bcx.unwrap(); let arg = BitCast(bcx, arg, llptrty); - let ret = adt::trans_get_discr(bcx, repr, arg, Some(ccx.int_type)); + let ret = adt::trans_get_discr(bcx, repr, arg, Some(Type::i64())); Store(bcx, ret, fcx.llretptr.unwrap()); match fcx.llreturn { Some(llreturn) => cleanup_and_Br(bcx, bcx, llreturn), @@ -324,7 +324,7 @@ impl Reflector { for (i, v) in variants.iter().enumerate() { let name = ccx.sess.str_of(v.name); let variant_args = ~[this.c_uint(i), - C_integral(self.bcx.ccx().int_type, v.disr_val, false), + C_u64(v.disr_val), this.c_uint(v.args.len()), this.c_slice(name)]; do this.bracketed("enum_variant", variant_args) |this| { diff --git a/src/libstd/reflect.rs b/src/libstd/reflect.rs index d63b14f982d..19fa9abc0da 100644 --- a/src/libstd/reflect.rs +++ b/src/libstd/reflect.rs @@ -16,7 +16,7 @@ Runtime type reflection #[allow(missing_doc)]; -use unstable::intrinsics::{Opaque, TyDesc, TyVisitor}; +use unstable::intrinsics::{Disr, Opaque, TyDesc, TyVisitor}; use libc::c_void; use mem; use unstable::raw; @@ -396,7 +396,7 @@ impl TyVisitor for MovePtrAdaptor { } fn visit_enter_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool { self.align(align); @@ -407,7 +407,7 @@ impl TyVisitor for MovePtrAdaptor { } fn visit_enter_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool { if ! self.inner.visit_enter_enum_variant(variant, disr_val, @@ -426,7 +426,7 @@ impl TyVisitor for MovePtrAdaptor { } fn visit_leave_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool { if ! self.inner.visit_leave_enum_variant(variant, disr_val, @@ -437,7 +437,7 @@ impl TyVisitor for MovePtrAdaptor { } fn visit_leave_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool { if ! self.inner.visit_leave_enum(n_variants, get_disr, sz, align) { return false; diff --git a/src/libstd/repr.rs b/src/libstd/repr.rs index d03621eb60d..dd68c57e37e 100644 --- a/src/libstd/repr.rs +++ b/src/libstd/repr.rs @@ -29,7 +29,7 @@ use reflect::{MovePtr, align}; use str::StrSlice; use to_str::ToStr; use vec::OwnedVector; -use unstable::intrinsics::{Opaque, TyDesc, TyVisitor, get_tydesc, visit_tydesc}; +use unstable::intrinsics::{Disr, Opaque, TyDesc, TyVisitor, get_tydesc, visit_tydesc}; use unstable::raw; /// Representations @@ -92,7 +92,7 @@ num_repr!(f64, "f64") // New implementation using reflect::MovePtr enum VariantState { - SearchingFor(int), + SearchingFor(Disr), Matched, AlreadyFound } @@ -473,7 +473,7 @@ impl<'self> TyVisitor for ReprVisitor<'self> { fn visit_enter_enum(&mut self, _n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { let disr = unsafe { @@ -484,7 +484,7 @@ impl<'self> TyVisitor for ReprVisitor<'self> { } fn visit_enter_enum_variant(&mut self, _variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool { let mut write = false; @@ -531,7 +531,7 @@ impl<'self> TyVisitor for ReprVisitor<'self> { } fn visit_leave_enum_variant(&mut self, _variant: uint, - _disr_val: int, + _disr_val: Disr, n_fields: uint, _name: &str) -> bool { match self.var_stk[self.var_stk.len() - 1] { @@ -547,7 +547,7 @@ impl<'self> TyVisitor for ReprVisitor<'self> { fn visit_leave_enum(&mut self, _n_variants: uint, - _get_disr: extern unsafe fn(ptr: *Opaque) -> int, + _get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { diff --git a/src/libstd/unstable/intrinsics.rs b/src/libstd/unstable/intrinsics.rs index 1900d9d5801..20563718a6c 100644 --- a/src/libstd/unstable/intrinsics.rs +++ b/src/libstd/unstable/intrinsics.rs @@ -75,6 +75,11 @@ pub struct TyDesc { #[cfg(not(test))] pub enum Opaque { } +#[cfg(stage0)] +pub type Disr = int; +#[cfg(not(stage0))] +pub type Disr = u64; + #[lang="ty_visitor"] #[cfg(not(test))] pub trait TyVisitor { @@ -140,19 +145,19 @@ pub trait TyVisitor { sz: uint, align: uint) -> bool; fn visit_enter_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool; fn visit_enter_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool; fn visit_enum_variant_field(&mut self, i: uint, offset: uint, inner: *TyDesc) -> bool; fn visit_leave_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool; fn visit_leave_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool; fn visit_enter_fn(&mut self, purity: uint, proto: uint, diff --git a/src/test/run-pass/enum-discrim-width-stuff.rs b/src/test/run-pass/enum-discrim-width-stuff.rs index 65f93b1c3c2..bc4cb87f06b 100644 --- a/src/test/run-pass/enum-discrim-width-stuff.rs +++ b/src/test/run-pass/enum-discrim-width-stuff.rs @@ -13,13 +13,8 @@ use std::mem; pub fn main() { enum E { V = 0x1717171717171717 } static C: E = V; - let expected: u64 = if mem::size_of::() < 8 { - 0x17171717 - } else { - 0x1717171717171717 - }; - assert_eq!(expected, V as u64); - assert_eq!(expected, C as u64); + assert_eq!(V as u64, 0x1717171717171717u64); + assert_eq!(C as u64, 0x1717171717171717u64); assert_eq!(format!("{:?}", V), ~"V"); assert_eq!(format!("{:?}", C), ~"V"); } diff --git a/src/test/run-pass/reflect-visit-data.rs b/src/test/run-pass/reflect-visit-data.rs index 9901f7493f7..de8d9470f10 100644 --- a/src/test/run-pass/reflect-visit-data.rs +++ b/src/test/run-pass/reflect-visit-data.rs @@ -15,7 +15,7 @@ use std::libc::c_void; use std::ptr; use std::mem; -use std::unstable::intrinsics::{TyDesc, get_tydesc, visit_tydesc, TyVisitor, Opaque}; +use std::unstable::intrinsics::{TyDesc, get_tydesc, visit_tydesc, TyVisitor, Disr, Opaque}; use std::unstable::raw::Vec; #[doc = "High-level interfaces to `std::unstable::intrinsics::visit_ty` reflection system."] @@ -380,7 +380,7 @@ impl TyVisitor for ptr_visit_adaptor { } fn visit_enter_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool { self.align(align); @@ -389,7 +389,7 @@ impl TyVisitor for ptr_visit_adaptor { } fn visit_enter_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool { if ! self.inner.visit_enter_enum_variant(variant, disr_val, @@ -405,7 +405,7 @@ impl TyVisitor for ptr_visit_adaptor { } fn visit_leave_enum_variant(&mut self, variant: uint, - disr_val: int, + disr_val: Disr, n_fields: uint, name: &str) -> bool { if ! self.inner.visit_leave_enum_variant(variant, disr_val, @@ -416,7 +416,7 @@ impl TyVisitor for ptr_visit_adaptor { } fn visit_leave_enum(&mut self, n_variants: uint, - get_disr: extern unsafe fn(ptr: *Opaque) -> int, + get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, sz: uint, align: uint) -> bool { if ! self.inner.visit_leave_enum(n_variants, get_disr, sz, align) { return false; } @@ -578,24 +578,24 @@ impl TyVisitor for my_visitor { _sz: uint, _align: uint) -> bool { true } fn visit_enter_enum(&mut self, _n_variants: uint, - _get_disr: extern unsafe fn(ptr: *Opaque) -> int, + _get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { // FIXME (#3732): this needs to rewind between enum variants, or something. true } fn visit_enter_enum_variant(&mut self, _variant: uint, - _disr_val: int, + _disr_val: Disr, _n_fields: uint, _name: &str) -> bool { true } fn visit_enum_variant_field(&mut self, _i: uint, _offset: uint, inner: *TyDesc) -> bool { self.visit_inner(inner) } fn visit_leave_enum_variant(&mut self, _variant: uint, - _disr_val: int, + _disr_val: Disr, _n_fields: uint, _name: &str) -> bool { true } fn visit_leave_enum(&mut self, _n_variants: uint, - _get_disr: extern unsafe fn(ptr: *Opaque) -> int, + _get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { true } fn visit_enter_fn(&mut self, _purity: uint, _proto: uint, diff --git a/src/test/run-pass/reflect-visit-type.rs b/src/test/run-pass/reflect-visit-type.rs index db68ee2b500..e77cb432c3a 100644 --- a/src/test/run-pass/reflect-visit-type.rs +++ b/src/test/run-pass/reflect-visit-type.rs @@ -10,7 +10,7 @@ #[feature(managed_boxes)]; -use std::unstable::intrinsics::{TyDesc, get_tydesc, visit_tydesc, TyVisitor, Opaque}; +use std::unstable::intrinsics::{TyDesc, get_tydesc, visit_tydesc, TyVisitor, Disr, Opaque}; struct MyVisitor { types: @mut ~[~str], @@ -114,22 +114,22 @@ impl TyVisitor for MyVisitor { _sz: uint, _align: uint) -> bool { true } fn visit_enter_enum(&mut self, _n_variants: uint, - _get_disr: extern unsafe fn(ptr: *Opaque) -> int, + _get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { true } fn visit_enter_enum_variant(&mut self, _variant: uint, - _disr_val: int, + _disr_val: Disr, _n_fields: uint, _name: &str) -> bool { true } fn visit_enum_variant_field(&mut self, _i: uint, _offset: uint, _inner: *TyDesc) -> bool { true } fn visit_leave_enum_variant(&mut self, _variant: uint, - _disr_val: int, + _disr_val: Disr, _n_fields: uint, _name: &str) -> bool { true } fn visit_leave_enum(&mut self, _n_variants: uint, - _get_disr: extern unsafe fn(ptr: *Opaque) -> int, + _get_disr: extern unsafe fn(ptr: *Opaque) -> Disr, _sz: uint, _align: uint) -> bool { true } fn visit_enter_fn(&mut self, _purity: uint, _proto: uint, From afab3307a03c3227706b634b731363004c33a0ac Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Thu, 3 Oct 2013 13:59:34 -0700 Subject: [PATCH 11/17] C-like enums are not always immediate --- src/librustc/middle/trans/expr.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index e1de6615206..9a019f98a49 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1726,9 +1726,14 @@ fn trans_imm_cast(bcx: @mut Block, expr: &ast::Expr, (cast_enum, cast_float) => { let bcx = bcx; let repr = adt::represent_type(ccx, t_in); - let slot = Alloca(bcx, ll_t_in, ""); - Store(bcx, llexpr, slot); - let lldiscrim_a = adt::trans_get_discr(bcx, repr, slot, Some(Type::i64())); + let llexpr_ptr; + if type_is_immediate(ccx, t_in) { + llexpr_ptr = Alloca(bcx, ll_t_in, ""); + Store(bcx, llexpr, llexpr_ptr); + } else { + llexpr_ptr = llexpr; + } + let lldiscrim_a = adt::trans_get_discr(bcx, repr, llexpr_ptr, Some(Type::i64())); match k_out { cast_integral => int_cast(bcx, ll_t_out, val_ty(lldiscrim_a), From de9bb9738d8fcd95222fc0c9383953881e31d4d5 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 29 Sep 2013 14:00:34 -0700 Subject: [PATCH 12/17] Add tests for enum discriminant sizing. --- .../compile-fail/enum-discrim-too-small.rs | 58 +++++++++++++ src/test/compile-fail/lint-ctypes-enum.rs | 25 ++++++ src/test/run-pass/enum-discrim-autosizing.rs | 62 ++++++++++++++ .../run-pass/enum-discrim-manual-sizing.rs | 84 +++++++++++++++++++ src/test/run-pass/enum-discrim-width-stuff.rs | 49 +++++++++-- 5 files changed, 271 insertions(+), 7 deletions(-) create mode 100644 src/test/compile-fail/enum-discrim-too-small.rs create mode 100644 src/test/compile-fail/lint-ctypes-enum.rs create mode 100644 src/test/run-pass/enum-discrim-autosizing.rs create mode 100644 src/test/run-pass/enum-discrim-manual-sizing.rs diff --git a/src/test/compile-fail/enum-discrim-too-small.rs b/src/test/compile-fail/enum-discrim-too-small.rs new file mode 100644 index 00000000000..2de50ad1d1d --- /dev/null +++ b/src/test/compile-fail/enum-discrim-too-small.rs @@ -0,0 +1,58 @@ +// Copyright 2013 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. + +#[repr(u8)] //~ NOTE discriminant type specified here +enum Eu8 { + Au8 = 23, + Bu8 = 223, + Cu8 = -23, //~ ERROR discriminant value outside specified type +} + +#[repr(i8)] //~ NOTE discriminant type specified here +enum Ei8 { + Ai8 = 23, + Bi8 = -23, + Ci8 = 223, //~ ERROR discriminant value outside specified type +} + +#[repr(u16)] //~ NOTE discriminant type specified here +enum Eu16 { + Au16 = 23, + Bu16 = 55555, + Cu16 = -22333, //~ ERROR discriminant value outside specified type +} + +#[repr(i16)] //~ NOTE discriminant type specified here +enum Ei16 { + Ai16 = 23, + Bi16 = -22333, + Ci16 = 55555, //~ ERROR discriminant value outside specified type +} + +#[repr(u32)] //~ NOTE discriminant type specified here +enum Eu32 { + Au32 = 23, + Bu32 = 3_000_000_000, + Cu32 = -2_000_000_000, //~ ERROR discriminant value outside specified type +} + +#[repr(i32)] //~ NOTE discriminant type specified here +enum Ei32 { + Ai32 = 23, + Bi32 = -2_000_000_000, + Ci32 = 3_000_000_000, //~ ERROR discriminant value outside specified type +} + +// u64 currently allows negative numbers, and i64 allows numbers greater than `1<<63`. This is a +// little counterintuitive, but since the discriminant can store all the bits, and extracting it +// with a cast requires specifying the signedness, there is no loss of information in those cases. +// This also applies to int and uint on 64-bit targets. + +pub fn main() { } diff --git a/src/test/compile-fail/lint-ctypes-enum.rs b/src/test/compile-fail/lint-ctypes-enum.rs new file mode 100644 index 00000000000..857e3bb4b8d --- /dev/null +++ b/src/test/compile-fail/lint-ctypes-enum.rs @@ -0,0 +1,25 @@ +// Copyright 2013 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. + +#[deny(ctypes)]; + +enum Z { } +enum U { A } +enum B { C, D } +enum T { E, F, G } + +extern { + fn zf(x: Z); + fn uf(x: U); + fn bf(x: B); //~ ERROR found enum type without foreign-function-safe + fn tf(x: T); //~ ERROR found enum type without foreign-function-safe +} + +pub fn main() { } diff --git a/src/test/run-pass/enum-discrim-autosizing.rs b/src/test/run-pass/enum-discrim-autosizing.rs new file mode 100644 index 00000000000..ef34115739d --- /dev/null +++ b/src/test/run-pass/enum-discrim-autosizing.rs @@ -0,0 +1,62 @@ +// Copyright 2013 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. + +use std::mem::size_of; + +enum Ei8 { + Ai8 = -1, + Bi8 = 0 +} + +enum Eu8 { + Au8 = 0, + Bu8 = 0x80 +} + +enum Ei16 { + Ai16 = -1, + Bi16 = 0x80 +} + +enum Eu16 { + Au16 = 0, + Bu16 = 0x8000 +} + +enum Ei32 { + Ai32 = -1, + Bi32 = 0x8000 +} + +enum Eu32 { + Au32 = 0, + Bu32 = 0x8000_0000 +} + +enum Ei64 { + Ai64 = -1, + Bi64 = 0x8000_0000 +} + +enum Eu64 { + Au64 = 0, + Bu64 = 0x8000_0000_0000_0000 +} + +pub fn main() { + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 4); + assert_eq!(size_of::(), 4); + assert_eq!(size_of::(), 8); + assert_eq!(size_of::(), 8); +} diff --git a/src/test/run-pass/enum-discrim-manual-sizing.rs b/src/test/run-pass/enum-discrim-manual-sizing.rs new file mode 100644 index 00000000000..16eaac082ab --- /dev/null +++ b/src/test/run-pass/enum-discrim-manual-sizing.rs @@ -0,0 +1,84 @@ +// Copyright 2013 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. + +use std::mem::size_of; + +#[repr(i8)] +enum Ei8 { + Ai8 = 0, + Bi8 = 1 +} + +#[repr(u8)] +enum Eu8 { + Au8 = 0, + Bu8 = 1 +} + +#[repr(i16)] +enum Ei16 { + Ai16 = 0, + Bi16 = 1 +} + +#[repr(u16)] +enum Eu16 { + Au16 = 0, + Bu16 = 1 +} + +#[repr(i32)] +enum Ei32 { + Ai32 = 0, + Bi32 = 1 +} + +#[repr(u32)] +enum Eu32 { + Au32 = 0, + Bu32 = 1 +} + +#[repr(i64)] +enum Ei64 { + Ai64 = 0, + Bi64 = 1 +} + +#[repr(u64)] +enum Eu64 { + Au64 = 0, + Bu64 = 1 +} + +#[repr(int)] +enum Eint { + Aint = 0, + Bint = 1 +} + +#[repr(uint)] +enum Euint { + Auint = 0, + Buint = 1 +} + +pub fn main() { + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), 1); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 2); + assert_eq!(size_of::(), 4); + assert_eq!(size_of::(), 4); + assert_eq!(size_of::(), 8); + assert_eq!(size_of::(), 8); + assert_eq!(size_of::(), size_of::()); + assert_eq!(size_of::(), size_of::()); +} diff --git a/src/test/run-pass/enum-discrim-width-stuff.rs b/src/test/run-pass/enum-discrim-width-stuff.rs index bc4cb87f06b..61efb2c8a42 100644 --- a/src/test/run-pass/enum-discrim-width-stuff.rs +++ b/src/test/run-pass/enum-discrim-width-stuff.rs @@ -8,13 +8,48 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::mem; +#[feature(macro_rules)]; + +macro_rules! check { + ($m:ident, $t:ty, $v:expr) => {{ + mod $m { + use std::mem::size_of; + enum E { + V = $v, + A = 0 + } + static C: E = V; + pub fn check() { + assert_eq!(size_of::(), size_of::<$t>()); + assert_eq!(V as $t, $v); + assert_eq!(C as $t, $v); + assert_eq!(format!("{:?}", V), ~"V"); + assert_eq!(format!("{:?}", C), ~"V"); + } + } + $m::check(); + }} +} pub fn main() { - enum E { V = 0x1717171717171717 } - static C: E = V; - assert_eq!(V as u64, 0x1717171717171717u64); - assert_eq!(C as u64, 0x1717171717171717u64); - assert_eq!(format!("{:?}", V), ~"V"); - assert_eq!(format!("{:?}", C), ~"V"); + check!(a, u8, 0x17); + check!(b, u8, 0xe8); + check!(c, u16, 0x1727); + check!(d, u16, 0xe8d8); + check!(e, u32, 0x17273747); + check!(f, u32, 0xe8d8c8b8); + check!(g, u64, 0x1727374757677787u64); + check!(h, u64, 0xe8d8c8b8a8988878u64); + + check!(z, i8, 0x17); + check!(y, i8, -0x17); + check!(x, i16, 0x1727); + check!(w, i16, -0x1727); + check!(v, i32, 0x17273747); + check!(u, i32, -0x17273747); + check!(t, i64, 0x1727374757677787); + check!(s, i64, -0x1727374757677787); + + enum Simple { A, B } + assert_eq!(std::mem::size_of::(), 1); } From 472d798dc1c0a7804b54c67b485e134fe097a57e Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 20 Oct 2013 11:13:41 -0700 Subject: [PATCH 13/17] Work around const_eval issues by changing signed integer `min_value`s. Otherwise, run-pass/deriving-primitive.rs breaks on 32-bit platforms, because `int::min_value` is `0xffffffff7fffffff` when evaluated for the discriminant declaration. --- src/libstd/num/int_macros.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstd/num/int_macros.rs b/src/libstd/num/int_macros.rs index 694e5e7f6bf..ba1a7a4f912 100644 --- a/src/libstd/num/int_macros.rs +++ b/src/libstd/num/int_macros.rs @@ -29,7 +29,8 @@ pub static bits : uint = $bits; pub static bytes : uint = ($bits / 8); pub static min_value: $T = (-1 as $T) << (bits - 1); -pub static max_value: $T = min_value - 1 as $T; +// FIXME(#9837): Compute min_value like this so the high bits that shouldn't exist are 0. +pub static max_value: $T = !min_value; impl CheckedDiv for $T { #[inline] From ac4644d7de7ec1528061dd4bb9639308c6018ee6 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Fri, 25 Oct 2013 21:22:00 -0700 Subject: [PATCH 14/17] Add another discriminant-size-related test, this time with fields. --- src/test/run-pass/small-enums-with-fields.rs | 41 ++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 src/test/run-pass/small-enums-with-fields.rs diff --git a/src/test/run-pass/small-enums-with-fields.rs b/src/test/run-pass/small-enums-with-fields.rs new file mode 100644 index 00000000000..c4f4b5e4e37 --- /dev/null +++ b/src/test/run-pass/small-enums-with-fields.rs @@ -0,0 +1,41 @@ +// Copyright 2013 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(macro_rules)]; + +use std::mem::size_of; + +macro_rules! check { + ($t:ty, $sz:expr, $($e:expr, $s:expr),*) => {{ + assert_eq!(size_of::<$t>(), $sz); + $({ + static S: $t = $e; + let v: $t = $e; + assert_eq!(S, v); + assert_eq!(format!("{:?}", v), ~$s); + assert_eq!(format!("{:?}", S), ~$s); + });* + }} +} + +pub fn main() { + check!(Option, 2, + None, "None", + Some(129u8), "Some(129u8)"); + check!(Option, 4, + None, "None", + Some(-20000i16), "Some(-20000i16)"); + check!(Either, 2, + Left(132u8), "Left(132u8)", + Right(-32i8), "Right(-32i8)"); + check!(Either, 4, + Left(132u8), "Left(132u8)", + Right(-20000i16), "Right(-20000i16)"); +} From 49f851c2c9e57465317938a0486495ef02ef5d81 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sat, 26 Oct 2013 12:12:53 -0700 Subject: [PATCH 15/17] Fix type_of for enums to not lose data in some cases when immediate. The previous implementation, when combined with small discriminants and immediate types, caused problems for types like `Either` which are now small enough to be immediate and can have fields intersecting the highest-alignment variant's alignment padding (which LLVM doesn't preserve). So let's not do that. --- src/librustc/middle/trans/adt.rs | 56 ++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index c3bda97e910..997bcbca9ce 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -382,30 +382,38 @@ fn generic_fields_of(cx: &mut CrateContext, r: &Repr, sizing: bool) -> ~[Type] { CEnum(ity, _, _) => ~[ll_inttype(cx, ity)], Univariant(ref st, _dtor) => struct_llfields(cx, st, sizing), NullablePointer{ nonnull: ref st, _ } => struct_llfields(cx, st, sizing), - General(_ity, ref sts) => { - // To get "the" type of a general enum, we pick the case - // with the largest alignment (so it will always align - // correctly in containing structures) and pad it out. - assert!(sts.len() >= 1); - let mut most_aligned = None; - let mut largest_align = 0; - let mut largest_size = 0; - for st in sts.iter() { - if largest_size < st.size { - largest_size = st.size; - } - if largest_align < st.align { - // Clang breaks ties by size; it is unclear if - // that accomplishes anything important. - largest_align = st.align; - most_aligned = Some(st); - } - } - let most_aligned = most_aligned.unwrap(); - let padding = largest_size - most_aligned.size; - - struct_llfields(cx, most_aligned, sizing) - + &[Type::array(&Type::i8(), padding)] + General(ity, ref sts) => { + // We need a representation that has: + // * The alignment of the most-aligned field + // * The size of the largest variant (rounded up to that alignment) + // * No alignment padding anywhere any variant has actual data + // (currently matters only for enums small enough to be immediate) + // * The discriminant in an obvious place. + // + // So we start with the discriminant, pad it up to the alignment with + // more of its own type, then use alignment-sized ints to get the rest + // of the size. + // + // Note: if/when we start exposing SIMD vector types (or f80, on some + // platforms that have it), this will need some adjustment. + let size = sts.iter().map(|st| st.size).max().unwrap(); + let most_aligned = sts.iter().max_by(|st| st.align).unwrap(); + let align = most_aligned.align; + let discr_ty = ll_inttype(cx, ity); + let discr_size = machine::llsize_of_alloc(cx, discr_ty) as u64; + let pad_ty = match align { + 1 => Type::i8(), + 2 => Type::i16(), + 4 => Type::i32(), + 8 if machine::llalign_of_min(cx, Type::i64()) == 8 => Type::i64(), + _ => fail!("Unsupported enum alignment: {:?}", align) + }; + assert_eq!(machine::llalign_of_min(cx, pad_ty) as u64, align); + let align_units = (size + align - 1) / align; + assert_eq!(align % discr_size, 0); + ~[discr_ty, + Type::array(&discr_ty, align / discr_size - 1), + Type::array(&pad_ty, align_units - 1)] } } } From c0190a9cfb8783332845ff69bf12663f7b3057d5 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Tue, 29 Oct 2013 09:07:30 -0700 Subject: [PATCH 16/17] Prevent unoptimized rustpkg tests from running out of stack. The actual fix would be to make rustpkg use `rustc::monitor` so it picks up anything special that rustc needs, but for now let's keep the tests from breaking. --- mk/tests.mk | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mk/tests.mk b/mk/tests.mk index d6fc5ecb8e5..17084699ef3 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -455,6 +455,17 @@ $(foreach host,$(CFG_HOST_TRIPLES), \ $(eval $(call DEF_TEST_CRATE_RULES,$(stage),$(target),$(host),$(crate))) \ )))))) +# FIXME (#10104): Raise the stack size to work around rustpkg bypassing +# the code in rustc that would take care of it. +define DEF_RUSTPKG_STACK_FIX +$$(call TEST_OK_FILE,$(1),$(2),$(3),rustpkg): export RUST_MIN_STACK=8000000 +endef + +$(foreach host,$(CFG_HOST_TRIPLES), \ + $(foreach target,$(CFG_TARGET_TRIPLES), \ + $(foreach stage,$(STAGES), \ + $(eval $(call DEF_RUSTPKG_STACK_FIX,$(stage),$(target),$(host)))))) + ###################################################################### # Rules for the compiletest tests (rpass, rfail, etc.) From 86a710e4545a383513c1247e48421142f3741984 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Wed, 30 Oct 2013 00:13:17 -0700 Subject: [PATCH 17/17] Fix check-fast breakage in new enum test. --- src/test/run-pass/enum-discrim-width-stuff.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-pass/enum-discrim-width-stuff.rs b/src/test/run-pass/enum-discrim-width-stuff.rs index 61efb2c8a42..6a0110436b3 100644 --- a/src/test/run-pass/enum-discrim-width-stuff.rs +++ b/src/test/run-pass/enum-discrim-width-stuff.rs @@ -51,5 +51,5 @@ pub fn main() { check!(s, i64, -0x1727374757677787); enum Simple { A, B } - assert_eq!(std::mem::size_of::(), 1); + assert_eq!(::std::mem::size_of::(), 1); }