`return` does not behave properly under `!`

`return` does not behave properly under `!`

[Michael Greenberg]

Following up an example from Harald van Dijk on the dash mailing list, I
discovered that zsh does not `return` properly under a `!`. Here are
four example programs:

```
f() {
    while return 5
    do  
        echo fail while1
        break
    done    
}
f
echo $?

g() {
    while ! return 6
    do  
        echo fail while2
        break
    done    
}
g
echo $?

h() {
    while return 7 && return 8
    do  
      echo fail while3
      break 
    done
}
h
echo $?

i() {
  ! return 9
  echo fail not
}
i
echo $?
```

I would expect the output:

```
5
6
7
9
```

But zsh produces:

```
5
0
7
0
```

The dash shell has a similar bug (no link, since the mailing list
archives aren't up yet), as does yash
<https://osdn.net/projects/yash/ticket/46224>.

Cheers,
Michael

Re: `return` does not behave properly under `!`

[Bart Schaefer]

On Tue, Dec 6, 2022 at 10:19 AM Michael Greenberg
<michael@greenberg.science> wrote:
>
> Following up an example from Harald van Dijk on the dash mailing list, I
> discovered that zsh does not `return` properly under a `!`.

This appears to have been fixed by Philippe Altherr's series of
patches for ERR_EXIT et al, zsh-workers 51001, 51071, 51076.

Re: `return` does not behave properly under `!`

[Stephane Chazelas]

2022-12-06 13:18:59 -0500, Michael Greenberg:
[...]
> i() {
>   ! return 9
>   echo fail not
> }
> i
> echo $?
> ```
[...]
> 0
[...]

That:

! return n

Behaves like return !n (and ! return behave like return !$? (or
return "$(( ! $? ))" in POSIX syntax) doesn't sound totally off
to me. As written, I would say the POSIX spec may even be
requiring it.

That's consistent with ! break causing the while/until loop it's
in returning with failure which I've seen code rely upon (AFAIK,
that's the only way to get those loops to return with failure;
doesn't work in pdksh and derivatives though).

-- 
Stephane

Re: `return` does not behave properly under `!`

[Michael Greenberg]

Hi Stephane and Bart (reply inline below, as it's relevant to Stephane),

On 2022-12-07 at 08:42:55 AM, Stephane Chazelas wrote:

> 2022-12-06 13:18:59 -0500, Michael Greenberg:
> [...]
>> i() {
>>   ! return 9
>>   echo fail not
>> }
>> i
>> echo $?
>> ```
> [...]
>> 0
> [...]
>
> That:
>
> ! return n
>
> Behaves like return !n (and ! return behave like return !$? (or
> return "$(( ! $? ))" in POSIX syntax) doesn't sound totally off
> to me. As written, I would say the POSIX spec may even be
> requiring it.

Here's what the POSIX spec says about `return`:

  The return utility shall cause the shell to stop executing the
  current function or dot script. If the shell is not currently
  executing a function or dot script, the results are unspecified.

  ...

  EXIT STATUS

  The value of the special parameter '?' shall be set to n, an unsigned
  decimal integer, or to the exit status of the last command executed if
  n is not specified. If n is not an unsigned decimal integer, or is
  greater than 255, the results are unspecified. When return is executed
  in a trap action, the last command is considered to be the command
  that executed immediately preceding the trap action.

And here's what it says about `!` on pipelines:

  If the pipeline does not begin with the ! reserved word, the exit
  status shall be the exit status of the last command specified in the
  pipeline. Otherwise, the exit status shall be the logical NOT of the
  exit status of the last command. That is, if the last command returns
  zero, the exit status shall be 1; if the last command returns greater
  than zero, the exit status shall be zero.

So my reading is the opposite: `return` doesn't have an exit status,
because it causes the currently executing function (or sourced script)
to terminate; it does, however, set $?. So `!` shouldn't affect it.

But even without a close reading of the spec, it's just not how early
return works in any language. Having the evaluation context beyond
`return` affect the returned value subverts the early and immediate
return---and, in so doing, invites bugs.

So far, dash, yash, and zsh are the only shells I've found with this
behavior, and the dash folks seem to agree that it's an undesirable
bug. And also...

On 2022-12-06 at 08:53:25 PM, Bart Schaefer wrote:

> On Tue, Dec 6, 2022 at 10:19 AM Michael Greenberg
> <michael@greenberg.science> wrote:
>>
>> Following up an example from Harald van Dijk on the dash mailing list, I
>> discovered that zsh does not `return` properly under a `!`.
>
> This appears to have been fixed by Philippe Altherr's series of
> patches for ERR_EXIT et al, zsh-workers 51001, 51071, 51076.

Ah---thank you! I wish there were a straightforward fulltext search of
the archives (though I don't know I would have found this anyway, as it
doesn't explicitly mention `return`). I'll try to test these patches to
confirm.

> That's consistent with ! break causing the while/until loop it's
> in returning with failure which I've seen code rely upon (AFAIK,
> that's the only way to get those loops to return with failure;
> doesn't work in pdksh and derivatives though).

That seems hardly relevant here: a return explicitly sets $?, unlike
break/continue (which both have exit statuses, unlike return).

Cheers,
Michael

Re: `return` does not behave properly under `!`

[Stephane Chazelas]

2022-12-07 10:05:34 -0500, Michael Greenberg:
> Hi Stephane and Bart (reply inline below, as it's relevant to Stephane),
[...]
> So my reading is the opposite: `return` doesn't have an exit status,
> because it causes the currently executing function (or sourced script)
> to terminate; it does, however, set $?. So `!` shouldn't affect it.
[...]

Agreed.

> But even without a close reading of the spec, it's just not how early
> return works in any language. Having the evaluation context beyond
> `return` affect the returned value subverts the early and immediate
> return---and, in so doing, invites bugs.
[...]

Agreed.

> So far, dash, yash, and zsh are the only shells I've found with this
> behavior, and the dash folks seem to agree that it's an undesirable
> bug. And also...

Also bosh.

I can't see why one would call ! return if not to expect that to
have an effect on the exit status.

$ zsh -c 'f() { ! { false || return; }; }; f; echo "$?"'
0

Is more of a problem though and I agree would be an undesirable bug..

[...]
> Ah---thank you! I wish there were a straightforward fulltext search of
> the archives (though I don't know I would have found this anyway, as it
> doesn't explicitly mention `return`). I'll try to test these patches to
> confirm.

You can download and rsync the full archive from
rsync://rsync.zsh.org/mla/zsh-workers/

And search in there.

$ rsync rsync.zsh.org::
========================================================
ZSH FTP primary site (rsync.zsh.org)

Example usage:
rsync -av --delete rsync://rsync.zsh.org/pub /local/path/to/zsh/pub

Always do a dry run (-n) first or --delete may bite you!
========================================================
Modules:
========================================================


pub             Zsh FTP Pub directory (~280MB as of Feb 2010)
mla             Zsh Mailing List Archive directory (~333MB as of Feb 2010)


> > That's consistent with ! break causing the while/until loop it's
> > in returning with failure which I've seen code rely upon (AFAIK,
> > that's the only way to get those loops to return with failure;
> > doesn't work in pdksh and derivatives though).
> 
> That seems hardly relevant here: a return explicitly sets $?, unlike
> break/continue (which both have exit statuses, unlike return).
[...]

They're still special builtins that break the flow, so I would
still consider it relevant. I've not checked zsh recently but
there are funny things you can do with some shells by calling
break/continue in a function where the corresponding "enclosing"
loop is in one of the callers. IIRC there was some POSIX
discussion about that and the text tightened.

-- 
Stephane

Re: `return` does not behave properly under `!`

[Bart Schaefer]

On Wed, Dec 7, 2022 at 7:05 AM Michael Greenberg
<michael@greenberg.science> wrote:
>
> On 2022-12-06 at 08:53:25 PM, Bart Schaefer wrote:
>
> > This appears to have been fixed by Philippe Altherr's series of
> > patches for ERR_EXIT et al, zsh-workers 51001, 51071, 51076.
>
> Ah---thank you!

Sorry, I'm wrong.  I just re-tested and got the 5/0/7/0 behavior.

Re: `return` does not behave properly under `!`

[Peter Stephenson]

> On 07/12/2022 16:01 Stephane Chazelas <stephane@chazelas.org> wrote:
> I can't see why one would call ! return if not to expect that to
> have an effect on the exit status.

I agree with Michael's argument that shows that logically it
can't have an effect on the status, whatever you want.  The return
happens immediately, so the effect of any status within the function
is not taken into account.  It's not actually any more use adding a
! in front than using

return $?
(( ! $? ))

Even if the ! had an effect, it would just be on the effect of
falling off the end, which you've explicitly told it you don't want.
I suppose a more precise parallel would be

{
  true
  return $?
} always {
  return (( !$? ))
}

which returns 0 as the second return comes too late to affect
the return status.

pws

Re: `return` does not behave properly under `!`

[Bart Schaefer]

On Wed, Dec 7, 2022 at 8:17 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> I agree with Michael's argument that shows that logically it
> can't have an effect on the status, whatever you want.

That's going to be ugly, because "return" is implemented as a builtin
that propagates lastval.  One level out, at the WC_SUBLIST_NOT
implementation, it has no idea that lastval came from "return" rather
than from any arbitrary sublist.

Re: `return` does not behave properly under `!`

[Michael Greenberg]

On 2022-12-07 at 04:01:13 PM, Stephane Chazelas wrote:

> 2022-12-07 10:05:34 -0500, Michael Greenberg:
>> Hi Stephane and Bart (reply inline below, as it's relevant to Stephane),
> [...]
>> So my reading is the opposite: `return` doesn't have an exit status,
>> because it causes the currently executing function (or sourced script)
>> to terminate; it does, however, set $?. So `!` shouldn't affect it.
> [...]
>
> Agreed.
>
>> But even without a close reading of the spec, it's just not how early
>> return works in any language. Having the evaluation context beyond
>> `return` affect the returned value subverts the early and immediate
>> return---and, in so doing, invites bugs.
> [...]
>
> Agreed.
>
>> So far, dash, yash, and zsh are the only shells I've found with this
>> behavior, and the dash folks seem to agree that it's an undesirable
>> bug. And also...
>
> Also bosh.
>
> I can't see why one would call ! return if not to expect that to
> have an effect on the exit status.
>
> $ zsh -c 'f() { ! { false || return; }; }; f; echo "$?"'
> 0
>
> Is more of a problem though and I agree would be an undesirable bug..

Like in your example, I think the pernicious case is when the ! and the
return are far apart. The scenario I imagine is something like:

f() {
  while ! { SOME_CONDITION_FOR_EARLY_EXIT && return 1 
            SOME_CONDITION_FOR_TERMINATION_WITH_POSTPROCESSING 
          }
  do 
    ...
  done
  ...
}

In process of running the loop's predicate, the script might realize
that it's just time to exit the function early (without
postprocessing). The outer negation in the predicate is meant to negate
the whole predicate, not the return. It would be surprising to have that
distant `!` affect the return (and having it negate the status would
make it impossible to return statuses other than 0 and 1 in this form).

>
> [...]
>> Ah---thank you! I wish there were a straightforward fulltext search of
>> the archives (though I don't know I would have found this anyway, as it
>> doesn't explicitly mention `return`). I'll try to test these patches to
>> confirm.
>
> You can download and rsync the full archive from
> rsync://rsync.zsh.org/mla/zsh-workers/

Oh---I didn't realize this rsync interface existed, thank you! Might be
good to document it on https://www.zsh.org/mla/.

>> That seems hardly relevant here: a return explicitly sets $?, unlike
>> break/continue (which both have exit statuses, unlike return).
> [...]
>
> They're still special builtins that break the flow, so I would
> still consider it relevant. I've not checked zsh recently but
> there are funny things you can do with some shells by calling
> break/continue in a function where the corresponding "enclosing"
> loop is in one of the callers. IIRC there was some POSIX
> discussion about that and the text tightened.

Yes, non-lexical break is a mess, with varying treatment in popular
shells (e.g., bash and dash disagree). Having ! affect return from a
distance amounts to a form of non-lexical return munging. As someone who
studies programming languages for a living, I would very much advise
against non-lexical control if we can avoid it. :)

I think there's a good opportunity to have shells in alignment here (and
maybe make POSIX more explicit): if zsh folks agree that return should
not be affected by an enclosing !, we can align bash/dash/zsh. (I'm
hopeful yash folks will agree, too.)

Cheers,
Michael

Re: `return` does not behave properly under `!`

[Peter Stephenson]

> On 07/12/2022 17:06 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Wed, Dec 7, 2022 at 8:17 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> >
> > I agree with Michael's argument that shows that logically it
> > can't have an effect on the status, whatever you want.
> 
> That's going to be ugly, because "return" is implemented as a builtin
> that propagates lastval.  One level out, at the WC_SUBLIST_NOT
> implementation, it has no idea that lastval came from "return" rather
> than from any arbitrary sublist.

Not particularly, though I'm not actually that fussed, since I can't
really see the use of this beyond a bit of somewhat arbitrary state
purity.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 1810fca5e..7049710c3 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1961,7 +1961,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 	}
 	else
 	    unqueue_signals();
-	if ((slflags & WC_SUBLIST_NOT) && !errflag)
+	if ((slflags & WC_SUBLIST_NOT) && !errflag && !retflag)
 	    lastval = !lastval;
     }
     if (!pline_level)
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 0312fe94e..b3aea1055 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -970,3 +970,15 @@ F:its expectations.
 0:Non-interactive shell command input is line buffered
 >Value is first
 >Value is second
+
+ fn() {
+   ! false
+ }
+0:! inverts the status of implicit return
+
+ fn () {
+   false
+   ! return
+ }
+ fn
+1:! does not affect return status of explicit return

Re: `return` does not behave properly under `!`

[Bart Schaefer]

On Wed, Dec 7, 2022 at 9:29 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> +       if ((slflags & WC_SUBLIST_NOT) && !errflag && !retflag)

Ah, I forgot we had "retflag" and was cringing at the thought of yet
another global.  I can now go back to cringing at the number of
globals we already have.

Re: `return` does not behave properly under `!`

[Michael Greenberg]

A tiny update:

On 2022-12-07 at 12:10:55 PM, Michael Greenberg wrote:

> I think there's a good opportunity to have shells in alignment here (and
> maybe make POSIX more explicit): if zsh folks agree that return should
> not be affected by an enclosing !, we can align bash/dash/zsh. (I'm
> hopeful yash folks will agree, too.)

yash maintainers agreed that this is a bug, which is now fixed
<https://osdn.net/projects/yash/ticket/46224>.

Hopefully zsh can follow suit (and perhaps we can raise this with POSIX
folks). I'd be happy to try for a patch, but I'm not at all familiar
with zsh's code...

Cheers,
Michael

Re: `return` does not behave properly under `!`

[Bart Schaefer]

On Fri, Dec 9, 2022 at 7:22 AM Michael Greenberg
<michael@greenberg.science> wrote:
>
> Hopefully zsh can follow suit (and perhaps we can raise this with POSIX
> folks). I'd be happy to try for a patch, but I'm not at all familiar
> with zsh's code...

Did you not see PWS's patch in workers/51125 ?  You were Cc'd ...

Re: `return` does not behave properly under `!`

[Michael Greenberg]

Derp! I _did_ see it, but apparently I forgot about it. Delighted zsh is
on board! :)

Cheers,
Michael

On 2022-12-09 at 08:21:53 AM, Bart Schaefer wrote:

> On Fri, Dec 9, 2022 at 7:22 AM Michael Greenberg
> <michael@greenberg.science> wrote:
>>
>> Hopefully zsh can follow suit (and perhaps we can raise this with POSIX
>> folks). I'd be happy to try for a patch, but I'm not at all familiar
>> with zsh's code...
>
> Did you not see PWS's patch in workers/51125 ?  You were Cc'd ...