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. I have confirmed this behaviour on two systems: (1) Apple MacBook Air, Ubuntu 20.04.2 LTS Linux Luftbuch 5.4.0-66-generic #74-Ubuntu SMP Wed Jan 27 22:54:38 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux zsh 5.8 (x86_64-ubuntu-linux-gnu) (2) Apple iMac, macOS 11.2.2 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 zsh 5.8 (x86_64-apple-darwin20.0) -------- Test Script -------- #!/usr/bin/env zsh set -e OS=$(uname) 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 echo "OS = ${OS}" zsh --version echo "works fine" x=$(( min(2,-1,3) )) echo "min(2,1,3) = $((x))" x=$(( max(2,-1,3) )) echo "max(2,-1,3) = $((x))" x=$(( sum(2,-1,3) )) echo "sum(2,-1,3) = $((x))" echo "min, max, sum throw an error if result=0" echo " => set -e causes script to abort" x=$(( sum(-2,2) )) echo "sum(-2,2) = $((x))" x=$(( min(2,0,3) )) echo "min(2,0,3) = $((x))" x=$(( max(-2,0,-3) )) echo "max(-2,0,-3) = $((x))" -------- Output on Mac OS X -------- OS = Darwin zsh 5.8 (x86_64-apple-darwin20.0) works fine min(2,1,3) = -1 max(2,-1,3) = 3 sum(2,-1,3) = 4 min, max, sum throw an error if result=0 => set -e causes script to abort -------- Output on Ubuntu -------- OS = Linux zsh 5.8 (x86_64-ubuntu-linux-gnu) works fine min(2,1,3) = -1 max(2,-1,3) = 3 sum(2,-1,3) = 4 min, max, sum throw an error if result=0 => set -e causes script to abort

[-- 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

On Sun, Mar 7, 2021 at 9:17 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Nikolaus Thiel wrote on Sun, Mar 07, 2021 at 17:37:09 +0100: > > > > When the result of min, max or sum equals 0, the functions throw an error. I would not equate "return nozero" with "throw an error", FWIW. > Ouch. Patch series attached. I think your regression tests cover this, but use of "true" avoids changing the result in "functions -M" context, correct? Is it worth testing invalid cases? Such as uses outside math context where the arguments are not syntax checked? As long as we're on the subject: zsh_math_func_min() { local result=$1 shift local arg for arg ; do (( $arg < result )) && result=$arg done (( result )) true # Careful here: `return 0` evaluates an arithmetic expression } Because of the way math context works, if any of $@ is a string that can be interpreted as a math expression, the above will evaluate it at least twice (and up to $# times in the case of $1). This could have side-effects. zsh_math_func_min() { local result=$(( $1 )) shift local arg n for arg ; do (( n = arg )) # evaluate arg exactly once (( n < result && (result = n) )) done (( result )) true # Careful here: `return 0` evaluates an arithmetic expression }

Bart Schaefer wrote on Sun, 07 Mar 2021 21:39 +00:00: > On Sun, Mar 7, 2021 at 9:17 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > Nikolaus Thiel wrote on Sun, Mar 07, 2021 at 17:37:09 +0100: > > > > > > When the result of min, max or sum equals 0, the functions throw an error. > > I would not equate "return nozero" with "throw an error", FWIW. > > > Ouch. Patch series attached. > > I think your regression tests cover this, but use of "true" avoids > changing the result in "functions -M" context, correct? Yeah. «return 0» causes the arithmetic expression to evaluate to 0, because the argument to «return» is math-evaluated. > Is it worth testing invalid cases? Such as uses outside math context > where the arguments are not syntax checked? I'd say this specific example of an invalid case is lower priority, since there's no promise of any particular failure mode. We could add a test to ensure it doesn't format C:, of course. A test for min() and max() with zero arguments would make sense, since those cases actually do promise a specific failure mode. Could also add tests with three arguments in various permutations. If you have ideas, feel free to write them and post them; I'll transplant them into Z02 once I have committed it. > As long as we're on the subject: > > zsh_math_func_min() { > local result=$1 > shift > local arg > for arg ; do > (( $arg < result )) && result=$arg > done > (( result )) > true # Careful here: `return 0` evaluates an arithmetic expression > } > > Because of the way math context works, if any of $@ is a string that > can be interpreted as a math expression, the above will evaluate it at > least twice (and up to $# times in the case of $1). This could have > side-effects. Could you post a regression test for this? Related to code as it is in master, are «(( $arg < result ))» and «(( arg < result ))» equivalent? > zsh_math_func_min() { > local result=$(( $1 )) Doesn't this still do multiple string-to-number conversions? > shift > local arg n > for arg ; do > (( n = arg )) # evaluate arg exactly once > (( n < result && (result = n) )) > done > (( result )) > true # Careful here: `return 0` evaluates an arithmetic expression > } Hmm. I'll go ahead and push the series now, then, so it'll be easier for you to work on revisions. If anyone has commit reviews, don't let the fact that I've pushed things affect you. Cheers, Daniel

On Sun, Mar 7, 2021 at 1:57 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > Bart Schaefer wrote on Sun, 07 Mar 2021 21:39 +00:00: > > > Is it worth testing invalid cases? Such as uses outside math context > > where the arguments are not syntax checked? > > If you have ideas, feel free to write them and post them; I'll > transplant them into Z02 once I have committed it. One possibility below; I'll follow up if I think of more. With your patch adding "true" at the end, we get this: % zsh_math_func_min "foo bar" x y z zsh_math_func_min:7: bad math expression: operator expected at `bar' zsh_math_func_min:7: bad math expression: operator expected at `bar' zsh_math_func_min:7: bad math expression: operator expected at `bar' zsh_math_func_min:9: bad math expression: operator expected at `bar' toltec-ubuntu% echo $? 0 Doesn't seem as though anything that prints that many error messages should return zero. > > Because of the way math context works, if any of $@ is a string that > > can be interpreted as a math expression, the above will evaluate it at > > least twice (and up to $# times in the case of $1). This could have > > side-effects. > > Could you post a regression test for this? In thinking more about it, I believe this only matters when the function is called outside of math context. In math context, the arguments are all going to be evaluated down to numbers before they are passed to the function. % (( x = 0 )); zsh_math_func_min "x += 2" 4 5 6 % print $x 8 % (( (x = 0), min("x += 2", 4, 5, 6) )) % print $x 2 > Related to code as it is in master, are «(( $arg < result ))» and > «(( arg < result ))» equivalent? No, but again math context matters. % arg="x += 2" x=0 result=0 % (( $arg > result )) && result=$arg % print $result $x $(( result )) $x x += 2 1 3 3 % x=0 result=0 % (( arg > result )) && (( result=arg )) % print $result $x $(( result )) $x 4 4 4 4 > > zsh_math_func_min() { > > local result=$(( $1 )) > > Doesn't this still do multiple string-to-number conversions? I don't think so ... the $(( ... )) should turn $1 into a single number assigned to result. What are you thinking about?

[-- Attachment #1: Type: text/plain, Size: 771 bytes --] Daniel Shahaf wrote on Sun, Mar 07, 2021 at 17:17:12 +0000: > 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. Patch series attached. I base64'd the attachments (Mutt ☺) to avoid triggering workers/48587. Would break grepping the raw mbox for them. (I didn't want to spam 5 separate messages onto the list. I do already have a draft fix for workers/48587, but haven't deployed it yet.) I can't say the semantics are very user-friendly. For «return 0» or «foo[42]=success» to change the result of an arithmetic function is surprising. At least it'll be a documented surprise, as opposed to an undocumented one. Cheers, Daniel [-- Attachment #2: 0001-docs-functions-M-Move-an-example-to-be-near-the-.patch.txt --] [-- Type: text/plain, Size: 1652 bytes --] From c2bbcbf4baa9d9f182ff25b79e2de7c0bbd1c320 Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Fri, 16 Apr 2021 17:33:52 +0000 Subject: [PATCH 1/5] docs: functions -M: Move an example to be near the specification of the relevant flag --- Doc/Zsh/builtins.yo | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index a7afe42cf..62c30c917 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -924,6 +924,12 @@ opening and matching closing parenthesis is passed to the function as a single argument, even if it includes commas or white space. The minimum and maximum argument specifiers must therefore be 1 if given. An empty argument list is passed as a zero-length string. +Thus, the following string function takes a single argument, including +the commas, and prints 11: + +example(stringfn+LPAR()RPAR() { (( $#1 )) } +functions -Ms stringfn +print $(( stringfn+LPAR()foo,bar,rod+RPAR() ))) tt(functions -M) with no arguments lists all such user-defined functions in the same form as a definition. With the additional option tt(-m) and @@ -941,13 +947,6 @@ For example, the following prints the cube of 3: example(zmath_cube+LPAR()RPAR() { (( $1 * $1 * $1 )) } functions -M cube 1 1 zmath_cube print $(( cube+LPAR()3+RPAR() ))) - -The following string function takes a single argument, including -the commas, so prints 11: - -example(stringfn+LPAR()RPAR() { (( $#1 )) } -functions -Ms stringfn -print $(( stringfn+LPAR()foo,bar,rod+RPAR() ))) ) module(getcap)(zsh/cap) findex(getln) [-- Attachment #3: 0002-docs-functions-M-Document-the-return-status-resu.patch.txt --] [-- Type: text/plain, Size: 3668 bytes --] From 5d32996b704b0680528c75acce8ade975d4670de Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Fri, 16 Apr 2021 18:03:30 +0000 Subject: [PATCH 2/5] docs: functions -M: Document the return status / result interdependency gotchas (cf. 48147). --- Doc/Zsh/builtins.yo | 58 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index 62c30c917..80a9e97db 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -914,9 +914,53 @@ expressions. The name of the function in tt($0) is var(mathfn) (not var(shellfn) as would usually be the case), provided the option tt(FUNCTION_ARGZERO) is in effect. The positional parameters in the shell function correspond to the arguments of the mathematical function call. -The result of the last arithmetical expression evaluated -inside the shell function (even if it is a form that normally only returns -a status) gives the result of the mathematical function. + +The result of the last arithmetical expression evaluated inside the shell +function gives the result of the mathematical function. This is not limited to +arithmetic substitutions of the form tt($+LPAR()+LPAR())var(...)tt(+RPAR()+RPAR()), +but also includes arithmetical expressions evaluated in any other way, including +by the tt(let) builtin, +by tt(+LPAR()+LPAR())var(...)tt(+RPAR()+RPAR()) statements, +and even +by the tt(return) builtin +and +by array subscripts. +Therefore, care must be taken not to use syntactical constructs that perform +arithmetic evaluation after evaluating what is to be the result of the function. +For example: + +example(# WRONG +zmath_cube+LPAR()+RPAR() { + (( $1 * $1 * $1 )) + return 0 +} +functions -M cube 1 1 zmath_cube +print $(( cube+LPAR()3+RPAR() ))) + +This will print `tt(0)' because of the tt(return). + +Commenting the tt(return) out would lead to a different problem: the +tt(+LPAR()+LPAR())var(...)tt(+RPAR()+RPAR()) statement would become +the last statement in the function, so the em(return status) (tt($?)) of the +function would be non-zero (indicating failure) whenever the em(arithmetic +result) of the function would happen to be zero (numerically): + +example(# WRONG +zmath_cube+LPAR()+RPAR() { + (( $1 * $1 * $1 )) +} +functions -M cube 1 1 zmath_cube +print $(( cube+LPAR()0+RPAR() ))) + +Instead, the tt(true) builtin can be used: + +example(# RIGHT +zmath_cube+LPAR()+RPAR() { + (( $1 * $1 * $1 )) + true +} +functions -M cube 1 1 zmath_cube +print $(( cube+LPAR()3+RPAR() ))) If the additional option tt(-s) is given to tt(functions -M), the argument to the function is a single string: anything between the @@ -927,7 +971,7 @@ argument list is passed as a zero-length string. Thus, the following string function takes a single argument, including the commas, and prints 11: -example(stringfn+LPAR()RPAR() { (( $#1 )) } +example(stringfn+LPAR()RPAR() { (( $#1 )); true } functions -Ms stringfn print $(( stringfn+LPAR()foo,bar,rod+RPAR() ))) @@ -941,12 +985,6 @@ additional option tt(-m) the arguments are treated as patterns and all functions whose var(mathfn) matches the pattern are removed. Note that the shell function implementing the behaviour is not removed (regardless of whether its name coincides with var(mathfn)). - -For example, the following prints the cube of 3: - -example(zmath_cube+LPAR()RPAR() { (( $1 * $1 * $1 )) } -functions -M cube 1 1 zmath_cube -print $(( cube+LPAR()3+RPAR() ))) ) module(getcap)(zsh/cap) findex(getln) [-- Attachment #4: 0003-docs-functions-M-Add-a-subheading-and-index-entr.patch.txt --] [-- Type: text/plain, Size: 1226 bytes --] From 90a76320587e7a9e6a978809674451495a538fec Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Fri, 16 Apr 2021 18:03:45 +0000 Subject: [PATCH 3/5] docs: functions -M: Add a subheading and index entries. --- Doc/Zsh/builtins.yo | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index 80a9e97db..264ee9484 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -893,6 +893,10 @@ without affecting the other. A typical idiom is that var(oldfn) is the name of a library shell function which is then redefined to call tt(newfn), thereby installing a modified version of the function. +em(The )tt(-M)em( and )tt(+M)em( flags) +cindex(defining mathematical functions) +cindex(functions, defining mathematical) + Use of the tt(-M) option may not be combined with any of the options handled by tt(typeset -f). @@ -929,6 +933,8 @@ Therefore, care must be taken not to use syntactical constructs that perform arithmetic evaluation after evaluating what is to be the result of the function. For example: +findex(zmath_cube) +findex(cube) example(# WRONG zmath_cube+LPAR()+RPAR() { (( $1 * $1 * $1 )) [-- Attachment #5: 0004-docs-return-Give-examples-of-using-arithmetic-ev.patch.txt --] [-- Type: text/plain, Size: 1651 bytes --] From 08cee6879c28b5f4761050f12a3ec2b22544df62 Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Fri, 16 Apr 2021 18:04:13 +0000 Subject: [PATCH 4/5] docs: return: Give examples of using arithmetic evaluation. --- Doc/Zsh/builtins.yo | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo index 264ee9484..640cfb674 100644 --- a/Doc/Zsh/builtins.yo +++ b/Doc/Zsh/builtins.yo @@ -1626,9 +1626,15 @@ cindex(functions, returning from) item(tt(return) [ var(n) ])( Causes a shell function or `tt(.)' script to return to the invoking script with the return status specified by -an arithmetic expression var(n). If var(n) +an arithmetic expression var(n). +For example, the following prints `tt(42)': + +example(() { integer foo=40; return "foo + 2" } +echo $?) + +If var(n) is omitted, the return status is that of the last command -executed. +executed. If tt(return) was executed from a trap in a tt(TRAP)var(NAL) function, the effect is different for zero and non-zero return status. With zero @@ -1637,7 +1643,7 @@ will return to whatever it was previously processing; with a non-zero status, the shell will behave as interrupted except that the return status of the trap is retained. Note that the numeric value of the signal which caused the trap is passed as the first argument, so the statement -`tt(return $((128PLUS()$1)))' will return the same status as if the signal +`tt(return "128PLUS()$1")' will return the same status as if the signal had not been trapped. ) module(sched)(zsh/sched) [-- Attachment #6: 0005-zmathfuncdef-Fix-the-workers-48147-return-status.patch.txt --] [-- Type: text/plain, Size: 788 bytes --] From 1f195629a23ded3ba4294bf4838eada6a695f07c Mon Sep 17 00:00:00 2001 From: Daniel Shahaf <d.s@daniel.shahaf.name> Date: Fri, 16 Apr 2021 18:04:39 +0000 Subject: [PATCH 5/5] zmathfuncdef: Fix the workers/48147 return status / 'set -e' bug. Not tested. --- Functions/Misc/zmathfuncdef | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Functions/Misc/zmathfuncdef b/Functions/Misc/zmathfuncdef index e5692e769..5ed991f68 100644 --- a/Functions/Misc/zmathfuncdef +++ b/Functions/Misc/zmathfuncdef @@ -78,7 +78,7 @@ if ! zmodload -e zsh/mathfunc; then fi { - eval "$fname() { (( $body )) }" + eval "$fname() { (( $body )); true }" } always { # Remove math function if shell function definition failed. if (( TRY_BLOCK_ERROR )); then

```
Bart Schaefer wrote on Sun, Mar 07, 2021 at 18:25:00 -0800:
> On Sun, Mar 7, 2021 at 1:57 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > Bart Schaefer wrote on Sun, 07 Mar 2021 21:39 +00:00:
> > > zsh_math_func_min() {
> > > local result=$(( $1 ))
> >
> > Doesn't this still do multiple string-to-number conversions?
>
> I don't think so ... the $(( ... )) should turn $1 into a single
> number assigned to result. What are you thinking about?
When «$foo» appears inside «$((…))», it's expanded as a string that's
then parsed as math. The result of «$((…))» is then assigned to the
$result, which is eventually used as «(( result ))», where — because
${(t)result} is non-integer scalar — there will be another
string-to-integer conversion.
Sorry for the delay on this.
Daniel
```

```
On Fri, Apr 16, 2021 at 11:26 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> When «$foo» appears inside «$((…))», it's expanded as a string that's
> then parsed as math. The result of «$((…))» is then assigned to the
> $result, which is eventually used as «(( result ))», where — because
> ${(t)result} is non-integer scalar — there will be another
> string-to-integer conversion.
Yes, there's another string-to-integer conversion, but I haven't
figured out how «result=$((...))» can possibly assign any string that
is not already a plain number, so the string-to-integer conversion is
free of meaningful side-effects. Have I missed something?
```

Again sorry for the delay on getting back to this. Bart Schaefer wrote on Sun, Mar 07, 2021 at 18:25:00 -0800: > On Sun, Mar 7, 2021 at 1:57 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > > > Bart Schaefer wrote on Sun, 07 Mar 2021 21:39 +00:00: > > > > > Is it worth testing invalid cases? Such as uses outside math context > > > where the arguments are not syntax checked? > > > > If you have ideas, feel free to write them and post them; I'll > > transplant them into Z02 once I have committed it. > > One possibility below; I'll follow up if I think of more. > > With your patch adding "true" at the end, we get this: > > % zsh_math_func_min "foo bar" x y z > zsh_math_func_min:7: bad math expression: operator expected at `bar' > zsh_math_func_min:7: bad math expression: operator expected at `bar' > zsh_math_func_min:7: bad math expression: operator expected at `bar' > zsh_math_func_min:9: bad math expression: operator expected at `bar' > toltec-ubuntu% echo $? > 0 > > Doesn't seem as though anything that prints that many error messages > should return zero. > Patch below. > > > Because of the way math context works, if any of $@ is a string that > > > can be interpreted as a math expression, the above will evaluate it at > > > least twice (and up to $# times in the case of $1). This could have > > > side-effects. > > > > Could you post a regression test for this? > > In thinking more about it, I believe this only matters when the > function is called outside of math context. In math context, the > arguments are all going to be evaluated down to numbers before they > are passed to the function. > > % (( x = 0 )); zsh_math_func_min "x += 2" 4 5 6 > % print $x > 8 > % (( (x = 0), min("x += 2", 4, 5, 6) )) > % print $x > 2 I see. Still, going to fix this, if only because those min() max() sum() implementations are virtually the only example of `functions -M` in the tree, and it'd be good to have a complete example. > > Related to code as it is in master, are «(( $arg < result ))» and > > «(( arg < result ))» equivalent? > > No, but again math context matters. > > % arg="x += 2" x=0 result=0 > % (( $arg > result )) && result=$arg > % print $result $x $(( result )) $x > x += 2 1 3 3 > % x=0 result=0 > % (( arg > result )) && (( result=arg )) > % print $result $x $(( result )) $x > 4 4 4 4 Nice. So the first one interpolates $arg as in double-quoted strings and then parses the string " x += 2 > result ", where the > operator has higher precedence, and «2 > result» evaluates to 1, and the «print» then evaluates everything left to right; whereas the latter sees «arg» and tries to evaluate that _as a number_, which works out to 2 (with a side effect), so the condition evaluates to true, and the assignment on the RHS then works out to 4 (with a side effect). Clear as a cloudless day ☺ Many thanks. Daniel diff --git a/Functions/Math/zmathfunc b/Functions/Math/zmathfunc index 8e4b78549..28f21e562 100644 --- a/Functions/Math/zmathfunc +++ b/Functions/Math/zmathfunc @@ -6,7 +6,12 @@ zsh_math_func_min() { shift local arg for arg ; do - (( $arg < result )) && result=$arg + (( $arg < result )) + case $? in + (0) result=$arg;; + (1) ;; + (*) return $?;; + esac done (( result )) true # Careful here: `return 0` evaluates an arithmetic expression @@ -19,7 +24,12 @@ zsh_math_func_max() { shift local arg for arg ; do - (( $arg > result )) && result=$arg + (( $arg > result )) + case $? in + (0) result=$arg;; + (1) ;; + (*) return $?;; + esac done (( result )) true # Careful here: `return 0` evaluates an arithmetic expression @@ -31,7 +41,7 @@ zsh_math_func_sum() { local sum local arg for arg ; do - (( sum += $arg )) + (( sum += arg )) done (( sum )) true # Careful here: `return 0` evaluates an arithmetic expression

On Fri, Apr 16, 2021 at 12:51 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > > [...] the first one interpolates $arg as in double-quoted strings > and then parses the string " x += 2 > result ", where the > operator has > higher precedence, and «2 > result» evaluates to 1, and the «print» then > evaluates everything left to right; whereas the latter sees «arg» and > tries to evaluate that _as a number_, which works out to 2 (with a side > effect), so the condition evaluates to true, and the assignment on the > RHS then works out to 4 (with a side effect). Clear as a cloudless day Given that, I'm puzzled why you did > + (( $arg < result )) > + case $? in > + (0) result=$arg;; > + (1) ;; > + (*) return $?;; > + esac > done > (( result )) and why not (( arg < result )) case $? in (0) (( result = arg ));; (1) ;; (*) return $?;; esac done (( result )) ?? The (( $arg < result )) formulation still potentially has different operator precedence in and out of math context, and result=$arg followed by (( result )) still does repeat interpretation of $arg. Why ever interpolate $arg as a string?

```
Bart Schaefer wrote on Fri, 16 Apr 2021 20:05 +00:00:
> and why not
>
> (( arg < result ))
> case $? in
> (0) (( result = arg ));;
> (1) ;;
> (*) return $?;;
> esac
> done
> (( result ))
>
> ??
I simply missed that in the pre-posting review. I'll change it before committing.
Thanks for the review, Bart.
Daniel
```

```
Hi Daniel,
On Fri, Apr 16, 2021, at 2:21 PM, Daniel Shahaf wrote:
> Daniel Shahaf wrote on Sun, Mar 07, 2021 at 17:17:12 +0000:
> > 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.
>
> Patch series attached.
Were you still planning on pushing these?
vq
```

```
Lawrence Velázquez wrote on Sun, May 16, 2021 at 10:54:13 -0400:
> Hi Daniel,
>
> On Fri, Apr 16, 2021, at 2:21 PM, Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Sun, Mar 07, 2021 at 17:17:12 +0000:
> > > 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.
> >
> > Patch series attached.
>
> Were you still planning on pushing these?
Pushed, with the extraneous whitespace change in 0004 reverted. Thanks
once again, Lawrence.
Daniel
```