Allow enum discriminator exprs to refer to declared consts

Also some work towards #3521

Closes #2428

Author: catamorphism

src/libsyntax/visit.rs

@@ -182,6 +182,8 @@ fn visit_enum_def<E>(enum_definition: ast::enum_def, tps: ~[ast::ty_param],
                 visit_enum_def(enum_definition, tps, e, v);
             }
         }
+        // Visit the disr expr if it exists
+        vr.node.disr_expr.iter(|ex| v.visit_expr(*ex, e, v));
     }
 }
 

src/rustc/middle/const_eval.rs

@@ -1,4 +1,4 @@
-use syntax::{ast,ast_util,visit};
+use syntax::{ast,ast_map,ast_util,visit};
 use ast::*;
 
 //
@@ -135,28 +135,7 @@ fn classify(e: @expr,
               // FIXME: (#3728) we can probably do something CCI-ish
               // surrounding nonlocal constants. But we don't yet.
               ast::expr_path(_) => {
-                match def_map.find(e.id) {
-                  Some(ast::def_const(def_id)) => {
-                    if ast_util::is_local(def_id) {
-                        let ty = ty::expr_ty(tcx, e);
-                        if ty::type_is_integral(ty) {
-                            integral_const
-                        } else {
-                            general_const
-                        }
-                    } else {
-                        non_const
-                    }
-                  }
-                  Some(_) => {
-                    non_const
-                  }
-                  None => {
-                    tcx.sess.span_bug(e.span,
-                                      ~"unknown path when \
-                                        classifying constants");
-                  }
-                }
+                lookup_constness(tcx, e)
               }
 
               _ => non_const
@@ -167,6 +146,40 @@ fn classify(e: @expr,
     }
 }
 
+fn lookup_const(tcx: ty::ctxt, e: @expr) -> Option<@expr> {
+    match tcx.def_map.find(e.id) {
+        Some(ast::def_const(def_id)) => {
+            if ast_util::is_local(def_id) {
+                match tcx.items.find(def_id.node) {
+                    None => None,
+                    Some(ast_map::node_item(it, _)) => match it.node {
+                        item_const(_, const_expr) => Some(const_expr),
+                        _ => None
+                    },
+                    Some(_) => None
+                }
+            }
+            else { None }
+        }
+        Some(_) => None,
+        None => None
+    }
+}
+
+fn lookup_constness(tcx: ty::ctxt, e: @expr) -> constness {
+    match lookup_const(tcx, e) {
+        Some(rhs) => {
+            let ty = ty::expr_ty(tcx, rhs);
+            if ty::type_is_integral(ty) {
+                integral_const
+            } else {
+                general_const
+            }
+        }
+        None => non_const
+    }
+}
+
 fn process_crate(crate: @ast::crate,
                  def_map: resolve::DefMap,
                  tcx: ty::ctxt) {
@@ -204,58 +217,67 @@ impl const_val : cmp::Eq {
     pure fn ne(other: &const_val) -> bool { !self.eq(other) }
 }
 
-// FIXME: issue #1417
 fn eval_const_expr(tcx: middle::ty::ctxt, e: @expr) -> const_val {
+    match eval_const_expr_partial(tcx, e) {
+        Ok(r) => r,
+        Err(s) => fail s
+    }
+}
+
+fn eval_const_expr_partial(tcx: middle::ty::ctxt, e: @expr)
+    -> Result<const_val, ~str> {
     use middle::ty;
-    fn fromb(b: bool) -> const_val { const_int(b as i64) }
+    fn fromb(b: bool) -> Result<const_val, ~str> { Ok(const_int(b as i64)) }
     match e.node {
       expr_unary(neg, inner) => {
-        match eval_const_expr(tcx, inner) {
-          const_float(f) => const_float(-f),
-          const_int(i) => const_int(-i),
-          const_uint(i) => const_uint(-i),
-          const_str(_) => fail ~"Negate on string",
-          const_bool(_) => fail ~"Negate on boolean"
+        match eval_const_expr_partial(tcx, inner) {
+          Ok(const_float(f)) => Ok(const_float(-f)),
+          Ok(const_int(i)) => Ok(const_int(-i)),
+          Ok(const_uint(i)) => Ok(const_uint(-i)),
+          Ok(const_str(_)) => Err(~"Negate on string"),
+          Ok(const_bool(_)) => Err(~"Negate on boolean"),
+          err => err
         }
       }
       expr_unary(not, inner) => {
-        match eval_const_expr(tcx, inner) {
-          const_int(i) => const_int(!i),
-          const_uint(i) => const_uint(!i),
-          const_bool(b) => const_bool(!b),
-          _ => fail ~"Not on float or string"
+        match eval_const_expr_partial(tcx, inner) {
+          Ok(const_int(i)) => Ok(const_int(!i)),
+          Ok(const_uint(i)) => Ok(const_uint(!i)),
+          Ok(const_bool(b)) => Ok(const_bool(!b)),
+          _ => Err(~"Not on float or string")
         }
       }
       expr_binary(op, a, b) => {
-        match (eval_const_expr(tcx, a), eval_const_expr(tcx, b)) {
-          (const_float(a), const_float(b)) => {
+        match (eval_const_expr_partial(tcx, a),
+               eval_const_expr_partial(tcx, b)) {
+          (Ok(const_float(a)), Ok(const_float(b))) => {
             match op {
-              add => const_float(a + b),
-              subtract => const_float(a - b),
-              mul => const_float(a * b),
-              div => const_float(a / b),
-              rem => const_float(a % b),
+              add => Ok(const_float(a + b)),
+              subtract => Ok(const_float(a - b)),
+              mul => Ok(const_float(a * b)),
+              div => Ok(const_float(a / b)),
+              rem => Ok(const_float(a % b)),
               eq => fromb(a == b),
               lt => fromb(a < b),
               le => fromb(a <= b),
               ne => fromb(a != b),
               ge => fromb(a >= b),
               gt => fromb(a > b),
-              _ => fail ~"Can't do this op on floats"
+              _ => Err(~"Can't do this op on floats")
             }
           }
-          (const_int(a), const_int(b)) => {
+          (Ok(const_int(a)), Ok(const_int(b))) => {
             match op {
-              add => const_int(a + b),
-              subtract => const_int(a - b),
-              mul => const_int(a * b),
-              div => const_int(a / b),
-              rem => const_int(a % b),
-              and | bitand => const_int(a & b),
-              or | bitor => const_int(a | b),
-              bitxor => const_int(a ^ b),
-              shl => const_int(a << b),
-              shr => const_int(a >> b),
+              add => Ok(const_int(a + b)),
+              subtract => Ok(const_int(a - b)),
+              mul => Ok(const_int(a * b)),
+              div => Ok(const_int(a / b)),
+              rem => Ok(const_int(a % b)),
+              and | bitand => Ok(const_int(a & b)),
+              or | bitor => Ok(const_int(a | b)),
+              bitxor => Ok(const_int(a ^ b)),
+              shl => Ok(const_int(a << b)),
+              shr => Ok(const_int(a >> b)),
               eq => fromb(a == b),
               lt => fromb(a < b),
               le => fromb(a <= b),
@@ -264,18 +286,18 @@ fn eval_const_expr(tcx: middle::ty::ctxt, e: @expr) -> const_val {
               gt => fromb(a > b)
             }
           }
-          (const_uint(a), const_uint(b)) => {
+          (Ok(const_uint(a)), Ok(const_uint(b))) => {
             match op {
-              add => const_uint(a + b),
-              subtract => const_uint(a - b),
-              mul => const_uint(a * b),
-              div => const_uint(a / b),
-              rem => const_uint(a % b),
-              and | bitand => const_uint(a & b),
-              or | bitor => const_uint(a | b),
-              bitxor => const_uint(a ^ b),
-              shl => const_uint(a << b),
-              shr => const_uint(a >> b),
+              add => Ok(const_uint(a + b)),
+              subtract => Ok(const_uint(a - b)),
+              mul => Ok(const_uint(a * b)),
+              div => Ok(const_uint(a / b)),
+              rem => Ok(const_uint(a % b)),
+              and | bitand => Ok(const_uint(a & b)),
+              or | bitor => Ok(const_uint(a | b)),
+              bitxor => Ok(const_uint(a ^ b)),
+              shl => Ok(const_uint(a << b)),
+              shr => Ok(const_uint(a >> b)),
               eq => fromb(a == b),
               lt => fromb(a < b),
               le => fromb(a <= b),
@@ -285,70 +307,76 @@ fn eval_const_expr(tcx: middle::ty::ctxt, e: @expr) -> const_val {
             }
           }
           // shifts can have any integral type as their rhs
-          (const_int(a), const_uint(b)) => {
+          (Ok(const_int(a)), Ok(const_uint(b))) => {
             match op {
-              shl => const_int(a << b),
-              shr => const_int(a >> b),
-              _ => fail ~"Can't do this op on an int and uint"
+              shl => Ok(const_int(a << b)),
+              shr => Ok(const_int(a >> b)),
+              _ => Err(~"Can't do this op on an int and uint")
             }
           }
-          (const_uint(a), const_int(b)) => {
+          (Ok(const_uint(a)), Ok(const_int(b))) => {
             match op {
-              shl => const_uint(a << b),
-              shr => const_uint(a >> b),
-              _ => fail ~"Can't do this op on a uint and int"
+              shl => Ok(const_uint(a << b)),
+              shr => Ok(const_uint(a >> b)),
+              _ => Err(~"Can't do this op on a uint and int")
             }
           }
-          (const_bool(a), const_bool(b)) => {
-            const_bool(match op {
+          (Ok(const_bool(a)), Ok(const_bool(b))) => {
+            Ok(const_bool(match op {
               and => a && b,
               or => a || b,
               bitxor => a ^ b,
               bitand => a & b,
               bitor => a | b,
               eq => a == b,
               ne => a != b,
-              _ => fail ~"Can't do this op on bools"
-             })
+              _ => return Err(~"Can't do this op on bools")
+             }))
           }
-          _ => fail ~"Bad operands for binary"
+          _ => Err(~"Bad operands for binary")
         }
       }
       expr_cast(base, _) => {
         let ety = ty::expr_ty(tcx, e);
-        let base = eval_const_expr(tcx, base);
+        let base = eval_const_expr_partial(tcx, base);
         match ty::get(ety).sty {
           ty::ty_float(_) => {
             match base {
-              const_uint(u) => const_float(u as f64),
-              const_int(i) => const_float(i as f64),
-              const_float(_) => base,
-              _ => fail ~"Can't cast float to str"
+              Ok(const_uint(u)) => Ok(const_float(u as f64)),
+              Ok(const_int(i)) => Ok(const_float(i as f64)),
+              Ok(const_float(_)) => base,
+              _ => Err(~"Can't cast float to str")
             }
           }
           ty::ty_uint(_) => {
             match base {
-              const_uint(_) => base,
-              const_int(i) => const_uint(i as u64),
-              const_float(f) => const_uint(f as u64),
-              _ => fail ~"Can't cast str to uint"
+              Ok(const_uint(_)) => base,
+              Ok(const_int(i)) => Ok(const_uint(i as u64)),
+              Ok(const_float(f)) => Ok(const_uint(f as u64)),
+              _ => Err(~"Can't cast str to uint")
             }
           }
           ty::ty_int(_) | ty::ty_bool => {
             match base {
-              const_uint(u) => const_int(u as i64),
-              const_int(_) => base,
-              const_float(f) => const_int(f as i64),
-              _ => fail ~"Can't cast str to int"
+              Ok(const_uint(u)) => Ok(const_int(u as i64)),
+              Ok(const_int(_)) => base,
+              Ok(const_float(f)) => Ok(const_int(f as i64)),
+              _ => Err(~"Can't cast str to int")
             }
           }
-          _ => fail ~"Can't cast this type"
+          _ => Err(~"Can't cast this type")
         }
       }
-      expr_lit(lit) => lit_to_const(lit),
+      expr_path(_) => {
+          match lookup_const(tcx, e) {
+              Some(actual_e) => eval_const_expr_partial(tcx, actual_e),
+              None => Err(~"Non-constant path in constant expr")
+          }
+      }
+      expr_lit(lit) => Ok(lit_to_const(lit)),
       // If we have a vstore, just keep going; it has to be a string
-      expr_vstore(e, _) => eval_const_expr(tcx, e),
-      _ => fail ~"Unsupported constant expr"
+      expr_vstore(e, _) => eval_const_expr_partial(tcx, e),
+      _ => Err(~"Unsupported constant expr")
     }
 }
 

src/rustc/middle/resolve.rs

@@ -257,7 +257,10 @@ enum RibKind {
     MethodRibKind(node_id, MethodSort),
 
     // We passed through a function *item* scope. Disallow upvars.
-    OpaqueFunctionRibKind
+    OpaqueFunctionRibKind,
+
+    // We're in a constant item. Can't refer to dynamic stuff.
+    ConstantItemRibKind
 }
 
 // Methods can be required or provided. Required methods only occur in traits.
@@ -3114,9 +3117,16 @@ impl Resolver {
 
                     return None;
                 }
+                ConstantItemRibKind => {
+                    // Still doesn't deal with upvars
+                    self.session.span_err(span,
+                                          ~"attempt to use a non-constant \
+                                            value in a constant");
+
+                }
             }
 
-            rib_index += 1u;
+            rib_index += 1;
         }
 
         return Some(dl_def(def));
@@ -3130,8 +3140,8 @@ impl Resolver {
         // XXX: Try caching?
 
         let mut i = (*ribs).len();
-        while i != 0u {
-            i -= 1u;
+        while i != 0 {
+            i -= 1;
             let rib = (*ribs).get_elt(i);
             match rib.bindings.find(name) {
                 Some(def_like) => {
@@ -3179,7 +3189,33 @@ impl Resolver {
         }
 
         match item.node {
-            item_enum(_, type_parameters) |
+
+            // enum item: resolve all the variants' discrs,
+            // then resolve the ty params
+            item_enum(enum_def, type_parameters) => {
+
+                for enum_def.variants.each() |variant| {
+                    do variant.node.disr_expr.iter() |dis_expr| {
+                        // resolve the discriminator expr
+                        // as a constant
+                        self.with_constant_rib(|| {
+                            self.resolve_expr(*dis_expr, visitor); 
+                        });
+                    }
+                }
+
+                // n.b. the discr expr gets visted twice.
+                // but maybe it's okay since the first time will signal an
+                // error if there is one? -- tjc
+                do self.with_type_parameter_rib
+                        (HasTypeParameters(&type_parameters, item.id, 0,
+                                           NormalRibKind))
+                        || {
+
+                    visit_item(item, (), visitor);
+                }
+            }
+                
             item_ty(_, type_parameters) => {
                 do self.with_type_parameter_rib
                         (HasTypeParameters(&type_parameters, item.id, 0u,
@@ -3344,7 +3380,9 @@ impl Resolver {
             }
 
             item_const(*) => {
-                visit_item(item, (), visitor);
+                self.with_constant_rib(|| {
+                    visit_item(item, (), visitor);
+                });
             }
 
           item_mac(*) => {
@@ -3401,6 +3439,12 @@ impl Resolver {
         f();
         (*self.label_ribs).pop();
     }
+    fn with_constant_rib(f: fn()) {
+        (*self.value_ribs).push(@Rib(ConstantItemRibKind));
+        f();
+        (*self.value_ribs).pop();
+    }
+
 
     fn resolve_function(rib_kind: RibKind,
                         optional_declaration: Option<@fn_decl>,
@@ -4127,7 +4171,7 @@ impl Resolver {
                                                  namespace);
         }
 
-        if path.idents.len() > 1u {
+        if path.idents.len() > 1 {
             return self.resolve_module_relative_path(path,
                                                   self.xray_context,
                                                   namespace);

src/rustc/middle/typeck/check.rs

@@ -2219,9 +2219,14 @@ fn check_block(fcx0: @fn_ctxt, blk: ast::blk) -> bool {
 fn check_const(ccx: @crate_ctxt, _sp: span, e: @ast::expr, id: ast::node_id) {
     let rty = ty::node_id_to_type(ccx.tcx, id);
     let fcx = blank_fn_ctxt(ccx, rty, e.id);
+    let declty = fcx.ccx.tcx.tcache.get(local_def(id)).ty;
+    check_const_with_ty(fcx, _sp, e, declty);
+}
+
+fn check_const_with_ty(fcx: @fn_ctxt, _sp: span, e: @ast::expr,
+                       declty: ty::t) {
     check_expr(fcx, e, None);
     let cty = fcx.expr_ty(e);
-    let declty = fcx.ccx.tcx.tcache.get(local_def(id)).ty;
     demand::suptype(fcx, e.span, declty, cty);
     regionck::regionck_expr(fcx, e);
     writeback::resolve_type_vars_in_expr(fcx, e);
@@ -2259,27 +2264,31 @@ fn check_enum_variants(ccx: @crate_ctxt,
                 variants: &mut ~[ty::variant_info]) {
         let rty = ty::node_id_to_type(ccx.tcx, id);
         for vs.each |v| {
-            match v.node.disr_expr {
-              Some(e) => {
-                let fcx = blank_fn_ctxt(ccx, rty, e.id);
-                check_expr(fcx, e, None);
-                let cty = fcx.expr_ty(e);
+            do v.node.disr_expr.iter |e_ref| {
+                let e = *e_ref;
+                debug!("disr expr, checking %s",
+                       expr_to_str(e, ccx.tcx.sess.intr()));
                 let declty = ty::mk_int(ccx.tcx);
-                demand::suptype(fcx, e.span, declty, cty);
+                let fcx = blank_fn_ctxt(ccx, rty, e.id);
+                check_const_with_ty(fcx, e.span, e, declty);
                 // check_expr (from check_const pass) doesn't guarantee
                 // that the expression is in an form that eval_const_expr can
                 // handle, so we may still get an internal compiler error
-                match const_eval::eval_const_expr(ccx.tcx, e) {
-                  const_eval::const_int(val) => {
+
+                match const_eval::eval_const_expr_partial(ccx.tcx, e) {
+                  Ok(const_eval::const_int(val)) => {
                     *disr_val = val as int;
                   }
-                  _ => {
+                  Ok(_) => {
                     ccx.tcx.sess.span_err(e.span, ~"expected signed integer \
                                                     constant");
                   }
+                  Err(err) => {
+                    ccx.tcx.sess.span_err(e.span,
+                     #fmt("expected constant: %s", err));
+
+                  }
                 }
-              }
-              _ => ()
             }
             if vec::contains(*disr_vals, &*disr_val) {
                 ccx.tcx.sess.span_err(v.span,

src/test/compile-fail/issue-3521-2.rs

@@ -0,0 +1,7 @@
+fn main() {
+    let foo = 100;
+
+    const y: int = foo + 1; //~ ERROR: attempt to use a non-constant value in a constant
+
+    log(error, y);
+}

src/test/run-pass/issue-2428.rs

@@ -0,0 +1,10 @@
+fn main() {
+    let foo = 100;
+    const quux: int = 5;
+
+    enum Stuff {
+        Bar = quux
+    }
+
+    assert (Bar as int == quux);
+}