Skip to content

Use 64-bit integers to represent discriminants #8504

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions src/librustc/metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use middle::astencode::vtable_decoder_helpers;


use std::hash::HashUtil;
use std::uint;
use std::u64;
use std::io::WriterUtil;
use std::io;
use std::option;
Expand Down Expand Up @@ -207,9 +207,9 @@ fn each_reexport(d: ebml::Doc, f: &fn(ebml::Doc) -> bool) -> bool {
reader::tagged_docs(d, tag_items_data_item_reexport, f)
}

fn variant_disr_val(d: ebml::Doc) -> Option<uint> {
fn variant_disr_val(d: ebml::Doc) -> Option<ty::Disr> {
do reader::maybe_get_doc(d, tag_disr_val).chain |val_doc| {
do reader::with_doc_data(val_doc) |data| { uint::parse_bytes(data, 10u) }
do reader::with_doc_data(val_doc) |data| { u64::parse_bytes(data, 10u) }
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,9 @@ fn encode_discriminant(ecx: &EncodeContext,

fn encode_disr_val(_: &EncodeContext,
ebml_w: &mut writer::Encoder,
disr_val: uint) {
disr_val: ty::Disr) {
ebml_w.start_tag(tag_disr_val);
let s = uint::to_str(disr_val);
let s = disr_val.to_str();
ebml_w.writer.write(s.as_bytes());
ebml_w.end_tag();
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/trans/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ pub enum VecLenOpt {
// range)
enum Opt {
lit(Lit),
var(/* disr val */ uint, @adt::Repr),
var(ty::Disr, @adt::Repr),
range(@ast::expr, @ast::expr),
vec_len(/* length */ uint, VecLenOpt, /*range of matches*/(uint, uint))
}
Expand Down Expand Up @@ -987,7 +987,7 @@ struct ExtractedBlock {

fn extract_variant_args(bcx: @mut Block,
repr: &adt::Repr,
disr_val: uint,
disr_val: ty::Disr,
val: ValueRef)
-> ExtractedBlock {
let _icx = push_ctxt("match::extract_variant_args");
Expand Down
55 changes: 30 additions & 25 deletions src/librustc/middle/trans/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use middle::trans::common::*;
use middle::trans::machine;
use middle::trans::type_of;
use middle::ty;
use middle::ty::Disr;
use syntax::ast;
use util::ppaux::ty_to_str;

Expand All @@ -64,7 +65,7 @@ use middle::trans::type_::Type;
/// Representations.
pub enum Repr {
/// C-like enums; basically an int.
CEnum(uint, uint), // discriminant range
CEnum(Disr, Disr), // discriminant range
/**
* Single-case variants, and structs/tuples/records.
*
Expand All @@ -89,7 +90,7 @@ pub enum Repr {
* is represented such that `None` is a null pointer and `Some` is the
* identity function.
*/
NullablePointer{ nonnull: Struct, nndiscr: uint, ptrfield: uint,
NullablePointer{ nonnull: Struct, nndiscr: Disr, ptrfield: uint,
nullfields: ~[ty::t] }
}

Expand Down Expand Up @@ -140,7 +141,7 @@ 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: uint, tys: ~[ty::t] };
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
Expand Down Expand Up @@ -177,7 +178,7 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr {
// Since there's at least one
// non-empty body, explicit discriminants should have
// been rejected by a checker before this point.
if !cases.iter().enumerate().all(|(i,c)| c.discr == i) {
if !cases.iter().enumerate().all(|(i,c)| c.discr == (i as Disr)) {
cx.sess.bug(fmt!("non-C-like enum %s with specified \
discriminants",
ty::item_path_str(cx.tcx, def_id)))
Expand Down Expand Up @@ -305,16 +306,16 @@ pub fn trans_get_discr(bcx: @mut Block, r: &Repr, scrutinee: ValueRef)
-> ValueRef {
match *r {
CEnum(min, max) => load_discr(bcx, scrutinee, min, max),
Univariant(*) => C_uint(bcx.ccx(), 0),
General(ref cases) => load_discr(bcx, scrutinee, 0, cases.len() - 1),
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()))
}
}
}

fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: uint, ptrfield: uint,
fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: Disr, ptrfield: uint,
scrutinee: ValueRef) -> ValueRef {
let cmp = if nndiscr == 0 { IntEQ } else { IntNE };
let llptr = Load(bcx, GEPi(bcx, scrutinee, [0, ptrfield]));
Expand All @@ -323,10 +324,10 @@ fn nullable_bitdiscr(bcx: @mut Block, nonnull: &Struct, nndiscr: uint, ptrfield:
}

/// Helper for cases where the discriminant is simply loaded.
fn load_discr(bcx: @mut Block, scrutinee: ValueRef, min: uint, max: uint)
fn load_discr(bcx: @mut Block, scrutinee: ValueRef, min: Disr, max: Disr)
-> ValueRef {
let ptr = GEPi(bcx, scrutinee, [0, 0]);
if max + 1 == min {
if max + 1 == min { // FIXME: this isn't right anymore
// 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
Expand All @@ -347,16 +348,16 @@ fn load_discr(bcx: @mut Block, scrutinee: ValueRef, min: uint, max: uint)
*
* This should ideally be less tightly tied to `_match`.
*/
pub fn trans_case(bcx: @mut Block, r: &Repr, discr: uint) -> _match::opt_result {
pub fn trans_case(bcx: @mut Block, r: &Repr, discr: Disr) -> _match::opt_result {
match *r {
CEnum(*) => {
_match::single_result(rslt(bcx, C_uint(bcx.ccx(), discr)))
_match::single_result(rslt(bcx, C_disr(bcx.ccx(), discr)))
}
Univariant(*) => {
bcx.ccx().sess.bug("no cases for univariants or structs")
}
General(*) => {
_match::single_result(rslt(bcx, C_uint(bcx.ccx(), discr)))
_match::single_result(rslt(bcx, C_disr(bcx.ccx(), discr)))
}
NullablePointer{ _ } => {
assert!(discr == 0 || discr == 1);
Expand All @@ -370,11 +371,11 @@ pub fn trans_case(bcx: @mut Block, r: &Repr, discr: uint) -> _match::opt_result
* representation. The fields, if any, should then be initialized via
* `trans_field_ptr`.
*/
pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: uint) {
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_uint(bcx.ccx(), discr), GEPi(bcx, val, [0, 0]))
Store(bcx, C_disr(bcx.ccx(), discr), GEPi(bcx, val, [0, 0]))
}
Univariant(ref st, true) => {
assert_eq!(discr, 0);
Expand All @@ -385,7 +386,7 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: uint) {
assert_eq!(discr, 0);
}
General(*) => {
Store(bcx, C_uint(bcx.ccx(), discr), GEPi(bcx, val, [0, 0]))
Store(bcx, C_disr(bcx.ccx(), discr), GEPi(bcx, val, [0, 0]))
}
NullablePointer{ nonnull: ref nonnull, nndiscr, ptrfield, _ } => {
if discr != nndiscr {
Expand All @@ -401,7 +402,7 @@ pub fn trans_start_init(bcx: @mut Block, r: &Repr, val: ValueRef, discr: uint) {
* The number of fields in a given case; for use when obtaining this
* information from the type or definition is less convenient.
*/
pub fn num_args(r: &Repr, discr: uint) -> uint {
pub fn num_args(r: &Repr, discr: Disr) -> uint {
match *r {
CEnum(*) => 0,
Univariant(ref st, dtor) => {
Expand All @@ -416,7 +417,7 @@ pub fn num_args(r: &Repr, discr: uint) -> uint {
}

/// Access a field, at a point when the value's case is known.
pub fn trans_field_ptr(bcx: @mut Block, r: &Repr, val: ValueRef, discr: uint,
pub fn trans_field_ptr(bcx: @mut Block, r: &Repr, val: ValueRef, discr: Disr,
ix: uint) -> ValueRef {
// Note: if this ever needs to generate conditionals (e.g., if we
// decide to do some kind of cdr-coding-like non-unique repr
Expand Down Expand Up @@ -494,13 +495,13 @@ pub fn trans_drop_flag_ptr(bcx: @mut Block, r: &Repr, val: ValueRef) -> ValueRef
* this could be changed in the future to avoid allocating unnecessary
* space after values of shorter-than-maximum cases.
*/
pub fn trans_const(ccx: &mut CrateContext, r: &Repr, discr: uint,
pub fn trans_const(ccx: &mut CrateContext, r: &Repr, discr: Disr,
vals: &[ValueRef]) -> ValueRef {
match *r {
CEnum(min, max) => {
assert_eq!(vals.len(), 0);
assert!(min <= discr && discr <= max);
C_uint(ccx, discr)
C_disr(ccx, discr)
}
Univariant(ref st, _dro) => {
assert_eq!(discr, 0);
Expand All @@ -509,7 +510,7 @@ pub fn trans_const(ccx: &mut CrateContext, r: &Repr, discr: uint,
General(ref cases) => {
let case = &cases[discr];
let max_sz = cases.iter().map(|x| x.size).max().unwrap();
let discr_ty = C_uint(ccx, discr);
let discr_ty = C_disr(ccx, discr);
let contents = build_const_struct(ccx, case,
~[discr_ty] + vals);
C_struct(contents + &[padding(max_sz - case.size)])
Expand Down Expand Up @@ -581,15 +582,15 @@ fn roundup(x: u64, a: u64) -> u64 { ((x + (a - 1)) / a) * a }

/// Get the discriminant of a constant value. (Not currently used.)
pub fn const_get_discrim(ccx: &mut CrateContext, r: &Repr, val: ValueRef)
-> uint {
-> Disr {
match *r {
CEnum(*) => const_to_uint(val) as uint,
CEnum(*) => const_to_uint(val) as Disr,
Univariant(*) => 0,
General(*) => const_to_uint(const_get_elt(ccx, val, [0])) as uint,
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 */
(1 - nndiscr) as uint
(1 - nndiscr) as Disr
} else {
nndiscr
}
Expand All @@ -605,7 +606,7 @@ pub fn const_get_discrim(ccx: &mut CrateContext, r: &Repr, val: ValueRef)
* raw LLVM-level structs and arrays.)
*/
pub fn const_get_field(ccx: &mut CrateContext, r: &Repr, val: ValueRef,
_discr: uint, ix: uint) -> ValueRef {
_discr: Disr, ix: uint) -> ValueRef {
match *r {
CEnum(*) => ccx.sess.bug("element access in C-like enum const"),
Univariant(*) => const_struct_field(ccx, val, ix),
Expand Down Expand Up @@ -644,3 +645,7 @@ pub fn is_newtypeish(r: &Repr) -> bool {
_ => false
}
}

fn C_disr(cx: &CrateContext, i: Disr) -> ValueRef {
return C_integral(cx.int_type, i, false);
}
8 changes: 4 additions & 4 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ pub fn iter_structural_ty(cx: @mut Block, av: ValueRef, t: ty::t,
for variant in (*variants).iter() {
let variant_cx =
sub_block(cx, ~"enum-iter-variant-" +
uint::to_str(variant.disr_val));
variant.disr_val.to_str());
let variant_cx =
iter_variant(variant_cx, repr, av, *variant,
substs.tps, |x,y,z| f(x,y,z));
Expand Down Expand Up @@ -1972,7 +1972,7 @@ pub fn trans_enum_variant(ccx: @mut CrateContext,
_enum_id: ast::NodeId,
variant: &ast::variant,
args: &[ast::variant_arg],
disr: uint,
disr: ty::Disr,
param_substs: Option<@param_substs>,
llfndecl: ValueRef) {
let _icx = push_ctxt("trans_enum_variant");
Expand Down Expand Up @@ -2021,7 +2021,7 @@ pub fn trans_enum_variant_or_tuple_like_struct<A:IdAndTy>(
ccx: @mut CrateContext,
ctor_id: ast::NodeId,
args: &[A],
disr: uint,
disr: ty::Disr,
param_substs: Option<@param_substs>,
llfndecl: ValueRef)
{
Expand Down Expand Up @@ -2629,7 +2629,7 @@ pub fn trans_constant(ccx: &mut CrateContext, it: @ast::item) {
}
};
unsafe {
llvm::LLVMSetInitializer(discrim_gvar, C_uint(ccx, disr_val));
llvm::LLVMSetInitializer(discrim_gvar, C_uint(ccx, disr_val /*bad*/ as uint));
llvm::LLVMSetGlobalConstant(discrim_gvar, True);
}
ccx.discrims.insert(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ fn const_expr_unadjusted(cx: @mut CrateContext, e: &ast::expr) -> ValueRef {
(expr::cast_enum, expr::cast_float) => {
let repr = adt::represent_type(cx, basety);
let discr = adt::const_get_discrim(cx, repr, v);
let iv = C_uint(cx, discr);
let iv = C_integral(cx.int_type, discr, false);
let ety_cast = expr::cast_type_kind(ety);
match ety_cast {
expr::cast_integral => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/trans/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ pub fn trans_local_var(bcx: @mut Block, def: ast::def) -> Datum {
pub fn with_field_tys<R>(tcx: ty::ctxt,
ty: ty::t,
node_id_opt: Option<ast::NodeId>,
op: &fn(uint, (&[ty::field])) -> R) -> R {
op: &fn(ty::Disr, (&[ty::field])) -> R) -> R {
match ty::get(ty).sty {
ty::ty_struct(did, ref substs) => {
op(0, struct_fields(tcx, did, substs))
Expand Down Expand Up @@ -1258,7 +1258,7 @@ struct StructBaseInfo {
* - `optbase` contains information on the base struct (if any) from
* which remaining fields are copied; see comments on `StructBaseInfo`.
*/
fn trans_adt(bcx: @mut Block, repr: &adt::Repr, discr: uint,
fn trans_adt(bcx: @mut Block, repr: &adt::Repr, discr: ty::Disr,
fields: &[(uint, @ast::expr)],
optbase: Option<StructBaseInfo>,
dest: Dest) -> @mut Block {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/trans/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,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),
this.c_uint(v.disr_val),
this.c_uint(v.disr_val /*bad*/ as uint),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this'll be the one that has to change in continued work. Looking good so far!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'm going to fix that one now, because otherwise a 32-to-64-bit cross-compiled program can do this:

task <unnamed> failed at 'enum value matched no variant', /home/jld/src/rust/src/libstd/repr.rs:521

when trying to %? a variant with a non-32-bit discriminant.

this.c_uint(v.args.len()),
this.c_slice(name)];
do this.bracketed("enum_variant", variant_args) |this| {
Expand Down
14 changes: 8 additions & 6 deletions src/librustc/middle/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ use syntax::abi::AbiSet;
use syntax;
use extra::enum_set::{EnumSet, CLike};

pub static INITIAL_DISCRIMINANT_VALUE: uint = 0;
pub type Disr = u64;

pub static INITIAL_DISCRIMINANT_VALUE: Disr = 0;

// Data types

Expand Down Expand Up @@ -3759,7 +3761,7 @@ pub struct VariantInfo {
ctor_ty: t,
name: ast::ident,
id: ast::def_id,
disr_val: uint,
disr_val: Disr,
vis: visibility
}

Expand All @@ -3770,7 +3772,7 @@ impl VariantInfo {
/// Does not do any caching of the value in the type context.
pub fn from_ast_variant(cx: ctxt,
ast_variant: &ast::variant,
discriminant: uint) -> VariantInfo {
discriminant: Disr) -> VariantInfo {

let ctor_ty = node_id_to_type(cx, ast_variant.node.id);

Expand Down Expand Up @@ -3964,7 +3966,7 @@ pub fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[@VariantInfo] {
node: ast::item_enum(ref enum_definition, _),
_
}, _) => {
let mut last_discriminant: Option<uint> = None;
let mut last_discriminant: Option<Disr> = None;
@enum_definition.variants.iter().map(|variant| {

let mut discriminant = match last_discriminant {
Expand All @@ -3974,8 +3976,8 @@ pub fn enum_variants(cx: ctxt, id: ast::def_id) -> @~[@VariantInfo] {

match variant.node.disr_expr {
Some(e) => match const_eval::eval_const_expr_partial(&cx, e) {
Ok(const_eval::const_int(val)) => discriminant = val as uint,
Ok(const_eval::const_uint(val)) => discriminant = val as uint,
Ok(const_eval::const_int(val)) => discriminant = val as Disr,
Ok(const_eval::const_uint(val)) => discriminant = val as Disr,
Ok(_) => {
cx.sess.span_err(e.span, "expected signed integer constant");
}
Expand Down
Loading