Skip to content

Allow enum discriminants to not be uint, and use smallest possible size by default. #9613

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Oct 30, 2013
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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.
  • Loading branch information
jld committed Oct 29, 2013
commit a027f164bcda6f99d0b44f79ca9f676ecc12a50a
24 changes: 13 additions & 11 deletions src/librustc/middle/trans/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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.
Expand All @@ -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) {
Expand All @@ -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 };
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
54 changes: 50 additions & 4 deletions src/librustc/middle/typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(&current_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);

Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/tag-variant-disr-dup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ...

Expand Down