For loop bug

For loop bug

[Philippe Troin]

Tried on zsh 4.0.4 and 4.0.6:

  % for i in 1 2 3; do echo $i || break; done && echo X
  1
  X
  2
  X
  3
  X
  X
  % 

That looks somewhat incorrect...

Removing the "|| break" makes it work as expected:

  % for i in 1 2 3; do echo $i; done && echo X 
  1
  2
  3
  X
  % 

Same with grouping the echo and break statements:

  % for i in 1 2 3; do {echo $i || break}; done && echo X
  1
  2
  3
  X
  % 

Or did I miss something?

Phil.

Re: For loop bug

[Peter Stephenson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]

Philippe Troin wrote:
> Tried on zsh 4.0.4 and 4.0.6:
> 
>   % for i in 1 2 3; do echo $i || break; done && echo X
>   1
>   X
>   2
>   X
>   3
>   X
>   X
>   % 
> 
> That looks somewhat incorrect...

Oh, *yuk*.

> Or did I miss something?

Non, hélas.

The wordcode is pretty horrific to debug: it's uncommented and as an
unstructured array of integers completely opaque to all debugging tools.
Tentative patch below --- passes all tests, including the new one (which
Philippe will recognise).  It's actually against 4.1, but I doubt if
much has changed here.

I may leave this till Monday for people to scratch their heads over.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.45
diff -u -r1.45 exec.c
--- Src/exec.c	5 Aug 2002 12:35:59 -0000	1.45
+++ Src/exec.c	13 Sep 2002 23:18:57 -0000
@@ -877,8 +877,15 @@
 			 * for this sublist.                                   */
 			donetrap = 1;
 			goto sublist_done;
-		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
+			/*
+			 * Treat this in the same way as if we reached
+			 * the end of the sublist normally.
+			 */
 			donetrap = 1;
+			state->pc = next;
+			goto sublist_done;
+		    }
 		}
 		cmdpush(CS_CMDOR);
 		break;
Index: Test/A01grammar.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v
retrieving revision 1.5
diff -u -r1.5 A01grammar.ztst
--- Test/A01grammar.ztst	22 Aug 2001 15:59:27 -0000	1.5
+++ Test/A01grammar.ztst	13 Sep 2002 23:18:58 -0000
@@ -319,3 +319,14 @@
   done < /dev/null | { read name; print done }
 0:Bug regression: `while' loop with redirection and pipeline
 >done
+
+# This used to be buggy and print X at the end of each loop.
+  for f in 1 2 3 4; do
+    print $f || break
+  done && print X
+0:Handling of ||'s and &&'s with a for loop in between
+>1
+>2
+>3
+>4
+>X

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: For loop bug

[Bart Schaefer]

On Sep 14, 12:31am, Peter Stephenson wrote:
}
} Tentative patch below --- passes all tests, including the new one (which
} Philippe will recognise).  It's actually against 4.1, but I doubt if
} much has changed here.

Applies cleanly to the patches branch and passes "make check".

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: For loop bug

[Philippe Troin]

Peter Stephenson <pws@pwstephenson.fsnet.co.uk> writes:

> Philippe Troin wrote:
> > Tried on zsh 4.0.4 and 4.0.6:
> > 
> >   % for i in 1 2 3; do echo $i || break; done && echo X
> >   1
> >   X
> >   2
> >   X
> >   3
> >   X
> >   X
> >   % 
> > 
> > That looks somewhat incorrect...
> 
> Oh, *yuk*.
> 
> > Or did I miss something?
> 
> Non, hélas.
> 
> The wordcode is pretty horrific to debug: it's uncommented and as an
> unstructured array of integers completely opaque to all debugging tools.
> Tentative patch below --- passes all tests, including the new one (which
> Philippe will recognise).  It's actually against 4.1, but I doubt if
> much has changed here.
> 
> I may leave this till Monday for people to scratch their heads over.

Does it fix this one as well?

  % for i in 1 2 3; do { {echo $i; [[ $i == 2 ]] && exit 1; } || break }; \
  > done && echo X
  1
  X
  % 

I would have expected:

  1
  2
  X 

or 

  1
  2

But surely not 1 X.

Phil.

Re: For loop bug

[Bart Schaefer]

On Sep 13,  5:58pm, Philippe Troin wrote:
} Subject: Re: For loop bug
}
} Peter Stephenson <pws@pwstephenson.fsnet.co.uk> writes:
} 
} Does it fix this one as well?
} 
}   % for i in 1 2 3; do { {echo $i; [[ $i == 2 ]] && exit 1; } || break }; \
}   > done && echo X
}   1
}   X
}   % 

That is not a bug.

zagzig% [[ x == y ]] && true
zagzig% echo $?
1

Hence { [[ $i == 2 ]] && exit 1; } is false, and { false || break } means
break.  The && expression is only true if -both- branches are true.

Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
likely produce something like

login:

which might surprise you even more.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: For loop bug

[Peter Stephenson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]

Peter Stephenson wrote:

[Hey, Mozilla handles RFC822 downloads from the mailing list archive as
a special type which saves as a regular mailbox!  Cool!  Don't bother
telling me you've known for years.]

> Philippe Troin wrote:
> > Or did I miss something?
> 
> Non, hélas.

I did, however: after fifteen hours, it finally occurred to me that
where there's an or there's an and.  This replaces the previous patch
and fixes the equivalent bug, too.  (I've subtly changed a couple of
comments owing to pure pedantry.)  All tests still pass.  I'll still
wait and see if Sven has any comment.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.45
diff -u -r1.45 exec.c
--- Src/exec.c	5 Aug 2002 12:35:59 -0000	1.45
+++ Src/exec.c	14 Sep 2002 14:35:00 -0000
@@ -851,8 +851,15 @@
 			 * for this sublist.                                   */
 			donetrap = 1;
 			goto sublist_done;
-		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
 			donetrap = 1;
+			/*
+			 * Treat this in the same way as if we reached
+			 * the end of the sublist normally.
+			 */
+			state->pc = next;
+			goto sublist_done;
+		    }
 		}
 		cmdpush(CS_CMDAND);
 		break;
@@ -877,8 +884,15 @@
 			 * for this sublist.                                   */
 			donetrap = 1;
 			goto sublist_done;
-		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END)
+		    } else if (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END) {
 			donetrap = 1;
+			/*
+			 * Treat this in the same way as if we reached
+			 * the end of the sublist normally.
+			 */
+			state->pc = next;
+			goto sublist_done;
+		    }
 		}
 		cmdpush(CS_CMDOR);
 		break;
Index: Test/A01grammar.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A01grammar.ztst,v
retrieving revision 1.5
diff -u -r1.5 A01grammar.ztst
--- Test/A01grammar.ztst	22 Aug 2001 15:59:27 -0000	1.5
+++ Test/A01grammar.ztst	14 Sep 2002 14:35:00 -0000
@@ -319,3 +319,21 @@
   done < /dev/null | { read name; print done }
 0:Bug regression: `while' loop with redirection and pipeline
 >done
+
+# This used to be buggy and print X at the end of each iteration.
+  for f in 1 2 3 4; do
+    print $f || break
+  done && print X
+0:Handling of ||'s and &&'s with a for loop in between
+>1
+>2
+>3
+>4
+>X
+
+# Same bug for &&, used to print `no' at the end of each iteration
+  for f in 1 2 3 4; do
+    false && print strange
+  done || print no
+0:Handling of &&'s and ||'s with a for loop in between
+>no

-- 
Peter Stephenson <pws@pwstephenson.fsnet.co.uk>
Work: pws@csr.com
Web: http://www.pwstephenson.fsnet.co.uk

Re: For loop bug

[Philippe Troin]

"Bart Schaefer" <schaefer@brasslantern.com> writes:

> On Sep 13,  5:58pm, Philippe Troin wrote:
> } Subject: Re: For loop bug
> }
> } Peter Stephenson <pws@pwstephenson.fsnet.co.uk> writes:
> } 
> } Does it fix this one as well?
> } 
> }   % for i in 1 2 3; do { {echo $i; [[ $i == 2 ]] && exit 1; } || break }; \
> }   > done && echo X
> }   1
> }   X
> }   % 
> 
> That is not a bug.
> 
> zagzig% [[ x == y ]] && true
> zagzig% echo $?
> 1
> 
> Hence { [[ $i == 2 ]] && exit 1; } is false, and { false || break } means
> break.  The && expression is only true if -both- branches are true.

Agreed.

> Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
> likely produce something like
> 
> login:
> 
> which might surprise you even more.

I'm not sure I get this part.

But shouldn't it print:

  1
  2
  X

???

The echo $i with i set to 2 occurs before the exit and break
statements.

Phil.

Re: For loop bug

[Bart Schaefer]

[Excerpts reordered for clarity.]

On Sep 15,  9:58pm, Philippe Troin wrote:
} Subject: Re: For loop bug
}
} "Bart Schaefer" <schaefer@brasslantern.com> writes:
} 
} > On Sep 13,  5:58pm, Philippe Troin wrote:
} > } 
} > }   % for i in 1 2 3; do { {echo $i; [[ $i == 2 ]] && exit 1; } || break };
} > }   > done && echo X
} > }   1
} > }   X
} > }   % 
} > 
} > That is not a bug.
}
} But shouldn't it print:
} 
}   1
}   2
}   X

No, it shouldn't.  "for i in 1 2 3 ..."

OK, so the first time around the loop i=1, and zsh echoes 1.

Now we're testing [[ $i == 2 ]], which is [[ 1 == 2 ]] which is false;
hence we have { false && exit 1; } which doesn't call exit but also
doesn't evaluate as true, because the first branch of the && is false.

So the outer expression becomes { { false } || break }; and the loop ends
immediately.

} The echo $i with i set to 2 occurs before the exit and break
} statements.

$i never has a chance to get set to 2.

} > Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
} > likely produce something like
} > 
} > login:
} > 
} > which might surprise you even more.
} 
} I'm not sure I get this part.

If you call { exit 1 } from an interactive zsh, the shell will exit and
you'll end up back at your login prompt (or your xterm will disappear or
your remote connection will drop or whatever).  Expressions in { } are
evaluated in the current shell.  If it were ( exit 1 ) with parens, that's
evaluated in a subshell and the interactive shell would keep running.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: For loop bug

[Philippe Troin]

"Bart Schaefer" <schaefer@brasslantern.com> writes:

> [Excerpts reordered for clarity.]
> 
> On Sep 15,  9:58pm, Philippe Troin wrote:
> } Subject: Re: For loop bug
> }
> } "Bart Schaefer" <schaefer@brasslantern.com> writes:
> } 
> } > On Sep 13,  5:58pm, Philippe Troin wrote:
> } > } 
> } > }   % for i in 1 2 3; do { {echo $i; [[ $i == 2 ]] && exit 1; } || break };
> } > }   > done && echo X
> } > }   1
> } > }   X
> } > }   % 
> } > 
> } > That is not a bug.
> }
> } But shouldn't it print:
> } 
> }   1
> }   2
> }   X
> 
> No, it shouldn't.  "for i in 1 2 3 ..."
> 
> OK, so the first time around the loop i=1, and zsh echoes 1.
> 
> Now we're testing [[ $i == 2 ]], which is [[ 1 == 2 ]] which is false;
> hence we have { false && exit 1; } which doesn't call exit but also
> doesn't evaluate as true, because the first branch of the && is false.
> 
> So the outer expression becomes { { false } || break }; and the loop ends
> immediately.
> 
> } The echo $i with i set to 2 occurs before the exit and break
> } statements.
> 
> $i never has a chance to get set to 2.

Got that part. Thanks for the clarifications.
 
> } > Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
> } > likely produce something like
> } > 
> } > login:
> } > 
> } > which might surprise you even more.
> } 
> } I'm not sure I get this part.
> 
> If you call { exit 1 } from an interactive zsh, the shell will exit and
> you'll end up back at your login prompt (or your xterm will disappear or
> your remote connection will drop or whatever).  Expressions in { } are
> evaluated in the current shell.  If it were ( exit 1 ) with parens, that's
> evaluated in a subshell and the interactive shell would keep running.

Indeed. However my version of zsh does not exit: it goes on happily as
if nothing happened.

Phil.

Re: For loop bug

[Hans Dieter Pearcey]

On Mon, Sep 16, 2002 at 11:26:45AM -0700, Philippe Troin wrote:
> "Bart Schaefer" <schaefer@brasslantern.com> writes:
> > $i never has a chance to get set to 2.
> 
> Got that part. Thanks for the clarifications.
>  
> > } > Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
>
> Indeed. However my version of zsh does not exit: it goes on happily as
> if nothing happened.

Because, as above, $i never gets to 2, and so the [[ 2 == 2 ]] && exit 1
is not reached.

hdp.

Re: For loop bug

[Philippe Troin]

Hans Dieter Pearcey <hdp@pobox.com> writes:

> On Mon, Sep 16, 2002 at 11:26:45AM -0700, Philippe Troin wrote:
> > "Bart Schaefer" <schaefer@brasslantern.com> writes:
> > > $i never has a chance to get set to 2.
> > 
> > Got that part. Thanks for the clarifications.
> >  
> > > } > Further, { } is not a subshell, so { [[ 2 == 2 ]] && exit 1; } would most
> >
> > Indeed. However my version of zsh does not exit: it goes on happily as
> > if nothing happened.
> 
> Because, as above, $i never gets to 2, and so the [[ 2 == 2 ]] && exit 1
> is not reached.

You're absolutely right. No bugs here :-)

Phil.

Re: For loop bug

[Peter Stephenson]

I've committed the second version of this patch to both branches.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: For loop bug

[Sven Wischnowsky]

Peter Stephenson wrote:

> I did, however: after fifteen hours, it finally occurred to me that
> where there's an or there's an and.  This replaces the previous patch
> and fixes the equivalent bug, too.  (I've subtly changed a couple of
> comments owing to pure pedantry.)  All tests still pass.  I'll still
> wait and see if Sven has any comment.

Sorry for the delay, I was (and actually still am ;-) on holiday.

The patch looks sensible as far as I can see. I'm wondering why we
didn't see the bug before. Weird.


Bye
  Sven

-- 
Sven Wischnowsky                          wischnow@berkom.de