Re: "set -e" handling is broken with zsh 5.3.1 and 5.4.1

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Sun, 27 Aug 2017 02:50:40 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> Consider:
> 
> ----------------------------------------
> #!/usr/bin/env zsh
> 
> set -e
> 
> f()
> {
>   [[ -z 1 ]] && false
> }
> 
> if false; then
>   :
> else
>   f
>   echo Fail 1
>   echo Fail 2
>   f
>   echo Fail 3
> fi
> ----------------------------------------
> 
> With
>   zsh 5.3.1-4+b1 under Debian/stretch
>   zsh 5.4.1-1 under Debian/unstable
> 
> I get:
> 
> % ./cond2-e; echo $?
> Fail 1
> Fail 2
> 1
> 
> I suppose that cond2-e should die just after f is called, before
> outputting anything.

Yes, I would say so.

This appears to have been deliberate, in that after "if" we usually
restore noerrexit beahaviour on the first thing we execute, but if it's
a function we don't.  However, I can't for the life of me work out why I
made that exeception --- certainly nothing goes wrong in the tests if I
apply the following, and I would expect to be testing whatever it was
made me think we needed the qualification.  It might have been to do
with empty functions, but making f empty, so it runs but doesn't change
the status, doesn't seem to do anything unexpected (we shouldn't
and don't exit).

I think I'll apply the effect of this after the release of 5.4.2, with a
test based on the code above plus some empty functions just in case, and
see what happens.

In the mean time somebody may find a more fiendish variant involving
functions that causes a further problem.

pws

diff --git a/Src/exec.c b/Src/exec.c
index cd99733..82277a3 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3025,7 +3025,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	preargs = NULL;
 
     /* if we get this far, it is OK to pay attention to lastval again */
-    if ((noerrexit & NOERREXIT_UNTIL_EXEC) && !is_shfunc)
+    if ((noerrexit & NOERREXIT_UNTIL_EXEC)/* && !is_shfunc*/)
 	noerrexit = 0;
 
     /* Do prefork substitutions.