Support explicit discriminant numbers on tag variants.

Addresses issue #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.
rust-lang · Jan 10, 2012 · 08abf8d · 08abf8d
1 parent d0fe672
commit 08abf8d
Show file tree

Hide file tree

Showing 12 changed files with 167 additions and 30 deletions.
diff --git a/src/comp/metadata/common.rs b/src/comp/metadata/common.rs
@@ -70,6 +70,9 @@ const tag_item_method: uint = 0x31u;
 const tag_impl_iface: uint = 0x32u;
 const tag_impl_iface_did: uint = 0x33u;
 
+// discriminator value for variants
+const tag_disr_val: uint = 0x34u;
+
 // djb's cdb hashes.
 fn hash_node_id(&&node_id: int) -> uint { ret 177573u ^ (node_id as uint); }
 

diff --git a/src/comp/metadata/decoder.rs b/src/comp/metadata/decoder.rs
@@ -88,6 +88,17 @@ fn variant_tag_id(d: ebml::doc) -> ast::def_id {
     ret parse_def_id(ebml::doc_data(tagdoc));
 }
 
+fn variant_disr_val(d: ebml::doc) -> option::t<int> {
+    alt ebml::maybe_get_doc(d, tag_disr_val) {
+      some(val_doc) {
+        let val_buf = ebml::doc_data(val_doc);
+        let val = int::parse_buf(val_buf, 10u);
+        ret some(val);
+      }
+      _ { ret none;}
+    }
+}
+
 fn doc_type(doc: ebml::doc, tcx: ty::ctxt, cdata: cmd) -> ty::t {
     let tp = ebml::get_doc(doc, tag_items_data_item_type);
     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)
     let item = find_item(id, items);
     let infos: [ty::variant_info] = [];
     let variant_ids = tag_variant_ids(item, cdata);
+    let disr_val = 0;
     for did: ast::def_id in variant_ids {
         let item = find_item(did.node, items);
         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)
           }
           _ { /* Nullary tag variant. */ }
         }
-        infos += [@{args: arg_tys, ctor_ty: ctor_ty, id: did}];
+        alt variant_disr_val(item) {
+          some(val) { disr_val = val; }
+          _         { /* empty */ }
+        }
+        infos += [@{args: arg_tys, ctor_ty: ctor_ty, id: did,
+                    disr_val: disr_val}];
+        disr_val += 1;
     }
     ret infos;
 }

diff --git a/src/comp/metadata/encoder.rs b/src/comp/metadata/encoder.rs
@@ -228,6 +228,12 @@ fn encode_discriminant(ecx: @encode_ctxt, ebml_w: ebml::writer, id: node_id) {
     ebml::end_tag(ebml_w);
 }
 
+fn encode_disr_val(_ecx: @encode_ctxt, ebml_w: ebml::writer, disr_val: int) {
+    ebml::start_tag(ebml_w, tag_disr_val);
+    ebml_w.writer.write(str::bytes(int::to_str(disr_val,10u)));
+    ebml::end_tag(ebml_w);
+}
+
 fn encode_tag_id(ebml_w: ebml::writer, id: def_id) {
     ebml::start_tag(ebml_w, tag_items_data_item_tag_id);
     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) {
 fn encode_tag_variant_info(ecx: @encode_ctxt, ebml_w: ebml::writer,
                            id: node_id, variants: [variant],
                            &index: [entry<int>], ty_params: [ty_param]) {
+    let disr_val = 0;
     for variant: variant in variants {
         index += [{val: variant.node.id, pos: ebml_w.writer.tell()}];
         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,
             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;
+        }
         encode_type_param_bounds(ebml_w, ecx, ty_params);
         ebml::end_tag(ebml_w);
+        disr_val += 1;
     }
 }
 

diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs
@@ -1726,17 +1726,15 @@ fn iter_structural_ty(cx: @block_ctxt, av: ValueRef, t: ty::t,
         Unreachable(unr_cx);
         let llswitch = Switch(cx, lldiscrim_a, unr_cx.llbb, n_variants);
         let next_cx = new_sub_block_ctxt(cx, "tag-iter-next");
-        let i = 0u;
         for variant: ty::variant_info in *variants {
             let variant_cx =
                 new_sub_block_ctxt(cx,
                                    "tag-iter-variant-" +
-                                       uint::to_str(i, 10u));
-            AddCase(llswitch, C_int(ccx, i as int), variant_cx.llbb);
+                                       int::to_str(variant.disr_val, 10u));
+            AddCase(llswitch, C_int(ccx, variant.disr_val), variant_cx.llbb);
             variant_cx =
                 iter_variant(variant_cx, llunion_a_ptr, variant, tps, tid, f);
             Br(variant_cx, next_cx.llbb);
-            i += 1u;
         }
         ret next_cx;
       }
@@ -2745,12 +2743,9 @@ fn trans_var(cx: @block_ctxt, sp: span, def: ast::def, id: ast::node_id)
             let bcx = alloc_result.bcx;
             let lltagptr = PointerCast(bcx, lltagblob, T_ptr(lltagty));
             let lldiscrimptr = GEPi(bcx, lltagptr, [0, 0]);
-            let d = if vec::len(*ty::tag_variants(ccx.tcx, tid)) != 1u {
-                let lldiscrim_gv = lookup_discriminant(bcx.fcx.lcx, vid);
-                let lldiscrim = Load(bcx, lldiscrim_gv);
-                lldiscrim
-            } else { C_int(ccx, 0) };
-            Store(bcx, d, lldiscrimptr);
+            let lldiscrim_gv = lookup_discriminant(bcx.fcx.lcx, vid);
+            let lldiscrim = Load(bcx, lldiscrim_gv);
+            Store(bcx, lldiscrim, lldiscrimptr);
             ret lval_no_env(bcx, lltagptr, temporary);
         }
       }
@@ -4685,7 +4680,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, index: int, is_degen: bool,
+                     variant: ast::variant, is_degen: bool,
                      ty_params: [ast::ty_param]) {
     let ccx = cx.ccx;
 
@@ -4735,7 +4730,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, index), lldiscrimptr);
+            Store(bcx, C_int(ccx, variant.node.disr_val), lldiscrimptr);
             GEPi(bcx, lltagptr, [0, 1])
         };
     i = 0u;
@@ -5086,10 +5081,8 @@ 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 i = 0;
         for variant: ast::variant in variants {
-            trans_tag_variant(sub_cx, item.id, variant, i, degen, tps);
-            i += 1;
+            trans_tag_variant(sub_cx, item.id, variant, degen, tps);
         }
       }
       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],
     visit::visit_item(it, new_pt, v);
     alt it.node {
       ast::item_tag(variants, _) {
-        let i = 0u;
         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 discrim_gvar = str::as_buf(s, {|buf|
                 llvm::LLVMAddGlobal(ccx.llmod, ccx.int_type, buf)
             });
-            llvm::LLVMSetInitializer(discrim_gvar, C_int(ccx, i as int));
+            llvm::LLVMSetInitializer(discrim_gvar, C_int(ccx, disr_val));
             llvm::LLVMSetGlobalConstant(discrim_gvar, True);
             ccx.discrims.insert(
                 ast_util::local_def(variant.node.id), discrim_gvar);
             ccx.discrim_symbols.insert(variant.node.id, s);
-            i += 1u;
         }
       }
       ast::item_impl(tps, some(@{node: ast::ty_path(_, id), _}), _, ms) {

diff --git a/src/comp/middle/trans_alt.rs b/src/comp/middle/trans_alt.rs
@@ -16,7 +16,7 @@ import trans_common::*;
 // An option identifying a branch (either a literal, a tag variant or a range)
 tag opt {
     lit(@ast::expr);
-    var(/* variant id */uint, /* variant dids */{tg: def_id, var: def_id});
+    var(/* disr val */int, /* variant dids */{tg: def_id, var: def_id});
     range(@ast::expr, @ast::expr);
 }
 fn opt_eq(a: opt, b: opt) -> bool {
@@ -53,7 +53,7 @@ fn trans_opt(bcx: @block_ctxt, o: opt) -> opt_result {
           }
         }
       }
-      var(id, _) { ret single_result(rslt(bcx, C_int(ccx, id as int))); }
+      var(disr_val, _) { ret single_result(rslt(bcx, C_int(ccx, disr_val))); }
       range(l1, l2) {
         ret range_result(rslt(bcx, trans::trans_const_expr(ccx, l1)),
                          rslt(bcx, trans::trans_const_expr(ccx, l2)));
@@ -64,10 +64,8 @@ fn trans_opt(bcx: @block_ctxt, o: opt) -> opt_result {
 fn variant_opt(ccx: @crate_ctxt, pat_id: ast::node_id) -> opt {
     let vdef = ast_util::variant_def_ids(ccx.tcx.def_map.get(pat_id));
     let variants = ty::tag_variants(ccx.tcx, vdef.tg);
-    let i = 0u;
     for v: ty::variant_info in *variants {
-        if vdef.var == v.id { ret var(i, vdef); }
-        i += 1u;
+        if vdef.var == v.id { ret var(v.disr_val, vdef); }
     }
     fail;
 }

diff --git a/src/comp/middle/ty.rs b/src/comp/middle/ty.rs
@@ -2686,7 +2686,8 @@ fn impl_iface(cx: ctxt, id: ast::def_id) -> option::t<t> {
 }
 
 // Tag information
-type variant_info = @{args: [ty::t], ctor_ty: ty::t, id: ast::def_id};
+type variant_info = @{args: [ty::t], ctor_ty: ty::t, id: ast::def_id,
+                      disr_val: int};
 
 fn tag_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] {
     alt cx.tag_var_cache.find(id) {
@@ -2705,7 +2706,9 @@ fn tag_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] {
                 } else { [] };
                 @{args: arg_tys,
                   ctor_ty: ctor_ty,
-                  id: ast_util::local_def(variant.node.id)}
+                  id: ast_util::local_def(variant.node.id),
+                  disr_val: variant.node.disr_val
+                 }
             })
           }
         }

diff --git a/src/comp/syntax/ast.rs b/src/comp/syntax/ast.rs
@@ -416,7 +416,8 @@ type native_mod =
 
 type variant_arg = {ty: @ty, id: node_id};
 
-type variant_ = {name: ident, args: [variant_arg], id: node_id};
+type variant_ = {name: ident, args: [variant_arg], id: node_id,
+                disr_val: int, disr_expr: option::t<@expr>};
 
 type variant = spanned<variant_>;
 

diff --git a/src/comp/syntax/fold.rs b/src/comp/syntax/fold.rs
@@ -469,7 +469,9 @@ fn noop_fold_variant(v: variant_, fld: ast_fold) -> variant_ {
         ret {ty: fld.fold_ty(va.ty), id: va.id};
     }
     let fold_variant_arg = bind fold_variant_arg_(_, fld);
-    ret {name: v.name, args: vec::map(v.args, fold_variant_arg), id: v.id};
+    ret {name: v.name, args: vec::map(v.args, fold_variant_arg), id: v.id,
+         disr_val: v.disr_val,  disr_expr: v.disr_expr
+         /* FIXME: is this right (copying disr_val and disr_expr) */};
 }
 
 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
@@ -2072,11 +2072,16 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
             spanned(ty.span.lo, ty.span.hi,
                     {name: id,
                      args: [{ty: ty, id: p.get_id()}],
-                     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);
     }
     expect(p, token::LBRACE);
+    let all_nullary = true;
+    let have_disr = false;
+    let disr_val = 0;
     while p.peek() != token::RBRACE {
         let tok = p.peek();
         alt tok {
@@ -2086,8 +2091,10 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
             p.bump();
             let args: [ast::variant_arg] = [];
             let vhi = p.get_hi_pos();
+            let disr_expr = none;
             alt p.peek() {
               token::LPAREN. {
+                all_nullary = false;
                 let arg_tys = parse_seq(token::LPAREN, token::RPAREN,
                                         seq_sep(token::COMMA),
                                         {|p| parse_ty(p, false)}, p);
@@ -2096,12 +2103,41 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
                 }
                 vhi = arg_tys.span.hi;
               }
+              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) {
+                    disr_val = val as int;
+                    // FIXME: check that value is in range
+                  }
+                }
+                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);
+              }
               _ {/* empty */ }
             }
             expect(p, token::SEMI);
             p.get_id();
-            let vr = {name: p.get_str(name), args: args, id: p.get_id()};
+            let vr = {name: p.get_str(name), args: args, id: p.get_id(),
+                      disr_val: disr_val, disr_expr: disr_expr};
             variants += [spanned(vlo, vhi, vr)];
+            disr_val += 1;
           }
           token::RBRACE. {/* empty */ }
           _ {
@@ -2111,6 +2147,9 @@ fn parse_item_tag(p: parser, attrs: [ast::attribute]) -> @ast::item {
         }
     }
     let hi = p.get_hi_pos();
+    if (have_disr && !all_nullary) {
+        p.fatal("discriminator values can only be used with enum-like tag");
+    }
     p.bump();
     ret mk_item(p, lo, hi, id, ast::item_tag(variants, ty_params), attrs);
 }

diff --git a/src/test/compile-fail/tag-variant-disr-dup.rs b/src/test/compile-fail/tag-variant-disr-dup.rs
@@ -0,0 +1,11 @@
+//error-pattern:discriminator value already exists
+
+// black and white have the same discriminator value ...
+
+tag color {
+    red = 0xff0000;
+    green = 0x00ff00;
+    blue = 0x0000ff;
+    black = 0x000000;
+    white = 0x000000;
+}
diff --git a/src/test/compile-fail/tag-variant-disr-non-nullary.rs b/src/test/compile-fail/tag-variant-disr-non-nullary.rs
@@ -0,0 +1,11 @@
+//error-pattern: discriminator values can only be used with enum-like tag
+// black and white have the same discriminator value ...
+
+tag color {
+    red = 0xff0000;
+    green = 0x00ff00;
+    blue = 0x0000ff;
+    black = 0x000000;
+    white = 0xffffff;
+    other (str);
+}
diff --git a/src/test/run-pass/tag-variant-disr-val.rs b/src/test/run-pass/tag-variant-disr-val.rs
@@ -0,0 +1,47 @@
+tag color {
+    red = 0xff0000;
+    green = 0x00ff00;
+    blue = 0x0000ff;
+    black = 0x000000;
+    white = 0xFFFFFF;
+    imaginary = -1;
+}
+
+fn main() {
+    test_color(red, 0xff0000, "red");
+    test_color(green, 0x00ff00, "green");
+    test_color(blue, 0x0000ff, "blue");
+    test_color(black, 0x000000, "black");
+    test_color(white, 0xFFFFFF, "white");
+    test_color(imaginary, -1, "imaginary");
+}
+
+fn test_color(color: color, val: int, name: str) unsafe {
+    assert unsafe::reinterpret_cast(color) == val;
+    assert get_color_alt(color) == name;
+    assert get_color_if(color) == name;
+}
+
+fn get_color_alt(color: color) -> str {
+    alt color {
+      red. {"red"}
+      green. {"green"}
+      blue. {"blue"}
+      black. {"black"}
+      white. {"white"}
+      imaginary. {"imaginary"}
+      _ {"unknown"}
+    }
+}
+
+fn get_color_if(color: color) -> str {
+    if color == red {"red"}
+    else if color == green {"green"}
+    else if color == blue {"blue"}
+    else if color == black {"black"}
+    else if color == white {"white"}
+    else if color == imaginary {"imaginary"}
+    else {"unknown"}
+}
+
+