From 1b2a51ee6674006974ce3b3d52012dd24bc5b9f5 Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Sun, 15 Jan 2012 17:10:59 -0700 Subject: [PATCH 1/3] In the tutorial, document that C-like enums can have the discriminator values set and that it is possible to cast them to scalar values. --- doc/tutorial/data.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/tutorial/data.md b/doc/tutorial/data.md index 678714d388bfb..29183220d3013 100644 --- a/doc/tutorial/data.md +++ b/doc/tutorial/data.md @@ -103,6 +103,24 @@ equivalent to a C enum: This will define `north`, `east`, `south`, and `west` as constants, all of which have type `direction`. +When the enum is C like, that is none of the variants have parameters, +it is possible to explicit set the discriminator values to an integer +value: + + enum color { + red = 0xff0000; + green = 0x00ff00; + blue = 0x0000ff; + } + +If an explicit discriminator is not specified for a variant, the value +defaults to the value of the previous variant plus one. If the first +variant does not have a discriminator, it defaults to 0. For example, +the value of `north` is 0, `east` is 1, etc. + +When an enum is C-like the `as` cast operator can be used to get the +discriminator's value. + There is a special case for enums with a single variant. These are From 50d991d173c842c2ecb8fcea11ff30298b49341d Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Sun, 15 Jan 2012 19:56:20 -0700 Subject: [PATCH 2/3] Update pretty printer to print out disr. values. Partly fixes issue #1510. "rustc --pretty=typed" fails. --- src/comp/syntax/print/pprust.rs | 8 ++++++++ src/test/run-pass/enum-disr-val-pretty.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 src/test/run-pass/enum-disr-val-pretty.rs diff --git a/src/comp/syntax/print/pprust.rs b/src/comp/syntax/print/pprust.rs index e0c1acc4fcc01..38449d6e255b5 100644 --- a/src/comp/syntax/print/pprust.rs +++ b/src/comp/syntax/print/pprust.rs @@ -428,6 +428,14 @@ fn print_item(s: ps, &&item: @ast::item) { commasep(s, consistent, v.node.args, print_variant_arg); pclose(s); } + alt v.node.disr_expr { + some(expr) { + nbsp(s); + word_nbsp(s, "="); + print_expr(s, expr); + } + _ {} + } word(s.s, ";"); maybe_print_trailing_comment(s, v.span, none::); } diff --git a/src/test/run-pass/enum-disr-val-pretty.rs b/src/test/run-pass/enum-disr-val-pretty.rs new file mode 100644 index 0000000000000..d051aa233d60a --- /dev/null +++ b/src/test/run-pass/enum-disr-val-pretty.rs @@ -0,0 +1,16 @@ +// pp-exact + +enum color { red = 1; green; blue; imaginary = -1; } + +fn main() { + test_color(red, 1, "red"); + test_color(green, 2, "green"); + test_color(blue, 3, "blue"); + test_color(imaginary, -1, "imaginary"); +} + +fn test_color(color: color, val: int, name: str) { + assert (color as int == val); + assert (color as float == val as float); +} + From a4d4420c2096d6972e1d777b9314072d3a2b5f8a Mon Sep 17 00:00:00 2001 From: Kevin Atkinson Date: Mon, 16 Jan 2012 02:36:47 -0700 Subject: [PATCH 3/3] Don't evaluate discriminator value constants when parsing. Remove disr_val from ast::variant_ and always use ty::variant_info when the value is needed. Move what was done during parsing into other passes, primary typeck.rs. This move also correctly type checks the disr. value expression; thus, fixing rustc --pretty=typed when disr. values are used. --- src/comp/metadata/encoder.rs | 9 ++-- src/comp/middle/check_const.rs | 7 +++ src/comp/middle/trans.rs | 17 +++++-- src/comp/middle/ty.rs | 15 +++++- src/comp/middle/typeck.rs | 50 +++++++++++++++++++ src/comp/syntax/ast.rs | 2 +- src/comp/syntax/ast_util.rs | 3 +- src/comp/syntax/fold.rs | 17 ++----- src/comp/syntax/parse/parser.rs | 29 +---------- .../tag-variant-disr-type-mismatch.rs | 8 +++ 10 files changed, 106 insertions(+), 51 deletions(-) create mode 100644 src/test/compile-fail/tag-variant-disr-type-mismatch.rs diff --git a/src/comp/metadata/encoder.rs b/src/comp/metadata/encoder.rs index 537c420aa71ce..41235c01d3dc7 100644 --- a/src/comp/metadata/encoder.rs +++ b/src/comp/metadata/encoder.rs @@ -232,6 +232,8 @@ fn encode_tag_variant_info(ecx: @encode_ctxt, ebml_w: ebml::writer, id: node_id, variants: [variant], &index: [entry], ty_params: [ty_param]) { let disr_val = 0; + let i = 0; + let vi = ty::tag_variants(ecx.ccx.tcx, {crate: local_crate, node: id}); for variant: variant in variants { index += [{val: variant.node.id, pos: ebml_w.writer.tell()}]; ebml::start_tag(ebml_w, tag_items_data_item); @@ -244,13 +246,14 @@ fn encode_tag_variant_info(ecx: @encode_ctxt, ebml_w: ebml::writer, encode_symbol(ecx, ebml_w, variant.node.id); } encode_discriminant(ecx, ebml_w, variant.node.id); - if variant.node.disr_val != disr_val { - encode_disr_val(ecx, ebml_w, variant.node.disr_val); - disr_val = variant.node.disr_val; + if vi[i].disr_val != disr_val { + encode_disr_val(ecx, ebml_w, vi[i].disr_val); + disr_val = vi[i].disr_val; } encode_type_param_bounds(ebml_w, ecx, ty_params); ebml::end_tag(ebml_w); disr_val += 1; + i += 1; } } diff --git a/src/comp/middle/check_const.rs b/src/comp/middle/check_const.rs index c0460e90cb06f..943c9184fe209 100644 --- a/src/comp/middle/check_const.rs +++ b/src/comp/middle/check_const.rs @@ -15,6 +15,13 @@ fn check_crate(sess: session, crate: @crate) { fn check_item(it: @item, &&_is_const: bool, v: visit::vt) { alt it.node { item_const(_, ex) { v.visit_expr(ex, true, v); } + item_tag(vs, _) { + for var in vs { + option::may(var.node.disr_expr) {|ex| + v.visit_expr(ex, true, v); + } + } + } _ { visit::visit_item(it, false, v); } } } diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs index f7774883fabaf..44b55b0c34ddd 100644 --- a/src/comp/middle/trans.rs +++ b/src/comp/middle/trans.rs @@ -4543,7 +4543,7 @@ fn trans_res_ctor(cx: @local_ctxt, sp: span, dtor: ast::fn_decl, fn trans_tag_variant(cx: @local_ctxt, tag_id: ast::node_id, - variant: ast::variant, is_degen: bool, + variant: ast::variant, disr: int, is_degen: bool, ty_params: [ast::ty_param]) { let ccx = cx.ccx; @@ -4593,7 +4593,7 @@ fn trans_tag_variant(cx: @local_ctxt, tag_id: ast::node_id, let lltagptr = PointerCast(bcx, fcx.llretptr, T_opaque_tag_ptr(ccx)); let lldiscrimptr = GEPi(bcx, lltagptr, [0, 0]); - Store(bcx, C_int(ccx, variant.node.disr_val), lldiscrimptr); + Store(bcx, C_int(ccx, disr), lldiscrimptr); GEPi(bcx, lltagptr, [0, 1]) }; i = 0u; @@ -4938,8 +4938,13 @@ fn trans_item(cx: @local_ctxt, item: ast::item) { ast::item_tag(variants, tps) { let sub_cx = extend_path(cx, item.ident); let degen = vec::len(variants) == 1u; + let vi = ty::tag_variants(cx.ccx.tcx, {crate: ast::local_crate, + node: item.id}); + let i = 0; for variant: ast::variant in variants { - trans_tag_variant(sub_cx, item.id, variant, degen, tps); + trans_tag_variant(sub_cx, item.id, variant, + vi[i].disr_val, degen, tps); + i += 1; } } ast::item_const(_, expr) { trans_const(cx.ccx, expr, item.id); } @@ -5268,10 +5273,13 @@ fn trans_constant(ccx: @crate_ctxt, it: @ast::item, &&pt: [str], visit::visit_item(it, new_pt, v); alt it.node { ast::item_tag(variants, _) { + let vi = ty::tag_variants(ccx.tcx, {crate: ast::local_crate, + node: it.id}); + let i = 0; for variant in variants { let p = new_pt + [variant.node.name, "discrim"]; let s = mangle_exported_name(ccx, p, ty::mk_int(ccx.tcx)); - let disr_val = variant.node.disr_val; + let disr_val = vi[i].disr_val; let discrim_gvar = str::as_buf(s, {|buf| llvm::LLVMAddGlobal(ccx.llmod, ccx.int_type, buf) }); @@ -5280,6 +5288,7 @@ fn trans_constant(ccx: @crate_ctxt, it: @ast::item, &&pt: [str], ccx.discrims.insert( ast_util::local_def(variant.node.id), discrim_gvar); ccx.discrim_symbols.insert(variant.node.id, s); + i += 1; } } ast::item_impl(tps, some(@{node: ast::ty_path(_, id), _}), _, ms) { diff --git a/src/comp/middle/ty.rs b/src/comp/middle/ty.rs index c6777bfd491f6..1b5b24bcb4984 100644 --- a/src/comp/middle/ty.rs +++ b/src/comp/middle/ty.rs @@ -2631,17 +2631,30 @@ fn tag_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] { let result = if ast::local_crate != id.crate { @csearch::get_tag_variants(cx, id) } else { + // FIXME: Now that the variants are run through the type checker (to + // check the disr_expr if it exists), this code should likely be + // moved there to avoid having to call eval_const_expr twice alt cx.items.get(id.node) { ast_map::node_item(@{node: ast::item_tag(variants, _), _}) { + let disr_val = -1; @vec::map(variants, {|variant| let ctor_ty = node_id_to_monotype(cx, variant.node.id); let arg_tys = if vec::len(variant.node.args) > 0u { vec::map(ty_fn_args(cx, ctor_ty), {|a| a.ty}) } else { [] }; + alt variant.node.disr_expr { + some (ex) { + // FIXME: issue #1417 + disr_val = alt syntax::ast_util::eval_const_expr(ex) { + ast_util::const_int(val) {val as int} + } + } + _ {disr_val += 1;} + } @{args: arg_tys, ctor_ty: ctor_ty, id: ast_util::local_def(variant.node.id), - disr_val: variant.node.disr_val + disr_val: disr_val } }) } diff --git a/src/comp/middle/typeck.rs b/src/comp/middle/typeck.rs index d6b921bd6f991..741f3c11931d7 100644 --- a/src/comp/middle/typeck.rs +++ b/src/comp/middle/typeck.rs @@ -2461,6 +2461,55 @@ fn check_const(ccx: @crate_ctxt, _sp: span, e: @ast::expr, id: ast::node_id) { demand::simple(fcx, e.span, declty, cty); } +fn check_tag_variants(ccx: @crate_ctxt, _sp: span, vs: [ast::variant], + id: ast::node_id) { + // FIXME: this is kinda a kludge; we manufacture a fake function context + // and statement context for checking the initializer expression. + let rty = node_id_to_type(ccx.tcx, id); + let fixups: [ast::node_id] = []; + let fcx: @fn_ctxt = + @{ret_ty: rty, + purity: ast::pure_fn, + proto: ast::proto_box, + var_bindings: ty::unify::mk_var_bindings(), + locals: new_int_hash::(), + next_var_id: @mutable 0, + mutable fixups: fixups, + ccx: ccx}; + let disr_vals: [int] = []; + let disr_val = 0; + for v in vs { + alt v.node.disr_expr { + some(e) { + check_expr(fcx, e); + let cty = expr_ty(fcx.ccx.tcx, e); + let declty =ty::mk_int(fcx.ccx.tcx); + demand::simple(fcx, e.span, declty, cty); + // FIXME: issue #1417 + // Also, check_expr (from check_const pass) doesn't guarantee that + // the expression in an form that eval_const_expr can handle, so + // we may still get an internal compiler error + alt syntax::ast_util::eval_const_expr(e) { + syntax::ast_util::const_int(val) { + disr_val = val as int; + } + _ { + ccx.tcx.sess.span_err(e.span, + "expected signed integer constant"); + } + } + } + _ {} + } + if vec::member(disr_val, disr_vals) { + ccx.tcx.sess.span_err(v.span, + "discriminator value already exists."); + } + disr_vals += [disr_val]; + disr_val += 1; + } +} + // A generic function for checking the pred in a check // or if-check fn check_pred_expr(fcx: @fn_ctxt, e: @ast::expr) -> bool { @@ -2632,6 +2681,7 @@ fn check_method(ccx: @crate_ctxt, method: @ast::method) { fn check_item(ccx: @crate_ctxt, it: @ast::item) { alt it.node { ast::item_const(_, e) { check_const(ccx, it.span, e, it.id); } + ast::item_tag(vs, _) { check_tag_variants(ccx, it.span, vs, it.id); } ast::item_fn(decl, tps, body) { check_fn(ccx, ast::proto_bare, decl, body, it.id, none); } diff --git a/src/comp/syntax/ast.rs b/src/comp/syntax/ast.rs index 4ce4e1ab11139..b953362b1be89 100644 --- a/src/comp/syntax/ast.rs +++ b/src/comp/syntax/ast.rs @@ -409,7 +409,7 @@ type native_mod = type variant_arg = {ty: @ty, id: node_id}; type variant_ = {name: ident, args: [variant_arg], id: node_id, - disr_val: int, disr_expr: option::t<@expr>}; + disr_expr: option::t<@expr>}; type variant = spanned; diff --git a/src/comp/syntax/ast_util.rs b/src/comp/syntax/ast_util.rs index d8070aa098d6c..7d30f368ff85c 100644 --- a/src/comp/syntax/ast_util.rs +++ b/src/comp/syntax/ast_util.rs @@ -241,8 +241,7 @@ tag const_val { const_str(str); } -// FIXME (#1417): any function that uses this function should likely be moved -// into the middle end +// FIXME: issue #1417 fn eval_const_expr(e: @expr) -> const_val { fn fromb(b: bool) -> const_val { const_int(b as i64) } alt e.node { diff --git a/src/comp/syntax/fold.rs b/src/comp/syntax/fold.rs index 54684bca83b65..e74e1cd9ba75e 100644 --- a/src/comp/syntax/fold.rs +++ b/src/comp/syntax/fold.rs @@ -432,21 +432,12 @@ fn noop_fold_variant(v: variant_, fld: ast_fold) -> variant_ { } let fold_variant_arg = bind fold_variant_arg_(_, fld); let args = vec::map(v.args, fold_variant_arg); - let (de, dv) = alt v.disr_expr { - some(e) { - let de = fld.fold_expr(e); - // FIXME (#1417): see parser.rs - let dv = alt syntax::ast_util::eval_const_expr(e) { - ast_util::const_int(val) { - val as int - } - }; - (some(de), dv) - } - none. { (none, v.disr_val) } + let de = alt v.disr_expr { + some(e) {some(fld.fold_expr(e))} + none. {none} }; ret {name: v.name, args: args, id: v.id, - disr_val: dv, disr_expr: de}; + disr_expr: de}; } fn noop_fold_ident(&&i: ident, _fld: ast_fold) -> ident { ret i; } diff --git a/src/comp/syntax/parse/parser.rs b/src/comp/syntax/parse/parser.rs index 3f5cae0a16119..ef65a0bd89713 100644 --- a/src/comp/syntax/parse/parser.rs +++ b/src/comp/syntax/parse/parser.rs @@ -2023,7 +2023,6 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item { {name: id, args: [{ty: ty, id: p.get_id()}], id: p.get_id(), - disr_val: 0, disr_expr: none}); ret mk_item(p, lo, ty.span.hi, id, ast::item_tag([variant], ty_params), attrs); @@ -2031,7 +2030,6 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item { expect(p, token::LBRACE); let all_nullary = true; let have_disr = false; - let disr_val = 0; while p.token != token::RBRACE { let tok = p.token; alt tok { @@ -2056,38 +2054,15 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item { token::EQ. { have_disr = true; p.bump(); - let e = parse_expr(p); - // FIXME: eval_const_expr does no error checking, nor do I. - // Also, the parser is not the right place to do this; likely - // somewhere in the middle end so that constants can be - // refereed to, even if they are after the declaration for the - // type. Finally, eval_const_expr probably shouldn't exist as - // it Graydon puts it: "[I] am a little worried at its - // presence since it quasi-duplicates stuff that trans should - // probably be doing." (See issue #1417) - alt syntax::ast_util::eval_const_expr(e) { - syntax::ast_util::const_int(val) { - // FIXME: check that value is in range - disr_val = val as int; - } - } - if option::is_some - (vec::find - (variants, {|v| v.node.disr_val == disr_val})) - { - p.fatal("discriminator value " + /* str(disr_val) + */ - "already exists."); - } - disr_expr = some(e); + disr_expr = some(parse_expr(p)); } _ {/* empty */ } } expect(p, token::SEMI); p.get_id(); let vr = {name: p.get_str(name), args: args, id: p.get_id(), - disr_val: disr_val, disr_expr: disr_expr}; + disr_expr: disr_expr}; variants += [spanned(vlo, vhi, vr)]; - disr_val += 1; } token::RBRACE. {/* empty */ } _ { diff --git a/src/test/compile-fail/tag-variant-disr-type-mismatch.rs b/src/test/compile-fail/tag-variant-disr-type-mismatch.rs new file mode 100644 index 0000000000000..52badcd35e339 --- /dev/null +++ b/src/test/compile-fail/tag-variant-disr-type-mismatch.rs @@ -0,0 +1,8 @@ +//error-pattern: mismatched types + +tag color { + red = 1u; + blue = 2; +} + +fn main() {}