zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <okiddle@yahoo.co.uk>
To: zsh-workers@zsh.org
Subject: Re: inf and nan in arithmetic expansions
Date: Thu, 22 Mar 2018 00:46:28 +0100	[thread overview]
Message-ID: <25305.1521675988@thecus> (raw)
In-Reply-To: <17756.1518799875@thecus.kiddle.eu>

On 16 Feb, I wrote:
> It definitely needs more work. In particular, the code for checking
> "NaN" and "Inf" in math context should use strncmp with 3 as the length
> otherwise it only accepts Inf and NaN at the end of the arithmetic
> expression.

The following patch goes on top of 42369 to cleanup some rough edges and
to add test cases.

It still treats Inf and NaN as special keywords in math context but they
are matched case-insensitively. "infinity" is not matched unlike
strtod() which printf still uses. You can't assign to them in math
context.

Following the ISO C99 standard quoted by Vincent, printf will output
lowercase form - "inf", "nan" etc even on e.g. Solaris. I've left it as
"Inf"/"NaN" for $((..)) but I don't care a great deal either way.

The actual bounds checking for math functions was removed in 42369 which
leaves the flags being rather superfluous. This removes all trace of
them. I checked all the functions and you do just get values like NaN
and -Inf in the cases that were blocked before.

I had to revert the division by zero checking for integer division by
zero.

Like ksh93, you'll get "Inf" as output from echo "$((${inf=2}, inf))"
I don't really see a problem with that. There seemed to be consensus
against making Inf a variable and for case-insensitivity so that comes
as a consequence.

I'm open to consider any aspect of the behaviour.
Should I hold off with committing this for now or is it ok to go?

Oliver

diff --git a/Src/Modules/mathfunc.c b/Src/Modules/mathfunc.c
index a62154c50..01a2913ef 100644
--- a/Src/Modules/mathfunc.c
+++ b/Src/Modules/mathfunc.c
@@ -93,22 +93,6 @@ MS_RAND48
  * conversion), atan2.
  */
 
-/* Flags for bounds.  Note these must start at 1, not 0. */
-
-enum {
-  BF_POS    = 1,		/* must be positive */
-  BF_NONNEG = 2,		/* must be non-negative */
-  BF_FRAC   = 3,		/* must be -1 <= x <= 1 */
-  BF_GE1    = 4,		/* must be >= 1 */
-  BF_FRACO  = 5,		/* must be in open range -1 < x < 1 */
-  BF_INTPOS = 6,		/* must be non-integer or positive */
-  BF_GTRM1  = 7,		/* must be > -1 */
-  BF_NONZ   = 8,		/* must be nonzero */
-  BF_POS2   = 9			/* second argument must be positive */
-};
-
-#define BFLAG(x) ((x) << 8)
-
 /*
  * Flags for type of function: unlike the above, these must
  * be individually bit-testable.
@@ -121,18 +105,18 @@ enum {
     TF_NOASS  = 8		/* don't assign result as double */
 };
 
-#define TFLAG(x) ((x) << 16)
+#define TFLAG(x) ((x) << 8)
 
 
 static struct mathfunc mftab[] = {
-  NUMMATHFUNC("abs", math_func, 1, 1, MF_ABS | BFLAG(BF_FRAC) |
+  NUMMATHFUNC("abs", math_func, 1, 1, MF_ABS |
 	      TFLAG(TF_NOCONV|TF_NOASS)),
-  NUMMATHFUNC("acos", math_func, 1, 1, MF_ACOS | BFLAG(BF_FRAC)),
-  NUMMATHFUNC("acosh", math_func, 1, 1, MF_ACOSH | BFLAG(BF_GE1)),
-  NUMMATHFUNC("asin", math_func, 1, 1, MF_ASIN | BFLAG(BF_FRAC)),
+  NUMMATHFUNC("acos", math_func, 1, 1, MF_ACOS),
+  NUMMATHFUNC("acosh", math_func, 1, 1, MF_ACOSH),
+  NUMMATHFUNC("asin", math_func, 1, 1, MF_ASIN),
   NUMMATHFUNC("asinh", math_func, 1, 1, MF_ASINH),
   NUMMATHFUNC("atan", math_func, 1, 2, MF_ATAN),
-  NUMMATHFUNC("atanh", math_func, 1, 1, MF_ATANH | BFLAG(BF_FRACO)),
+  NUMMATHFUNC("atanh", math_func, 1, 1, MF_ATANH),
   NUMMATHFUNC("cbrt", math_func, 1, 1, MF_CBRT),
   NUMMATHFUNC("ceil", math_func, 1, 1, MF_CEIL),
   NUMMATHFUNC("copysign", math_func, 2, 2, MF_COPYSIGN),
@@ -146,20 +130,19 @@ static struct mathfunc mftab[] = {
   NUMMATHFUNC("float", math_func, 1, 1, MF_FLOAT),
   NUMMATHFUNC("floor", math_func, 1, 1, MF_FLOOR),
   NUMMATHFUNC("fmod", math_func, 2, 2, MF_FMOD),
-  NUMMATHFUNC("gamma", math_func, 1, 1, MF_GAMMA | BFLAG(BF_INTPOS)),
+  NUMMATHFUNC("gamma", math_func, 1, 1, MF_GAMMA),
   NUMMATHFUNC("hypot", math_func, 2, 2, MF_HYPOT),
-  NUMMATHFUNC("ilogb", math_func, 1, 1, MF_ILOGB | BFLAG(BF_NONZ) |
-	      TFLAG(TF_NOASS)),
+  NUMMATHFUNC("ilogb", math_func, 1, 1, MF_ILOGB | TFLAG(TF_NOASS)),
   NUMMATHFUNC("int", math_func, 1, 1, MF_INT | TFLAG(TF_NOASS)),
   NUMMATHFUNC("j0", math_func, 1, 1, MF_J0),
   NUMMATHFUNC("j1", math_func, 1, 1, MF_J1),
   NUMMATHFUNC("jn", math_func, 2, 2, MF_JN | TFLAG(TF_INT1)),
   NUMMATHFUNC("ldexp", math_func, 2, 2, MF_LDEXP | TFLAG(TF_INT2)),
-  NUMMATHFUNC("lgamma", math_func, 1, 1, MF_LGAMMA | BFLAG(BF_INTPOS)),
-  NUMMATHFUNC("log", math_func, 1, 1, MF_LOG | BFLAG(BF_POS)),
-  NUMMATHFUNC("log10", math_func, 1, 1, MF_LOG10 | BFLAG(BF_POS)),
-  NUMMATHFUNC("log1p", math_func, 1, 1, MF_LOG1P | BFLAG(BF_GTRM1)),
-  NUMMATHFUNC("logb", math_func, 1, 1, MF_LOGB | BFLAG(BF_NONZ)),
+  NUMMATHFUNC("lgamma", math_func, 1, 1, MF_LGAMMA),
+  NUMMATHFUNC("log", math_func, 1, 1, MF_LOG),
+  NUMMATHFUNC("log10", math_func, 1, 1, MF_LOG10),
+  NUMMATHFUNC("log1p", math_func, 1, 1, MF_LOG1P),
+  NUMMATHFUNC("logb", math_func, 1, 1, MF_LOGB),
   NUMMATHFUNC("nextafter", math_func, 2, 2, MF_NEXTAFTER),
 #ifdef HAVE_ERAND48
   STRMATHFUNC("rand48", math_string, MS_RAND48),
@@ -171,12 +154,12 @@ static struct mathfunc mftab[] = {
 #endif
   NUMMATHFUNC("sin", math_func, 1, 1, MF_SIN),
   NUMMATHFUNC("sinh", math_func, 1, 1, MF_SINH),
-  NUMMATHFUNC("sqrt", math_func, 1, 1, MF_SQRT | BFLAG(BF_NONNEG)),
+  NUMMATHFUNC("sqrt", math_func, 1, 1, MF_SQRT),
   NUMMATHFUNC("tan", math_func, 1, 1, MF_TAN),
   NUMMATHFUNC("tanh", math_func, 1, 1, MF_TANH),
-  NUMMATHFUNC("y0", math_func, 1, 1, MF_Y0 | BFLAG(BF_POS)),
-  NUMMATHFUNC("y1", math_func, 1, 1, MF_Y1 | BFLAG(BF_POS)),
-  NUMMATHFUNC("yn", math_func, 2, 2, MF_YN | BFLAG(BF_POS2) | TFLAG(TF_INT1))
+  NUMMATHFUNC("y0", math_func, 1, 1, MF_Y0),
+  NUMMATHFUNC("y1", math_func, 1, 1, MF_Y1),
+  NUMMATHFUNC("yn", math_func, 2, 2, MF_YN | TFLAG(TF_INT1))
 };
 
 /**/
diff --git a/Src/builtin.c b/Src/builtin.c
index fb59738f3..3852e7c52 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5236,8 +5236,14 @@ bin_print(char *name, char **args, Options ops, int func)
 			    errflag &= ~ERRFLAG_ERROR;
 			    ret = 1;
 			}
-			print_val(doubleval)
-			    break;
+			/* force consistent form for Inf/NaN output */
+			if (isnan(doubleval))
+			    count += fputs("nan", fout);
+			else if (isinf(doubleval))
+			    count += fputs((doubleval < 0.0) ? "-inf" : "inf", fout);
+		        else
+			    print_val(doubleval)
+			break;
 		    case 3:
 #ifdef ZSH_64_BIT_UTYPE
  		    	*d++ = 'l';
diff --git a/Src/math.c b/Src/math.c
index cdfe80bb4..32bccc6e9 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -593,7 +593,6 @@ isinf(double x)
 #endif
 
 #if !defined(HAVE_ISNAN)
-/**/
 static double
 store(double *x)
 {
@@ -816,27 +815,11 @@ zzlex(void)
 		ptr++;
 		break;
 	    }
-	case ' ':
+	case ' ': /* Fall through! */
 	case '\t':
 	case '\n':
 	    break;
-	/* Fall through! */
 	default:
-	    if (strcmp(ptr-1, "NaN") == 0) {
-		yyval.type = MN_FLOAT;
-		yyval.u.d = 0.0;
-		yyval.u.d /= yyval.u.d;
-		ptr += 2;
-		return NUM;
-	    }
-	    else if (strcmp(ptr-1, "Inf") == 0) {
-		yyval.type = MN_FLOAT;
-		yyval.u.d = 0.0;
-		yyval.u.d = 1.0 / yyval.u.d;
-		ptr += 2;
-		return NUM;
-	    }
-
 	    if (idigit(*--ptr) || *ptr == '.')
 		return lexconstant();
 	    if (*ptr == '#') {
@@ -860,6 +843,20 @@ zzlex(void)
 
 		p = ptr;
 		ptr = ie;
+		if (ie - p == 3) {
+		    if (strncasecmp(p, "NaN", 3) == 0) {
+			yyval.type = MN_FLOAT;
+			yyval.u.d = 0.0;
+			yyval.u.d /= yyval.u.d;
+			return NUM;
+		    }
+		    else if (strncasecmp(p, "Inf", 3) == 0) {
+			yyval.type = MN_FLOAT;
+			yyval.u.d = 0.0;
+			yyval.u.d = 1.0 / yyval.u.d;
+			return NUM;
+		    }
+		}
 		if (*ptr == '[' || (!cct && *ptr == '(')) {
 		    char op = *ptr, cp = ((*ptr == '[') ? ']' : ')');
 		    int l;
@@ -1114,6 +1111,10 @@ callmathfunc(char *o)
 static int
 notzero(mnumber a)
 {
+    if ((a.type & MN_INTEGER) && a.u.l == 0) {
+        zerr("division by zero");
+        return 0;
+    }
     return 1;
 }
 
diff --git a/Test/B03print.ztst b/Test/B03print.ztst
index c65568ad9..0ef3743ce 100644
--- a/Test/B03print.ztst
+++ b/Test/B03print.ztst
@@ -86,6 +86,17 @@
 >123.45 678
 >90.1 0
 
+ nan=0 inf=1 Infinity=2
+ printf '%.1f\n' -inf Infinity Inf nan NaN -Inf -0.0
+0:infinity constants
+>-inf
+>inf
+>inf
+>nan
+>nan
+>-inf
+>-0.0
+
  print -f 'arg: %b\n' -C2 '\x41' '\x42' '\x43'
 0:override -C when -f was given
 >arg: A
diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst
index 30409adf3..77a46ebd5 100644
--- a/Test/C01arith.ztst
+++ b/Test/C01arith.ztst
@@ -302,6 +302,40 @@
 ?(eval):1: bad math expression: operator expected at `2 '
 # ` for emacs shell mode
 
+  in=1 info=2 Infinity=3 Inf=4
+  print $(( in )) $(( info )) $(( Infinity )) $(( $Inf )) $(( inf )) $(( INF )) $(( Inf )) $(( iNF ))
+0:Infinity parsing
+>1 2 3 4 Inf Inf Inf Inf
+
+  integer Inf
+  print $(( Inf[0] ))
+1:Refer to Inf with an array subscript
+?(eval):2: bad base syntax
+
+  (( NaN = 1 ))
+2:Assign to NaN
+?(eval):1: bad math expression: lvalue required
+
+  a='Inf'
+  (( b = 1e500 ))
+  print $((1e500)) $(($((1e500)))) $(( a )) $b $(( b )) $(( 3.0 / 0 ))
+0:Overflow to infinity
+>Inf Inf Inf Inf Inf Inf
+
+  print $((1e500))
+  print $(( $((1e500)) ))
+0:Reinput infinity value into math context
+>Inf
+>Inf
+
+  print $((1e500/1e500)) $((-1e500/1e500)) $(( 24. % 0 ))
+0:NaN results
+>NaN NaN NaN
+
+  (( 3 / 0 ))
+2:Integer division by zero
+?(eval):1: division by zero
+
   integer varassi
   print $(( varassi = 5.5 / 2.0 ))
   print $varassi
diff --git a/Test/V03mathfunc.ztst b/Test/V03mathfunc.ztst
index 1edb7a279..d6b4e0987 100644
--- a/Test/V03mathfunc.ztst
+++ b/Test/V03mathfunc.ztst
@@ -100,8 +100,8 @@ F:This test fails if your math library doesn't have erand48().
 >1.50000
 
    print $(( sqrt(-1) ))
-1:Non-negative argument checking for square roots.
-?(eval):1: math: argument to sqrt out of range
+0:Non-negative argument checking for square roots.
+>NaN
 
 # Simple test that the pseudorandom number generators are producing
 # something that could conceivably be pseudorandom numbers in a


      parent reply	other threads:[~2018-03-21 23:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-07 22:30 Stephane Chazelas
2018-02-07 23:25 ` Oliver Kiddle
2018-02-08  9:38   ` Peter Stephenson
2018-02-08 12:46   ` Daniel Shahaf
2018-02-08 14:22     ` Stephane Chazelas
2018-02-09 15:31       ` Daniel Shahaf
2018-02-09 21:09         ` Stephane Chazelas
2018-02-10  0:10           ` Bart Schaefer
2018-02-16 16:51     ` Oliver Kiddle
2018-02-17  0:38       ` Daniel Shahaf
2018-02-19 14:19         ` Stephane Chazelas
2018-02-27 13:02       ` Vincent Lefevre
2018-02-27 15:25         ` Oliver Kiddle
2018-02-27 16:56           ` Vincent Lefevre
2018-03-21 23:46       ` Oliver Kiddle [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=25305.1521675988@thecus \
    --to=okiddle@yahoo.co.uk \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).