34065: following an "if" condition, do not test lastval for ERR_EXIT until a new command is run

Includes unposted regression tests.

4 files changed, 39 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 055cf2f21..52808bea4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2014-12-27  Barton E. Schaefer  <schaefer@zsh.org>
+
+	* unposted: Test/A05execution.ztst: regressions for 34065
+
+	* 34065: Src/exec.c, Src/loop.c: following an "if" condition, do
+	not test lastval for ERR_EXIT until a new command is run
+
 2014-12-21  Oliver Kiddle  <opk@zsh.org>
 
 	* Daniel Shahaf: 33977: Completion/Zsh/Command/_bindkey:
diff --git a/Src/exec.c b/Src/exec.c
index 6a7dbb1e1..eaf73df25 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2632,6 +2632,10 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	}
     }
 
+    /* if we get this far, it is OK to pay attention to lastval again */
+    if (noerrexit == 2 && !is_shfunc)
+	noerrexit = 0;
+
     /* Do prefork substitutions */
     esprefork = (assign || isset(MAGICEQUALSUBST)) ? PREFORK_TYPESET : 0;
     if (args && htok)
diff --git a/Src/loop.c b/Src/loop.c
index 8bb1ec9dd..7b3bdd2c8 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -102,7 +102,10 @@ execfor(Estate state, int do_exec)
 		addlinknode(args, dupstring(*x));
 	}
     }
-    /* lastval = 0; */
+
+    if (!args || empty(args))
+	lastval = 0;
+
     loops++;
     pushheap();
     cmdpush(CS_FOR);
@@ -238,10 +241,10 @@ execselect(Estate state, UNUSED(int do_exec))
     }
     if (!args || empty(args)) {
 	state->pc = end;
-	return 1;
+	return 0;
     }
     loops++;
-    /* lastval = 0; */
+
     pushheap();
     cmdpush(CS_SELECT);
     usezle = interact && SHTTY != -1 && isset(USEZLE);
@@ -519,14 +522,17 @@ execif(Estate state, int do_exec)
 	s = 1;
 	state->pc = next;
     }
-    noerrexit = olderrexit;
 
     if (run) {
+	/* we need to ignore lastval until we reach execcmd() */
+	noerrexit = olderrexit ? olderrexit : lastval ? 2 : 0;
 	cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN));
 	execlist(state, 1, do_exec);
 	cmdpop();
-    } else
+    } else {
+	noerrexit = olderrexit;
 	lastval = 0;
+    }
     state->pc = end;
 
     return lastval;
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index 042b2d0a5..cc2d34d23 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -269,3 +269,20 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
 1:The status of recently exited background jobs is recorded
 >3
 >2
+
+# Regression test for workers/34060 (patch in 34065)
+  setopt ERR_EXIT NULL_GLOB
+  if false; then :; else echo if:$?; fi
+  if false; then :; else for x in _*_; do :; done; echo for:$?; fi
+0:False "if" condition handled correctly by "for" loops with ERR_EXIT
+>if:1
+>for:0
+
+# Regression test for workers/34065 (uses setopt from preceding test)
+  select x; do :; done; echo $?
+  select x in; do :; done; echo $?
+  select x in _*_; do :; done; echo $?
+0:The status of "select" is zero when the loop body does not execute
+>0
+>0
+>0