Re: "{ } always { }" construct and return in called functions

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Wed, 13 May 2015 14:31:41 +0200
ia0 <zsh@xxxxxx> wrote:
> There is something I don't understand with the "{ } always { }" construct.
> What is the difference between the two following runs? I would expect the
> last run to behave as the first one. What am I missing here?
> 
> *% zsh --version*
> zsh 5.0.2 (x86_64-pc-linux-gnu)
> *% cat test*
> #!/bin/zsh
> 
> mytrue() { return 0 }
> mywrap() { echo BEGIN; $1; echo END }
> mytest() { { exit 1 } always { mywrap $1 } }
> 
> mytest $1
> *% ./test true*
> BEGIN
> END
> *1% ./test mytrue*
> BEGIN
> *1% *

Nested functions in always blocks with an exit pending are a corner case
that needs fixing.  I'm not sure if this affects exit traps for
functions, but possibly only in even weirder cases.

Follow-ups can go to zsh-workers --- there are no user-serviceable
parts inside.

pws


diff --git a/Src/builtin.c b/Src/builtin.c
index ffde5c9..70e75ff 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -4788,6 +4788,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
 mod_export int
 exit_pending;
 
+/* Shell level at which we exit if exit_pending */
+/**/
+mod_export int
+exit_level;
+
 /* break, bye, continue, exit, logout, return -- most of these take   *
  * one numeric argument, and the other (logout) is related to return. *
  * (return is treated as a logout when in a login shell.)             */
@@ -4865,6 +4870,7 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 		retflag = 1;
 		breaks = loops;
 		exit_pending = (num << 1) | 1;
+		exit_level = locallevel;
 	    }
 	} else
 	    zexit(num, 0);
diff --git a/Src/exec.c b/Src/exec.c
index 6a8b35a..527dffb 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5101,7 +5101,15 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
     }
     popheap();
 
-    if (exit_pending) {
+    /*
+     * Exit with a tidy up.
+     * Only leave if we're at the end of the appropriate function ---
+     * not a nested function.  As we usually skip the function body,
+     * the only likely case where we need that second test is
+     * when we have an "always" block.  The endparamscope() has
+     * already happened, hence the "+1" here.
+     */
+    if (exit_pending && exit_level == locallevel+1) {
 	if (locallevel > forklevel) {
 	    /* Still functions to return: force them to do so. */
 	    retflag = 1;
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index f04ddda..2de2919 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -589,3 +589,25 @@
 >success2
 >read it
 >read it
+
+  (
+  mywrap() { echo BEGIN; true; echo END }
+  mytest() { { exit 3 } always { mywrap }; print Exited before this }
+  mytest
+  print Exited before this, too
+  )
+3:Exit and always block with functions: simple
+>BEGIN
+>END
+
+  (
+  mytrue() { echo mytrue; return 0 }
+  mywrap() { echo BEGIN; mytrue; echo END }
+  mytest() { { exit 4 } always { mywrap }; print Exited before this }
+  mytest
+  print Exited before this, too
+  )
+4:Exit and always block with functions: nested
+>BEGIN
+>mytrue
+>END