Merge pull request #873 from monaka/pr-cleanup-numeric-20130223

Clean numeric.c up.

src/numeric.c

@@ -25,8 +25,6 @@
 #include <ieeefp.h>
 #endif
 
-#define RSHIFT(x,y) ((x)>>(int)(y))
-
 #ifdef MRB_USE_FLOAT
 #define floor(f) floorf(f)
 #define ceil(f) ceilf(f)
@@ -184,6 +182,7 @@ flo_to_s(mrb_state *mrb, mrb_value flt)
 {
   char buf[32];
   int n;
+
   mrb_float value = mrb_float(flt);
 
   if (isinf(value)) {
@@ -243,13 +242,14 @@ flo_mul(mrb_state *mrb, mrb_value x)
 static void
 flodivmod(mrb_state *mrb, mrb_float x, mrb_float y, mrb_float *divp, mrb_float *modp)
 {
-  mrb_float div, mod;
+  mrb_float div;
+  mrb_float mod;
 
   if (y == 0.0) {
-    *divp = str_to_mrb_float("inf");
-    *modp = str_to_mrb_float("nan");
-    return;
+    div = str_to_mrb_float("inf");
+    mod = str_to_mrb_float("nan");
   }
+  else {
     mod = fmod(x, y);
     if (isinf(x) && !isinf(y) && !isnan(y))
       div = x;
@@ -259,6 +259,8 @@ flodivmod(mrb_state *mrb, mrb_float x, mrb_float y, mrb_float *divp, mrb_float *
       mod += y;
       div -= 1.0;
     }
+  }
+
   if (modp) *modp = mod;
   if (divp) *divp = div;
 }
@@ -280,6 +282,7 @@ flo_mod(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
   mrb_float fy, mod;
+
   mrb_get_args(mrb, "o", &y);
 
   fy = mrb_to_flo(mrb, y);
@@ -339,6 +342,7 @@ flo_eq(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
   volatile mrb_float a, b;
+
   mrb_get_args(mrb, "o", &y);
 
   switch (mrb_type(y)) {
@@ -727,6 +731,7 @@ static mrb_value
 fix_mul(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
+
   mrb_get_args(mrb, "o", &y);
   return mrb_fixnum_mul(mrb, x, y);
 }
@@ -736,6 +741,8 @@ fixdivmod(mrb_state *mrb, mrb_int x, mrb_int y, mrb_int *divp, mrb_int *modp)
 {
   mrb_int div, mod;
 
+  /* TODO: add assert(y != 0) to make sure */
+
   if (y < 0) {
     if (x < 0)
       div = -x / -y;
@@ -802,6 +809,7 @@ static mrb_value
 fix_divmod(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
+
   mrb_get_args(mrb, "o", &y);
 
   if (mrb_fixnum_p(y)) {
@@ -841,6 +849,7 @@ static mrb_value
 fix_equal(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
+
   mrb_get_args(mrb, "o", &y);
 
   if (mrb_obj_equal(mrb, x, y)) return mrb_true_value();
@@ -900,6 +909,7 @@ fix_and(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
   mrb_int val;
+
   mrb_get_args(mrb, "o", &y);
 
   y = bit_coerce(mrb, y);
@@ -920,6 +930,7 @@ fix_or(mrb_state *mrb, mrb_value x)
 {
   mrb_value y;
   mrb_int val;
+
   mrb_get_args(mrb, "o", &y);
 
   y = bit_coerce(mrb, y);
@@ -942,33 +953,53 @@ fix_xor(mrb_state *mrb, mrb_value x)
   mrb_int val;
 
   mrb_get_args(mrb, "o", &y);
+
   y = bit_coerce(mrb, y);
   val = mrb_fixnum(x) ^ mrb_fixnum(y);
   return mrb_fixnum_value(val);
 }
 
+#define NUMERIC_SHIFT_WIDTH_MAX		(sizeof(mrb_int)*CHAR_BIT-1)
+
 static mrb_value
 lshift(mrb_state *mrb, mrb_int val, int width)
 {
-  if (width > (sizeof(mrb_int)*CHAR_BIT-1)) {
+  if (width > NUMERIC_SHIFT_WIDTH_MAX) {
       mrb_raisef(mrb, E_RANGE_ERROR, "width(%d) > (%d:sizeof(mrb_int)*CHAR_BIT-1)", width,
-		sizeof(mrb_int)*CHAR_BIT-1);
+		NUMERIC_SHIFT_WIDTH_MAX);
   }
   val = val << width;
   return mrb_fixnum_value(val);
 }
 
 static mrb_value
-rshift(mrb_int val, int i)
+rshift(mrb_int val, int width)
 {
-    if (i >= sizeof(mrb_int)*CHAR_BIT-1) {
-        if (val < 0) return mrb_fixnum_value(-1);
-        return mrb_fixnum_value(0);
+  if (width >= NUMERIC_SHIFT_WIDTH_MAX) {
+    if (val < 0) {
+      val = -1;
+    }
+    else {
+      val = 0;
+    }
+  }
+  else {
+    val = val >> width;
   }
-    val = RSHIFT(val, i);
+
   return mrb_fixnum_value(val);
 }
 
+static inline void
+fix_shift_get_width(mrb_state *mrb, mrb_int *width)
+{
+  mrb_value y;
+
+  mrb_get_args(mrb, "o", &y);
+  y = bit_coerce(mrb, y);
+  *width = mrb_fixnum(y);
+}
+
 /* 15.2.8.3.12 */
 /*
  * call-seq:
@@ -980,16 +1011,27 @@ rshift(mrb_int val, int i)
 static mrb_value
 fix_lshift(mrb_state *mrb, mrb_value x)
 {
-  mrb_value y;
-  mrb_int val, width;
+  mrb_int width;
+  mrb_value result;
+
+  fix_shift_get_width(mrb, &width);
+
+  if (width == 0) {
+    result = x;
+  }
+  else {
+    mrb_int val;
 
-  mrb_get_args(mrb, "o", &y);
     val = mrb_fixnum(x);
-  y = bit_coerce(mrb, y);
-  width = mrb_fixnum(y);
-  if (width < 0)
-      return rshift(val, -width);
-  return lshift(mrb, val, width);
+    if (width < 0) {
+      result = rshift(val, -width);
+    }
+    else {
+      result = lshift(mrb, val, width);
+    }
+  }
+
+  return result;
 }
 
 /* 15.2.8.3.13 */
@@ -1003,17 +1045,27 @@ fix_lshift(mrb_state *mrb, mrb_value x)
 static mrb_value
 fix_rshift(mrb_state *mrb, mrb_value x)
 {
-  mrb_value y;
-  mrb_int i, val;
+  mrb_int width;
+  mrb_value result;
+
+  fix_shift_get_width(mrb, &width);
+
+  if (width == 0) {
+    result = x;
+  }
+  else {
+    mrb_int val;
 
-  mrb_get_args(mrb, "o", &y);
     val = mrb_fixnum(x);
-  y = bit_coerce(mrb, y);
-  i = mrb_fixnum(y);
-    if (i == 0) return x;
-    if (i < 0)
-        return lshift(mrb, val, -i);
-    return rshift(val, i);
+    if (width < 0) {
+      result = lshift(mrb, val, -width);
+    }
+    else {
+      result = rshift(val, width);
+    }
+  }
+
+  return result;
 }
 
 /* 15.2.8.3.23 */
@@ -1148,14 +1200,6 @@ fix_minus(mrb_state *mrb, mrb_value self)
   return mrb_fixnum_minus(mrb, self, other);
 }
 
-/* 15.2.8.3.29 (x) */
-/*
- * call-seq:
- *   fix > other     => true or false
- *
- * Returns <code>true</code> if the value of <code>fix</code> is
- * greater than that of <code>other</code>.
- */
 
 mrb_value
 mrb_fix2str(mrb_state *mrb, mrb_value x, int base)
@@ -1279,6 +1323,7 @@ void
 mrb_init_numeric(mrb_state *mrb)
 {
   struct RClass *numeric, *integer, *fixnum, *fl;
+
   /* Numeric Class */
   numeric = mrb_define_class(mrb, "Numeric",  mrb->object_class);
   mrb_include_module(mrb, numeric, mrb_class_get(mrb, "Comparable"));