Re: errexit and (Z)ERR trap regression

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Wed, Jun 26, 2024 at 5:43 AM Philippe Altherr
<philippe.altherr@xxxxxxxxx> wrote:
>
> I think that the correct fix is the following:
>
>     if (isandor || isnot)
>         noerrexit = oldnoerrexit | NOERREXIT_EXIT | NOERREXIT_RETURN;
>     else
>         noerrexit = oldnoerrexit;

This doesn't seem necessary to me.  For one thing, it's equivalent to:

  noerrexit = oldnoerrexit;
  if (isandor || isnot)
    noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;

But assigning (noerrexit = oldnoerrexit) presumes that noerrexit was
(improperly?) cleared at or after the point where olderrexit was
recorded.  If that were the situation, then --

> For reminder, here is the current code:
>
>     if (isandor || isnot)
>         noerrexit |= NOERREXIT_EXIT | NOERREXIT_RETURN;

-- would be insufficient for the existing test cases, I think.  That
is, either (olderrexit == noerrexit), or it's not necessary to
OR-together olderrexit with the new values.

> Here are two other examples fixed by this patch:
>
>     zsh -c 'trap "echo Trapped!" ERR; true && if true; then false; fi'
>     zsh -c 'trap "echo Trapped!" ERR; true && {false} always {true}'

These two cases also pass with my patch from workers/52973.  Do you
have an example where my patch doesn't work?