Re: zmathfunc: min, max, sum throw error if result equals 0

[Daniel Shahaf <d.s@daniel.shahaf.name>]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Nikolaus Thiel wrote on Sun, Mar 07, 2021 at 17:37:09 +0100:
> There seems to be a bug in zmathfunc:
> 
> When the result of min, max or sum equals 0, the functions throw an error. If
> they are used within a script with the option "set -e" then the script aborts,
> cf. test script below.

Ouch.  Patch series attached.

However, the examples in zshbuiltins(1) (under `functions -M`) have the
same bug.  Would you perchance be interested in fixing those?  The
manual sources are in Doc/Zsh/*.yo.

Review below.

> if [[ ${OS} == "Darwin" ]]; then
>     ### Mac OS X
>     source /usr/share/zsh/5.8/functions/zmathfunc
> else
>     ### Ubuntu 20.04
>     source /usr/share/zsh/functions/Math/zmathfunc
> fi

autoload -Uz zmathfunc && zmathfunc

> echo "OS = ${OS}"

typeset -p OS

or use the builtin variable:

typeset -p OSTYPE

> zsh --version

echo $ZSH_VERSION $ZSH_PATCHLEVEL

>

Cheers,

Daniel


[-- Attachment #2: 0001-tests-Add-a-unit-test-for-zmathfunc-and-a-regres.patch.txt --]
[-- Type: text/plain, Size: 995 bytes --]

From eed36947fb1a35cdaf21638fb49ecfc4f40cdfc5 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sun, 7 Mar 2021 16:58:03 +0000
Subject: [PATCH 1/2] tests: Add a unit test for zmathfunc and a regression
 test for workers/48146 affecting it.

---
 Test/Z02zmathfunc.ztst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 Test/Z02zmathfunc.ztst

diff --git a/Test/Z02zmathfunc.ztst b/Test/Z02zmathfunc.ztst
new file mode 100644
index 000000000..94bc59576
--- /dev/null
+++ b/Test/Z02zmathfunc.ztst
@@ -0,0 +1,23 @@
+%prep
+  autoload -Uz zmathfunc && zmathfunc
+
+%test
+
+  echo $(( min(42, 43) )) $(( max(42, 43) )) $(( sum(42, 43) ))
+  echo $(( min(42) )) $(( max(42) )) $(( sum(42) ))
+  echo $(( sum() ))
+0:basic functionality test
+>42 43 85
+>42 42 42
+>0
+
+
+  (set -e; echo $(( min(0,   42) )))
+  (set -e; echo $(( max(0,  -42) )))
+  (set -e; echo $(( sum(42, -42) )))
+-f:regression test for ERR_EXIT 
+>0
+>0
+>0
+
+%clean

[-- Attachment #3: 0002-zmathfunc-Fix-bug-where-the-exit-code-would-be-n.patch.txt --]
[-- Type: text/plain, Size: 1853 bytes --]

From dca20086146e7c6022e394cc760ad39b12d96db2 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Sun, 7 Mar 2021 17:07:06 +0000
Subject: [PATCH 2/2] zmathfunc: Fix bug where the exit code would be non-zero
 if the expression evaluted to zero.

---
 Functions/Math/zmathfunc | 10 ++++++++--
 Test/Z02zmathfunc.ztst   |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Functions/Math/zmathfunc b/Functions/Math/zmathfunc
index 4ff40700d..8e4b78549 100644
--- a/Functions/Math/zmathfunc
+++ b/Functions/Math/zmathfunc
@@ -1,34 +1,40 @@
 #autoload
 
 zsh_math_func_min() {
+  emulate -L zsh
   local result=$1
   shift
   local arg
   for arg ; do
     (( $arg < result )) && result=$arg
   done
-  (( result )) # return
+  (( result ))
+  true # Careful here: `return 0` evaluates an arithmetic expression
 }
 functions -M min 1 -1 zsh_math_func_min # at least one argument
 
 zsh_math_func_max() {
+  emulate -L zsh
   local result=$1
   shift
   local arg
   for arg ; do
     (( $arg > result )) && result=$arg
   done
-  (( result )) # return
+  (( result ))
+  true # Careful here: `return 0` evaluates an arithmetic expression
 }
 functions -M max 1 -1 zsh_math_func_max # at least one argument
 
 zsh_math_func_sum() {
+  emulate -L zsh
   local sum
   local arg
   for arg ; do
     (( sum += $arg ))
   done
   (( sum ))
+  true # Careful here: `return 0` evaluates an arithmetic expression
 }
 functions -M sum 0 -1 zsh_math_func_sum
 
diff --git a/Test/Z02zmathfunc.ztst b/Test/Z02zmathfunc.ztst
index 94bc59576..43a0a0d76 100644
--- a/Test/Z02zmathfunc.ztst
+++ b/Test/Z02zmathfunc.ztst
@@ -15,7 +15,7 @@
   (set -e; echo $(( min(0,   42) )))
   (set -e; echo $(( max(0,  -42) )))
   (set -e; echo $(( sum(42, -42) )))
--f:regression test for ERR_EXIT 
+0:regression test for ERR_EXIT 
 >0
 >0
 >0