Re: Bug with continue?

Re: Bug with continue?

[Mikael Magnusson]

On 3/28/23, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
>> On 28/03/2023 10:32 Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> I notice this works differently in zsh than in other shells:
>>
>>   for x in 1 2 3 4; do
>>     continue &&
>>     list="$list$x " &&
>>     echo "x: $x"
>>   done
>>   echo "list: $list"
>>
>> Why did the statement after `continue` gets evaluated?
>>
>> The original code tries to do something useful `case "$x" in 1)
>> continue ;; esac &&` but it shouldn't matter.
>>
>> I tried in bash, ksh, and dash, and all of them continue immediately,
>> except zsh.
>>
>> That can't be the desired behavior, can it?
>
> That looks like it probably ought to be regarded as a bug to me, yes ---
> I guess it's been hidden because the test "if this statement successfully
> jumped somewhere completely different then..." isn't spectacularly useful.
> However, it's not logically wrong, either.
>
> I think we had something a little similar to this recently; it usually
> boils down to something quite simple once you've found it and I'll
> have a look when I've got more time.  (Patch would go to zsh-workers.)

Some observations that may or may not be helpful:

Replacing continue && with ! continue || has the same effect,
chaining continue && false && a=b still does the assignment,
doing continue && { a=b } works, eg it does not do the assignment,
same with if continue; then ...

-- 
Mikael Magnusson

Re: Bug with continue?

[Peter Stephenson]

> On 28/03/2023 12:28 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 28/03/2023 11:17 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > > On 28/03/2023 10:32 Felipe Contreras <felipe.contreras@gmail.com> wrote:
> > > I notice this works differently in zsh than in other shells:
> > > 
> > >   for x in 1 2 3 4; do
> > >     continue &&
> > >     list="$list$x " &&
> > >     echo "x: $x"
> > >   done
> > >   echo "list: $list"
> > > 
> > > Why did the statement after `continue` gets evaluated?
>...
> The bug I'm thinking of is zsh-workers/51125, though it looks like I
> committed the fix under zsh-workers/51134.  That was about wheter
> "! return" should invert the status of the return given it's already
> returned by the time that would happen.  This is similar, but looks
> like it's not the same.
> 
> This one is a little weird as if the immediately next statement is a
> print it doesn't get executed.  I'm suspecting some subtlety with handling
> retflag.
> 
> pws

(I meant "breaks", of course.)  Redirected to zsh-workers as previously
advertised.

"breaks" means multiple things, but I think they all have the same
implication for a successful "continue".  If I can think of some
further useful tests I'll add them.

Added the corresponding test for return for safety.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 3330bbce8..6454e4ccf 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1491,7 +1491,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 		 * we find a sublist followed by ORNEXT.                   */
 		if ((ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			    execsimple(state) :
-			    execpline(state, code, Z_SYNC, 0)))) {
+			    execpline(state, code, Z_SYNC, 0))) || breaks) {
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index b3aea1055..88928980f 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -982,3 +982,21 @@ F:its expectations.
  }
  fn
 1:! does not affect return status of explicit return
+
+  msg=unset
+  for x in 1 2 3 4 5; do
+    continue && msg=set && print Not executed
+    print Not executed, neither.
+  done
+  print $msg
+0:continue causes immediate continuation
+>unset
+
+  msg=unset
+  () {
+    return && msg=set && print Not executed
+    print Not executed, not nor neither.
+  }
+  print $msg
+0:return causes immediate return
+>unset

Re: Bug with continue?

[Peter Stephenson]

> On 28/03/2023 12:19 Mikael Magnusson <mikachu@gmail.com> wrote:
> Replacing continue && with ! continue || has the same effect,

Missed this one, thanks.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 3330bbce8..4328975b9 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1491,7 +1491,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 		 * we find a sublist followed by ORNEXT.                   */
 		if ((ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			    execsimple(state) :
-			    execpline(state, code, Z_SYNC, 0)))) {
+			    execpline(state, code, Z_SYNC, 0))) || breaks) {
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
@@ -1524,7 +1524,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 		 * we find a sublist followed by ANDNEXT.              */
 		if (!(ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			     execsimple(state) :
-			     execpline(state, code, Z_SYNC, 0)))) {
+			     execpline(state, code, Z_SYNC, 0))) || breaks) {
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index b3aea1055..d57085798 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -982,3 +982,39 @@ F:its expectations.
  }
  fn
 1:! does not affect return status of explicit return
+
+  msg=unset
+  for x in 1 2 3 4 5; do
+    continue && msg=set && print Not executed
+    print Not executed, neither.
+  done
+  print $msg
+0:continue causes immediate continuation
+>unset
+
+  msg=unset
+  () {
+    return && msg=set && print Not executed
+    print Not executed, not nor neither.
+  }
+  print $msg
+0:return causes immediate return
+>unset
+
+  msg=unset
+  for x in 1 2 3 4 5; do
+    ! continue || msg=set && print Not executed
+    print Not executed, neither.
+  done
+  print $msg
+0:! continue causes immediate continuation
+>unset
+
+  msg=unset
+  () {
+    ! return || msg=set && print Not executed
+    print Not executed, not nor neither.
+  }
+  print $msg
+0:! return causes immediate return
+>unset

Re: Bug with continue?

[Peter Stephenson]

> On 28/03/2023 12:28 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > On 28/03/2023 11:17 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> > > On 28/03/2023 10:32 Felipe Contreras <felipe.contreras@gmail.com> wrote:
> > > I notice this works differently in zsh than in other shells:
> > > 
> > >   for x in 1 2 3 4; do
> > >     continue &&
> > >     list="$list$x " &&
> > >     echo "x: $x"
> > >   done
> > >   echo "list: $list"
> > > 
> > > Why did the statement after `continue` gets evaluated?
>
> This one is a little weird as if the immediately next statement is a
> print it doesn't get executed.

For what it's worth, I suspect this difference is because the
assignment is going through execsimple() rather than the execcmd_...
series.  As it's name suggests execsimple() doesn't do a lot of testing.

I'd rather not tinker with that; the fix we have should be good enough.

pws

Re: Bug with continue?

[Felipe Contreras]

On Tue, Mar 28, 2023 at 7:17 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > On 28/03/2023 12:19 Mikael Magnusson <mikachu@gmail.com> wrote:
> > Replacing continue && with ! continue || has the same effect,
>
> Missed this one, thanks.
>
> pws
>
> diff --git a/Src/exec.c b/Src/exec.c
> index 3330bbce8..4328975b9 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -1491,7 +1491,7 @@ execlist(Estate state, int dont_change_job, int exiting)
>                  * we find a sublist followed by ORNEXT.                   */
>                 if ((ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
>                             execsimple(state) :
> -                           execpline(state, code, Z_SYNC, 0)))) {
> +                           execpline(state, code, Z_SYNC, 0))) || breaks) {
>                     state->pc = next;
>                     code = *state->pc++;
>                     next = state->pc + WC_SUBLIST_SKIP(code);
> @@ -1524,7 +1524,7 @@ execlist(Estate state, int dont_change_job, int exiting)
>                  * we find a sublist followed by ANDNEXT.              */
>                 if (!(ret = ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
>                              execsimple(state) :
> -                            execpline(state, code, Z_SYNC, 0)))) {
> +                            execpline(state, code, Z_SYNC, 0))) || breaks) {
>                     state->pc = next;
>                     code = *state->pc++;
>                     next = state->pc + WC_SUBLIST_SKIP(code);

FWIW I can confirm this fixes the issue both in my toy case, and the
real code that prompted me to report the issue.

Thanks.

-- 
Felipe Contreras