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.

Author: kevina

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<int>], 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;
     }
 }
 

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<bool>) {
     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); }
     }
 }

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) {

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 one 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
                  }
             })
           }

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::<int>(),
+          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, 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);
       }

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<variant_>;
 

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 {

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; }

src/comp/syntax/parse/parser.rs

@@ -2023,15 +2023,13 @@ 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);
     }
     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 */ }
           _ {

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() {}