Skip to content

Commit 0832d4b

Browse files
kevinakevina
committed
Support explicit discriminant numbers on tag variants.
Addresses issue rust-lang#1393. For now disallow disr. values unless all variants use nullary contractors (i.e. "enum-like"). Disr. values are now encoded in the crate metadata, but only when it will differ from the inferred value based on the order.
1 parent af086aa commit 0832d4b

File tree

12 files changed

+167
-30
lines changed

12 files changed

+167
-30
lines changed

‎src/comp/metadata/common.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ const tag_item_method: uint = 0x31u;
7070
const tag_impl_iface: uint = 0x32u;
7171
const tag_impl_iface_did: uint = 0x33u;
7272

73+
// discriminator value for variants
74+
const tag_disr_val: uint = 0x34u;
75+
7376
// djb's cdb hashes.
7477
fn hash_node_id(&&node_id: int) -> uint { ret 177573u ^ (node_id as uint); }
7578

‎src/comp/metadata/decoder.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,17 @@ fn variant_tag_id(d: ebml::doc) -> ast::def_id {
8888
ret parse_def_id(ebml::doc_data(tagdoc));
8989
}
9090

91+
fn variant_disr_val(d: ebml::doc) -> option::t<int> {
92+
alt ebml::maybe_get_doc(d, tag_disr_val) {
93+
some(val_doc) {
94+
let val_buf = ebml::doc_data(val_doc);
95+
let val = int::parse_buf(val_buf, 10u);
96+
ret some(val);
97+
}
98+
_ { ret none;}
99+
}
100+
}
101+
91102
fn doc_type(doc: ebml::doc, tcx: ty::ctxt, cdata: cmd) -> ty::t {
92103
let tp = ebml::get_doc(doc, tag_items_data_item_type);
93104
parse_ty_data(tp.data, cdata.cnum, tp.start, tcx, {|did|
@@ -240,6 +251,7 @@ fn get_tag_variants(cdata: cmd, id: ast::node_id, tcx: ty::ctxt)
240251
let item = find_item(id, items);
241252
let infos: [ty::variant_info] = [];
242253
let variant_ids = tag_variant_ids(item, cdata);
254+
let disr_val = 0;
243255
for did: ast::def_id in variant_ids {
244256
let item = find_item(did.node, items);
245257
let ctor_ty = item_type(item, tcx, cdata);
@@ -250,7 +262,13 @@ fn get_tag_variants(cdata: cmd, id: ast::node_id, tcx: ty::ctxt)
250262
}
251263
_ { /* Nullary tag variant. */ }
252264
}
253-
infos += [@{args: arg_tys, ctor_ty: ctor_ty, id: did}];
265+
alt variant_disr_val(item) {
266+
some(val) { disr_val = val; }
267+
_ { /* empty */ }
268+
}
269+
infos += [@{args: arg_tys, ctor_ty: ctor_ty, id: did,
270+
disr_val: disr_val}];
271+
disr_val += 1;
254272
}
255273
ret infos;
256274
}

‎src/comp/metadata/encoder.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ fn encode_discriminant(ecx: @encode_ctxt, ebml_w: ebml::writer, id: node_id) {
228228
ebml::end_tag(ebml_w);
229229
}
230230

231+
fn encode_disr_val(_ecx: @encode_ctxt, ebml_w: ebml::writer, disr_val: int) {
232+
ebml::start_tag(ebml_w, tag_disr_val);
233+
ebml_w.writer.write(str::bytes(int::to_str(disr_val,10u)));
234+
ebml::end_tag(ebml_w);
235+
}
236+
231237
fn encode_tag_id(ebml_w: ebml::writer, id: def_id) {
232238
ebml::start_tag(ebml_w, tag_items_data_item_tag_id);
233239
ebml_w.writer.write(str::bytes(def_to_str(id)));
@@ -237,6 +243,7 @@ fn encode_tag_id(ebml_w: ebml::writer, id: def_id) {
237243
fn encode_tag_variant_info(ecx: @encode_ctxt, ebml_w: ebml::writer,
238244
id: node_id, variants: [variant],
239245
&index: [entry<int>], ty_params: [ty_param]) {
246+
let disr_val = 0;
240247
for variant: variant in variants {
241248
index += [{val: variant.node.id, pos: ebml_w.writer.tell()}];
242249
ebml::start_tag(ebml_w, tag_items_data_item);
@@ -249,8 +256,13 @@ fn encode_tag_variant_info(ecx: @encode_ctxt, ebml_w: ebml::writer,
249256
encode_symbol(ecx, ebml_w, variant.node.id);
250257
}
251258
encode_discriminant(ecx, ebml_w, variant.node.id);
259+
if variant.node.disr_val != disr_val {
260+
encode_disr_val(ecx, ebml_w, variant.node.disr_val);
261+
disr_val = variant.node.disr_val;
262+
}
252263
encode_type_param_bounds(ebml_w, ecx, ty_params);
253264
ebml::end_tag(ebml_w);
265+
disr_val += 1;
254266
}
255267
}
256268

‎src/comp/middle/trans.rs

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,17 +1726,15 @@ fn iter_structural_ty(cx: @block_ctxt, av: ValueRef, t: ty::t,
17261726
Unreachable(unr_cx);
17271727
let llswitch = Switch(cx, lldiscrim_a, unr_cx.llbb, n_variants);
17281728
let next_cx = new_sub_block_ctxt(cx, "tag-iter-next");
1729-
let i = 0u;
17301729
for variant: ty::variant_info in *variants {
17311730
let variant_cx =
17321731
new_sub_block_ctxt(cx,
17331732
"tag-iter-variant-" +
1734-
uint::to_str(i, 10u));
1735-
AddCase(llswitch, C_int(ccx, i as int), variant_cx.llbb);
1733+
int::to_str(variant.disr_val, 10u));
1734+
AddCase(llswitch, C_int(ccx, variant.disr_val), variant_cx.llbb);
17361735
variant_cx =
17371736
iter_variant(variant_cx, llunion_a_ptr, variant, tps, tid, f);
17381737
Br(variant_cx, next_cx.llbb);
1739-
i += 1u;
17401738
}
17411739
ret next_cx;
17421740
}
@@ -2745,12 +2743,9 @@ fn trans_var(cx: @block_ctxt, sp: span, def: ast::def, id: ast::node_id)
27452743
let bcx = alloc_result.bcx;
27462744
let lltagptr = PointerCast(bcx, lltagblob, T_ptr(lltagty));
27472745
let lldiscrimptr = GEPi(bcx, lltagptr, [0, 0]);
2748-
let d = if vec::len(*ty::tag_variants(ccx.tcx, tid)) != 1u {
2749-
let lldiscrim_gv = lookup_discriminant(bcx.fcx.lcx, vid);
2750-
let lldiscrim = Load(bcx, lldiscrim_gv);
2751-
lldiscrim
2752-
} else { C_int(ccx, 0) };
2753-
Store(bcx, d, lldiscrimptr);
2746+
let lldiscrim_gv = lookup_discriminant(bcx.fcx.lcx, vid);
2747+
let lldiscrim = Load(bcx, lldiscrim_gv);
2748+
Store(bcx, lldiscrim, lldiscrimptr);
27542749
ret lval_no_env(bcx, lltagptr, temporary);
27552750
}
27562751
}
@@ -4685,7 +4680,7 @@ fn trans_res_ctor(cx: @local_ctxt, sp: span, dtor: ast::fn_decl,
46854680

46864681

46874682
fn trans_tag_variant(cx: @local_ctxt, tag_id: ast::node_id,
4688-
variant: ast::variant, index: int, is_degen: bool,
4683+
variant: ast::variant, is_degen: bool,
46894684
ty_params: [ast::ty_param]) {
46904685
let ccx = cx.ccx;
46914686

@@ -4735,7 +4730,7 @@ fn trans_tag_variant(cx: @local_ctxt, tag_id: ast::node_id,
47354730
let lltagptr =
47364731
PointerCast(bcx, fcx.llretptr, T_opaque_tag_ptr(ccx));
47374732
let lldiscrimptr = GEPi(bcx, lltagptr, [0, 0]);
4738-
Store(bcx, C_int(ccx, index), lldiscrimptr);
4733+
Store(bcx, C_int(ccx, variant.node.disr_val), lldiscrimptr);
47394734
GEPi(bcx, lltagptr, [0, 1])
47404735
};
47414736
i = 0u;
@@ -5086,10 +5081,8 @@ fn trans_item(cx: @local_ctxt, item: ast::item) {
50865081
ast::item_tag(variants, tps) {
50875082
let sub_cx = extend_path(cx, item.ident);
50885083
let degen = vec::len(variants) == 1u;
5089-
let i = 0;
50905084
for variant: ast::variant in variants {
5091-
trans_tag_variant(sub_cx, item.id, variant, i, degen, tps);
5092-
i += 1;
5085+
trans_tag_variant(sub_cx, item.id, variant, degen, tps);
50935086
}
50945087
}
50955088
ast::item_const(_, expr) { trans_const(cx.ccx, expr, item.id); }
@@ -5421,19 +5414,18 @@ fn trans_constant(ccx: @crate_ctxt, it: @ast::item, &&pt: [str],
54215414
visit::visit_item(it, new_pt, v);
54225415
alt it.node {
54235416
ast::item_tag(variants, _) {
5424-
let i = 0u;
54255417
for variant in variants {
54265418
let p = new_pt + [variant.node.name, "discrim"];
54275419
let s = mangle_exported_name(ccx, p, ty::mk_int(ccx.tcx));
5420+
let disr_val = variant.node.disr_val;
54285421
let discrim_gvar = str::as_buf(s, {|buf|
54295422
llvm::LLVMAddGlobal(ccx.llmod, ccx.int_type, buf)
54305423
});
5431-
llvm::LLVMSetInitializer(discrim_gvar, C_int(ccx, i as int));
5424+
llvm::LLVMSetInitializer(discrim_gvar, C_int(ccx, disr_val));
54325425
llvm::LLVMSetGlobalConstant(discrim_gvar, True);
54335426
ccx.discrims.insert(
54345427
ast_util::local_def(variant.node.id), discrim_gvar);
54355428
ccx.discrim_symbols.insert(variant.node.id, s);
5436-
i += 1u;
54375429
}
54385430
}
54395431
ast::item_impl(tps, some(@{node: ast::ty_path(_, id), _}), _, ms) {

‎src/comp/middle/trans_alt.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import trans_common::*;
1616
// An option identifying a branch (either a literal, a tag variant or a range)
1717
tag opt {
1818
lit(@ast::expr);
19-
var(/* variant id */uint, /* variant dids */{tg: def_id, var: def_id});
19+
var(/* disr val */int, /* variant dids */{tg: def_id, var: def_id});
2020
range(@ast::expr, @ast::expr);
2121
}
2222
fn opt_eq(a: opt, b: opt) -> bool {
@@ -53,7 +53,7 @@ fn trans_opt(bcx: @block_ctxt, o: opt) -> opt_result {
5353
}
5454
}
5555
}
56-
var(id, _) { ret single_result(rslt(bcx, C_int(ccx, id as int))); }
56+
var(disr_val, _) { ret single_result(rslt(bcx, C_int(ccx, disr_val))); }
5757
range(l1, l2) {
5858
ret range_result(rslt(bcx, trans::trans_const_expr(ccx, l1)),
5959
rslt(bcx, trans::trans_const_expr(ccx, l2)));
@@ -64,10 +64,8 @@ fn trans_opt(bcx: @block_ctxt, o: opt) -> opt_result {
6464
fn variant_opt(ccx: @crate_ctxt, pat_id: ast::node_id) -> opt {
6565
let vdef = ast_util::variant_def_ids(ccx.tcx.def_map.get(pat_id));
6666
let variants = ty::tag_variants(ccx.tcx, vdef.tg);
67-
let i = 0u;
6867
for v: ty::variant_info in *variants {
69-
if vdef.var == v.id { ret var(i, vdef); }
70-
i += 1u;
68+
if vdef.var == v.id { ret var(v.disr_val, vdef); }
7169
}
7270
fail;
7371
}

‎src/comp/middle/ty.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2686,7 +2686,8 @@ fn impl_iface(cx: ctxt, id: ast::def_id) -> option::t<t> {
26862686
}
26872687

26882688
// Tag information
2689-
type variant_info = @{args: [ty::t], ctor_ty: ty::t, id: ast::def_id};
2689+
type variant_info = @{args: [ty::t], ctor_ty: ty::t, id: ast::def_id,
2690+
disr_val: int};
26902691

26912692
fn tag_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] {
26922693
alt cx.tag_var_cache.find(id) {
@@ -2705,7 +2706,9 @@ fn tag_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] {
27052706
} else { [] };
27062707
@{args: arg_tys,
27072708
ctor_ty: ctor_ty,
2708-
id: ast_util::local_def(variant.node.id)}
2709+
id: ast_util::local_def(variant.node.id),
2710+
disr_val: variant.node.disr_val
2711+
}
27092712
})
27102713
}
27112714
}

‎src/comp/syntax/ast.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,8 @@ type native_mod =
416416

417417
type variant_arg = {ty: @ty, id: node_id};
418418

419-
type variant_ = {name: ident, args: [variant_arg], id: node_id};
419+
type variant_ = {name: ident, args: [variant_arg], id: node_id,
420+
disr_val: int, disr_expr: option::t<@expr>};
420421

421422
type variant = spanned<variant_>;
422423

‎src/comp/syntax/fold.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,9 @@ fn noop_fold_variant(v: variant_, fld: ast_fold) -> variant_ {
469469
ret {ty: fld.fold_ty(va.ty), id: va.id};
470470
}
471471
let fold_variant_arg = bind fold_variant_arg_(_, fld);
472-
ret {name: v.name, args: vec::map(v.args, fold_variant_arg), id: v.id};
472+
ret {name: v.name, args: vec::map(v.args, fold_variant_arg), id: v.id,
473+
disr_val: v.disr_val, disr_expr: v.disr_expr
474+
/* FIXME: is this right (copying disr_val and disr_expr) */};
473475
}
474476

475477
fn noop_fold_ident(&&i: ident, _fld: ast_fold) -> ident { ret i; }

‎src/comp/syntax/parse/parser.rs

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2067,11 +2067,16 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
20672067
spanned(ty.span.lo, ty.span.hi,
20682068
{name: id,
20692069
args: [{ty: ty, id: p.get_id()}],
2070-
id: p.get_id()});
2070+
id: p.get_id(),
2071+
disr_val: 0,
2072+
disr_expr: none});
20712073
ret mk_item(p, lo, ty.span.hi, id,
20722074
ast::item_tag([variant], ty_params), attrs);
20732075
}
20742076
expect(p, token::LBRACE);
2077+
let all_nullary = true;
2078+
let have_disr = false;
2079+
let disr_val = 0;
20752080
while p.peek() != token::RBRACE {
20762081
let tok = p.peek();
20772082
alt tok {
@@ -2081,8 +2086,10 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
20812086
p.bump();
20822087
let args: [ast::variant_arg] = [];
20832088
let vhi = p.get_hi_pos();
2089+
let disr_expr = none;
20842090
alt p.peek() {
20852091
token::LPAREN. {
2092+
all_nullary = false;
20862093
let arg_tys = parse_seq(token::LPAREN, token::RPAREN,
20872094
seq_sep(token::COMMA),
20882095
{|p| parse_ty(p, false)}, p);
@@ -2091,12 +2098,41 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
20912098
}
20922099
vhi = arg_tys.span.hi;
20932100
}
2101+
token::EQ. {
2102+
have_disr = true;
2103+
p.bump();
2104+
let e = parse_expr(p);
2105+
// FIXME: eval_const_expr does no error checking, nor do I.
2106+
// Also, the parser is not the right place to do this; likely
2107+
// somewhere in the middle end so that constants can be
2108+
// refereed to, even if they are after the declaration for the
2109+
// type. Finally, eval_const_expr probably shouldn't exist as
2110+
// it Graydon puts it: "[I] am a little worried at its
2111+
// presence since it quasi-duplicates stuff that trans should
2112+
// probably be doing." (See issue #1417)
2113+
alt syntax::ast_util::eval_const_expr(e) {
2114+
syntax::ast_util::const_int(val) {
2115+
disr_val = val as int;
2116+
// FIXME: check that value is in range
2117+
}
2118+
}
2119+
if option::is_some
2120+
(vec::find
2121+
(variants, {|v| v.node.disr_val == disr_val}))
2122+
{
2123+
p.fatal("discriminator value " + /* str(disr_val) + */
2124+
"already exists.");
2125+
}
2126+
disr_expr = some(e);
2127+
}
20942128
_ {/* empty */ }
20952129
}
20962130
expect(p, token::SEMI);
20972131
p.get_id();
2098-
let vr = {name: p.get_str(name), args: args, id: p.get_id()};
2132+
let vr = {name: p.get_str(name), args: args, id: p.get_id(),
2133+
disr_val: disr_val, disr_expr: disr_expr};
20992134
variants += [spanned(vlo, vhi, vr)];
2135+
disr_val += 1;
21002136
}
21012137
token::RBRACE. {/* empty */ }
21022138
_ {
@@ -2106,6 +2142,9 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
21062142
}
21072143
}
21082144
let hi = p.get_hi_pos();
2145+
if (have_disr && !all_nullary) {
2146+
p.fatal("discriminator values can only be used with enum-like tag");
2147+
}
21092148
p.bump();
21102149
ret mk_item(p, lo, hi, id, ast::item_tag(variants, ty_params), attrs);
21112150
}

‎src/test/compile-fail/tag-variant-disr-dup.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//error-pattern:discriminator value already exists
2+
3+
// black and white have the same discriminator value ...
4+
5+
tag color {
6+
red = 0xff0000;
7+
green = 0x00ff00;
8+
blue = 0x0000ff;
9+
black = 0x000000;
10+
white = 0x000000;
11+
}

‎src/test/compile-fail/tag-variant-disr-non-nullary.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//error-pattern: discriminator values can only be used with enum-like tag
2+
// black and white have the same discriminator value ...
3+
4+
tag color {
5+
red = 0xff0000;
6+
green = 0x00ff00;
7+
blue = 0x0000ff;
8+
black = 0x000000;
9+
white = 0xffffff;
10+
other (str);
11+
}

‎src/test/run-pass/tag-variant-disr-val.rs

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
tag color {
2+
red = 0xff0000;
3+
green = 0x00ff00;
4+
blue = 0x0000ff;
5+
black = 0x000000;
6+
white = 0xFFFFFF;
7+
imaginary = -1;
8+
}
9+
10+
fn main() {
11+
test_color(red, 0xff0000, "red");
12+
test_color(green, 0x00ff00, "green");
13+
test_color(blue, 0x0000ff, "blue");
14+
test_color(black, 0x000000, "black");
15+
test_color(white, 0xFFFFFF, "white");
16+
test_color(imaginary, -1, "imaginary");
17+
}
18+
19+
fn test_color(color: color, val: int, name: str) unsafe {
20+
assert unsafe::reinterpret_cast(color) == val;
21+
assert get_color_alt(color) == name;
22+
assert get_color_if(color) == name;
23+
}
24+
25+
fn get_color_alt(color: color) -> str {
26+
alt color {
27+
red. {"red"}
28+
green. {"green"}
29+
blue. {"blue"}
30+
black. {"black"}
31+
white. {"white"}
32+
imaginary. {"imaginary"}
33+
_ {"unknown"}
34+
}
35+
}
36+
37+
fn get_color_if(color: color) -> str {
38+
if color == red {"red"}
39+
else if color == green {"green"}
40+
else if color == blue {"blue"}
41+
else if color == black {"black"}
42+
else if color == white {"white"}
43+
else if color == imaginary {"imaginary"}
44+
else {"unknown"}
45+
}
46+
47+

0 commit comments

Comments
 (0)