Re: 'set -e' with '!' POSIX issue

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sun, 2 Oct 2016 11:01:18 +0100
Martijn Dekker <martijn@xxxxxxxx> wrote:
> 'set -e' ('set -o errexit') is not POSIX compliant on zsh because it
> doesn't ignore a command beginning with "!".

I don't really like the errexit logic as there are too many horrible
cases, since the place we need to test is different from where the code
is being executed (look at the particularly horrible oldnoeerrexit
logic).  This was not entirely trivial but we might get away with it.

The tests are for ERR_RETURN because that's easier to write; most of the
logic is shared with ERR_EXIT.

pws

diff --git a/Src/exec.c b/Src/exec.c
index 349414c..a429428 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -1226,6 +1226,7 @@ execlist(Estate state, int dont_change_job, int exiting)
     }
     while (wc_code(code) == WC_LIST && !breaks && !retflag && !errflag) {
 	int donedebug;
+	int this_noerrexit = 0;
 
 	ltype = WC_LIST_TYPE(code);
 	csp = cmdsp;
@@ -1309,9 +1310,12 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    goto sublist_done;
 	}
 	while (wc_code(code) == WC_SUBLIST) {
+	    int isend = (WC_SUBLIST_TYPE(code) == WC_SUBLIST_END);
 	    next = state->pc + WC_SUBLIST_SKIP(code);
 	    if (!oldnoerrexit)
-		noerrexit = (WC_SUBLIST_TYPE(code) != WC_SUBLIST_END);
+		noerrexit = !isend;
+	    if ((WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT) && isend)
+		this_noerrexit = 1;
 	    switch (WC_SUBLIST_TYPE(code)) {
 	    case WC_SUBLIST_END:
 		/* End of sublist; just execute, ignoring status. */
@@ -1427,7 +1431,7 @@ sublist_done:
 	/* Check whether we are suppressing traps/errexit *
 	 * (typically in init scripts) and if we haven't  *
 	 * already performed them for this sublist.       */
-	if (!noerrexit && !donetrap) {
+	if (!noerrexit && !this_noerrexit && !donetrap) {
 	    if (sigtrapped[SIGZERR] && lastval) {
 		dotrap(SIGZERR);
 		donetrap = 1;
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index 83c05aa..3a65b28 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -519,6 +519,43 @@
 >Yes
 F:Must be tested with a top-level script rather than source or function
 
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      false
+      print after
+  }
+  fn
+1:ERRRETURN, basic case
+>before
+
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      ! true
+      ! false
+      print after
+  }
+  fn
+0:ERRETURN with "!"
+>before
+>after
+
+  fn() {
+      emulate -L zsh
+      setopt errreturn
+      print before
+      ! true
+      ! false
+      false
+      print after
+  }
+  fn
+1:ERRETURN with "!" and a following false
+>before
+
 %clean
 
   rm -f TRAPEXIT