Re: Floating point modulus

[Peter Stephenson <p.stephenson@samsung.com>]

On Mon, 12 Jan 2015 09:56:28 +0000
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Sun, 11 Jan 2015 18:46:32 -0800
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > On Jan 11,  8:25pm, Peter Stephenson wrote:
> > }
> > } I'll leave that to you, but instead of an explicit rounding you could do
> > } basically the same calculation but assigned to a variable declared as an
> > } integer and output that.
> > 
> > Hm.  This seems like a bug:
> > 
> > % integer rnd
> > % print -- $(( rnd = ((29.1 % 13.0 * 10) + 0.5) ))
> > 31.500000000000014
> > 
> > Shouldn't the result of the $(( ... )) be an integer because rnd has been
> > declared as an integer?
>
> Whether or not it should be is another question.  Looking at the doc,
> with its references to C, you'd probably be entitled to expect assignment
> types propagated the way they do in C.

This appears to be straightforward.

It looks like we were actually assuming this behaviour in one existing
test, at least in the code, but then treating the result as floating
point anway.  Duh.

I've added some notes to the README about the change.

pws

diff --git a/README b/README
index e3ccc70..977f3fa 100644
--- a/README
+++ b/README
@@ -35,6 +35,54 @@ Zsh is a shell with lots of features.  For a list of some of these, see the
 file FEATURES, and for the latest changes see NEWS.  For more
 details, see the documentation.
 
+Incompatibilites between 5.0.7 and 5.0.8
+----------------------------------------
+
+A couple of arithmetic operations have changed: the new behaviour
+is intended to be more consistent, but is not compatible with the old.
+
+Previously, the modulus operation, `%', implicitly converted the
+operation to integer and output an integer result, even if one
+or both of the arguments were floating point.  Now, the C math
+library fmod() operator is used to implement the operation where
+one of the arguments is floating point.  For example:
+
+Old behavour:
+
+% print $(( 5.5 % 2 ))
+1
+
+New behaviour:
+
+% print $(( 5.5 % 2 ))
+1.5
+
+Previously, assignments to variables assigned the correct type to
+variables declared as floating point or integer, but this type was
+not propagated to the value of the expression, as a C programmer
+would naturally expect.  Now, the type of the variable is propagated
+so long as the variable is declared as a numeric type (however this
+happened, e.g. the variable may have been implicitly typed by a
+previous assignment).  For example:
+
+Old behaviour:
+
+% integer var
+% print $(( var = 5.5 / 2.0 ))
+2.2000000000000002
+% print $var
+2
+
+(the actual rounding error may vary).
+
+New behaviour:
+
+% integer var
+% print $(( var = 5.5 / 2.0 ))
+2
+% print $var
+2
+
 Incompatibilities between 5.0.2 and 5.0.5
 -----------------------------------------
 
diff --git a/Src/math.c b/Src/math.c
index 6d096e0..db42d0f 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -880,6 +880,8 @@ getcvar(char *s)
 static mnumber
 setmathvar(struct mathvalue *mvp, mnumber v)
 {
+    Param pm;
+
     if (mvp->pval) {
 	/*
 	 * This value may have been hanging around for a while.
@@ -909,7 +911,32 @@ setmathvar(struct mathvalue *mvp, mnumber v)
     if (noeval)
 	return v;
     untokenize(mvp->lval);
-    setnparam(mvp->lval, v);
+    pm = setnparam(mvp->lval, v);
+    if (pm) {
+	/*
+	 * If we are performing an assignment, we return the
+	 * number with the same type as the parameter we are
+	 * assigning to, in the spirit of the way assignments
+	 * in C work.  Note this was a change to long-standing
+	 * zsh behaviour.
+	 */
+	switch (PM_TYPE(pm->node.flags)) {
+	case PM_INTEGER:
+	    if (v.type != MN_INTEGER) {
+		v.u.l = (zlong)v.u.d;
+		v.type = MN_INTEGER;
+	    }
+	    break;
+
+	case PM_EFLOAT:
+	case PM_FFLOAT:
+	    if (v.type != MN_FLOAT) {
+		v.u.d = (double)v.u.l;
+		v.type = MN_FLOAT;
+	    }
+	    break;
+	}
+    }
     return v;
 }
 
diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst
index 8da17f7..8e0730d 100644
--- a/Test/C01arith.ztst
+++ b/Test/C01arith.ztst
@@ -16,7 +16,7 @@
   print -- $(( rnd = there * 10000 ))
 # save rounding problems by converting to integer
 0:basic floating point arithmetic
->31415.
+>31415
 
   integer rnd
   (( rnd = ((29.1 % 13.0 * 10) + 0.5) ))
@@ -300,3 +300,11 @@
   print $(( 0b2 ))
 1:Binary numbers don't tend to have 2's in
 ?(eval):1: bad math expression: operator expected at `2 '
+
+  integer varassi
+  print $(( varassi = 5.5 / 2.0 ))
+  print $varassi
+0:Integer variable assignment converts result to integer
+>2
+>2
+# It's hard to test for integer to float.