The commenting out also fixes the problem for case statements and braces
(i.e., for "{ ... }"). It works even if loop.c is reverted to the previous
state with "this_noerrexit = 1" statements, which seem more correct to me.

Apparently execwhile, like execif, needs more complicated
noerrexit resetting logic, even though I still don't understand what it's
doing in execif and why it's needed.

Philippe


On Sun, Nov 13, 2022 at 2:55 PM Philippe Altherr <philippe.altherr@gmail.com>
wrote:

> You shouldn't even be bothering with 5.8.1, it's been wrong all along;
>> it blindly never errexits at the end of an if/then/fi.
>
>
> I think that this isn't necessarily wrong. My understanding of the code so
> far is that the decision to trigger an ERR_EXIT is pushed down the
> evaluation stack. In "if cmd1; then cmd2; else cmd3; fi", only the
> evaluations of the (word codes representing the) commands "cmd1", "cmd2",
> or "cmd3" can ever trigger an ERR_EXIT. The evaluation of the (word codes
> representing the) if/then/else itself never triggers an ERR_EXIT. In other
> words only (the word codes representing) "basic commands", like function
> calls or UNIX commands, can ever trigger an ERR_EXIT. This strategy has the
> benefit that ERR_EXIT will be triggered exactly at the point where the
> fatal non-zero exit status was produced.
>
> 00 if
> 01   cmd1
> 02  then
> 03   cmd2
> 04. else
> 05.   cmd3
> 06 fi
>
> In the example above, with the strategy I described, and with the
> knowledge that the if condition never triggers an ERR_EXIT, it's guaranteed
> that an ERR_EXIT will only ever be thrown at line 03 or 05. If the
> triggering of the ERR_EXIT was sometimes delayed and delegated to the
> if/then/else, then ERR_EXIT could also be triggered at line 02 or 04, or
> worse at line 01 or 06, which wouldn't let you know whether the non-zero
> status originated from "cmd2" or from "cmd3". The delayed/delegated
> triggering looks undesirable because it gives you less information on the
> origin of the error. My understanding is that it's also never needed.
>
> The behavior of ERR_EXIT is controlled by the variables "noerrexit" and
> "local_noerrexit". My understanding of these variables is the following:
>
> - noerrexit: This variable is set to indicate that the triggering of
> ERR_EXIT must be disabled in the evaluation of any word code from the point
> where it's set until it's reset. For example it's set here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L550>
> in execif before the evaluation of the condition and reset here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L576>,
> here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L578>,
> here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L580>,
> and here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L585>
> after the evaluation of the condition. I don't really understand why the
> reseting is so complicated. It's much more straightforward in execwhile (
> here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L440>
> ).
>
> - local_noerrexit: This variable is set to indicate that the triggering of
> ERR_EXIT must be disabled in the remainder of the evaluation of the current
> word code. For example it's set at the end of each compound command, like
> here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/loop.c#L481>.
> This used to be a plain "this_noerrexit = 1", which I don't think was wrong.
>
> I think my patches so far have uncovered a different bug that was
>> already present but was masked by the foregoing, which is, that
>> noerrexit is unwound in cases where it should not be.  I think this is
>> happening at lines 1530-1531 of exec.c, right under the comment about
>> "hairy code near the end of execif()".  That's an area I didn't touch,
>> but I'm pretty sure it's restoring noerrexit to its state before
>> entering the "if" (oldnoerrexit) when it should be preserving the
>> state from the "&&" conditional.  In 5.8.1 this gets reversed again
>> via this_noerrexit.
>
>
> I must admit that I don't understand the NOERREXIT_UNTIL_EXEC logic here
> <https://github.com/zsh-users/zsh/blob/b1533066ca7d50c88b37ce72093c12cf19807818/Src/exec.c#L1529>,
> nor the complicated resetting logic of noerrexit at the end of execif. I
> was about to say that this doesn't seem to be the source of the problem
> because if, while, and for statements all behave the same in Zsh.
>
> function fooIf1()    { init; cond=true; if    $cond; then cond=false;
>> false        ; fi  ; }
>> function fooIf2()    { init; cond=true; if    $cond; then cond=false;
>> false && true; fi  ; }
>
>
>
> function fooWhile1() { init; cond=true; while $cond; do   cond=false;
>> false        ; done; }
>> function fooWhile2() { init; cond=true; while $cond; do   cond=false;
>> false && true; done; }
>
>
>
> function fooFor1()   { init; cond=true; for v in x ; do   cond=false;
>> false        ; done; }
>> function fooFor2()   { init; cond=true; for v in x ; do   cond=false;
>> false && true; done; }
>
>
> In the examples above fooIf1, fooWhile1, and fooFor1 all work correctly
> but fooIf2, fooWhile2, and fooFor2 fail to trigger ERR_EXIT in Zsh 5.8 and
> trigger it too early (in foo instead of in bar) in Zsh 5.9.
>
> However, if I comment out the NOERREXIT_UNTIL_EXEC logic in exec.c (or
> remove the negation), then fooIf2 and surprisingly also fooFor2 work
> correctly in Zsh 5.9 but not fooWhile2!?! fooWhile2 still triggers too
> early.
>
> So it looks like this may indeed be the start of the answer. But I'm still
> scratching my head on why that is.
>
> Philippe
>
>