Re: BUG: "return 0 && x=abc" does not skip parameter assignment

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

Below is a patch that fixes the problem and a few additional issues.

With the latest version of zsh, the following works:

$ () { while true; do return 1 || v=X; done }; echo v=$v

v=

but the version without loop fails to shortcut the assignment:

$ () { return 1 || v=X }; echo v=$v

v=X

This is because the implementation of return (and exit) pretends to perform a break, which in my opinion is bogus. It's also apparently unnecessary. All implementations of loops in loop.c take the retflag into account. Removing that code only breaks the test that was meant to test the return shortcutting described here.

To fix the second example above, the code implementing the shortcutting of && and || needs to take not only break/continue statements into account but also return statements. In addition to that, the current shortcutting for break/continue statements doesn't shortcut enough. The shortcutting of && only shortcuts further && operations but not || operation (and vice-versa). Thus, the following currently fails:

$ while true; do break && false || v=X; done; echo v=$v

v=X

Finally, ! shortcutting only takes return statements into account but not break/continue statements. The following works

$ () { ! return }; echo $?

0

but not this:

$ while true; do ! break; done; echo $?

1

The following patch fixes all these issues.

- Fix break/return shortcutting of &&, ||, and !

Philippe

diff --git a/Src/builtin.c b/Src/builtin.c
index 64a27511f..0506d8d33 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5811,7 +5811,6 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 	if ((isset(INTERACTIVE) && isset(SHINSTDIN))
 	    || locallevel || sourcelevel) {
 	    retflag = 1;
-	    breaks = loops;
 	    lastval = num;
 	    if (trap_state == TRAP_STATE_PRIMED && trap_return == -2
 		/*
@@ -5851,7 +5850,6 @@ bin_break(char *name, char **argv, UNUSED(Options ops), int func)
 		if (trap_state) 
 		    trap_state = TRAP_STATE_FORCE_RETURN;
 		retflag = 1;
-		breaks = loops;
 		exit_pending = 1;
 		exit_level = locallevel;
 		exit_val = num;
diff --git a/Src/exec.c b/Src/exec.c
index b0794abab..b1a0652da 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1558,12 +1558,14 @@ execlist(Estate state, int dont_change_job, int exiting)
 		 * we find a sublist followed by ORNEXT.                   */
 		if (((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			    execsimple(state) :
-			    execpline(state, code, Z_SYNC, 0)) || breaks) {
+			    execpline(state, code, Z_SYNC, 0)) ||
+		    breaks || retflag) {
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
 		    while (wc_code(code) == WC_SUBLIST &&
-			   WC_SUBLIST_TYPE(code) == WC_SUBLIST_AND) {
+			   (breaks || retflag ||
+			    WC_SUBLIST_TYPE(code) == WC_SUBLIST_AND)) {
 			state->pc = next;
 			code = *state->pc++;
 			next = state->pc + WC_SUBLIST_SKIP(code);
@@ -1591,12 +1593,14 @@ execlist(Estate state, int dont_change_job, int exiting)
 		 * we find a sublist followed by ANDNEXT.              */
 		if (!(((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_SIMPLE) ?
 			     execsimple(state) :
-			     execpline(state, code, Z_SYNC, 0))) || breaks) {
+			     execpline(state, code, Z_SYNC, 0))) ||
+		    breaks || retflag) {
 		    state->pc = next;
 		    code = *state->pc++;
 		    next = state->pc + WC_SUBLIST_SKIP(code);
 		    while (wc_code(code) == WC_SUBLIST &&
-			   WC_SUBLIST_TYPE(code) == WC_SUBLIST_OR) {
+			   (breaks || retflag ||
+			    WC_SUBLIST_TYPE(code) == WC_SUBLIST_OR)) {
 			state->pc = next;
 			code = *state->pc++;
 			next = state->pc + WC_SUBLIST_SKIP(code);
@@ -2036,7 +2040,7 @@ execpline(Estate state, wordcode slcode, int how, int last1)
 	}
 	else
 	    unqueue_signals();
-	if ((slflags & WC_SUBLIST_NOT) && !errflag && !retflag)
+	if ((slflags & WC_SUBLIST_NOT) && !errflag && !retflag && !breaks)
 	    lastval = !lastval;
     }
     if (!pline_level)
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index eea6ede2b..db48de8a9 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -982,38 +982,72 @@ F:its expectations.
  fn
 1:! does not affect return status of explicit return
 
+ for x in 1; do
+   ! continue
+ done
+0:! does not affect status of explicit continue
+
   msg=unset
-  for x in 1 2 3 4 5; do
+  for x in 1; do
     continue && msg=set && print Not executed
     print Not executed, neither.
   done
-  print $msg
-0:continue causes immediate continuation
->unset
+  print $?-$msg
+0:continue causes immediate continuation (immediate assign)
+>0-unset
+
+  msg=unset
+  for x in 1; do
+    continue && false || msg=set && print Not executed
+    print Not executed, neither.
+  done
+  print $?-$msg
+0:continue causes immediate continuation (assign after operator change)
+>0-unset
 
   msg=unset
   () {
-    return && msg=set && print Not executed
-    print Not executed, not nor neither.
+    return 7 || msg=set && print Not executed
+    print Not executed, neither.
   }
-  print $msg
-0:return causes immediate return
->unset
+  print $?-$msg
+0:return causes immediate return (immediate assign)
+>7-unset
 
   msg=unset
-  for x in 1 2 3 4 5; do
+  () {
+    return 7 || true && msg=set && print Not executed
+    print Not executed, neither.
+  }
+  print $?-$msg
+0:return causes immediate return (assign after operator change)
+>7-unset
+
+  $ZTST_testdir/../Src/zsh -fc '
+    msg=unset
+    () {
+      return 7 || msg=set && print Not executed
+      print Not executed, neither.
+    }
+    print $?-$msg
+  '
+0:return causes immediate return (no enclosing loops)
+>7-unset
+
+  msg=unset
+  for x in 1; do
     ! continue || msg=set && print Not executed
     print Not executed, neither.
   done
-  print $msg
+  print $?-$msg
 0:! continue causes immediate continuation
->unset
+>0-unset
 
   msg=unset
   () {
-    ! return || msg=set && print Not executed
-    print Not executed, not nor neither.
+    ! return 7 && msg=set && print Not executed
+    print Not executed, neither.
   }
-  print $msg
+  print $?-$msg
 0:! return causes immediate return
->unset
+>7-unset