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

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

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