[PATCH] Even more ERR_EXIT (was Re: More ERR_EXIT Re: Tests RE behavior of ERR_EXIT)

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Mon, Nov 14, 2022 at 5:11 PM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> [if workers/50929 makes 50897 redundant then] there's no point in
> bashing through if/case/for/while/repeat/select individually -- the
> only case we have to fix is this one:

I was half right.  Fixing that case revealed that it was necessary to
go through the others, because the "redundant" fix is one level too
far up the stack.

Attached patch removes 50929 again, and amends this_noerrexit from
50897 in each of the complex command exec* functions.  Finally it adds
tests to E01options for each of the cases that Philippe previously
outlined, with the exception of "select" which is interactive.

The tests from 50928 still pass with this patch.  I spent a ridiculous
amount of time with every change I made flopping back and forth
between C03 working and the new E01 failing and vice-versa, before
noticing what happened when I nested a brace expression inside another
one.

Also a minor fix for TRAPDEBUG that caused me some confusion earlier
in the process.

Are there more cases that need testing?

diff --git a/Src/exec.c b/Src/exec.c
index ce0c1f1ad..dc5d5dd23 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -451,7 +451,10 @@ execcursh(Estate state, int do_exec)
     cmdpop();
 
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
 
     return lastval;
 }
@@ -1442,8 +1445,6 @@ execlist(Estate state, int dont_change_job, int exiting)
 		    execsimple(state);
 		else
 		    execpline(state, code, ltype, (ltype & Z_END) && exiting);
-		if (!locallevel || unset(ERRRETURN))
-		    this_noerrexit = noerrexit;
 		state->pc = next;
 		goto sublist_done;
 		break;
@@ -1536,6 +1537,7 @@ sublist_done:
 	     */
 	    int oerrexit_opt = opts[ERREXIT];
 	    opts[ERREXIT] = 0;
+	    oldnoerrexit = noerrexit;
 	    noerrexit = NOERREXIT_EXIT | NOERREXIT_RETURN;
 	    exiting = donetrap;
 	    ret = lastval;
diff --git a/Src/loop.c b/Src/loop.c
index be5261369..cd2f5340b 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -208,7 +208,10 @@ execfor(Estate state, int do_exec)
     loops--;
     simple_pline = old_simple_pline;
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
     return lastval;
 }
 
@@ -336,7 +339,10 @@ execselect(Estate state, UNUSED(int do_exec))
     loops--;
     simple_pline = old_simple_pline;
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
     return lastval;
 }
 
@@ -478,7 +484,10 @@ execwhile(Estate state, UNUSED(int do_exec))
     popheap();
     loops--;
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
     return lastval;
 }
 
@@ -532,7 +541,10 @@ execrepeat(Estate state, UNUSED(int do_exec))
     loops--;
     simple_pline = old_simple_pline;
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
     return lastval;
 }
 
@@ -587,7 +599,10 @@ execif(Estate state, int do_exec)
 	    lastval = 0;
     }
     state->pc = end;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
 
     return lastval;
 }
@@ -701,7 +716,10 @@ execcase(Estate state, int do_exec)
 
     if (!anypatok)
 	lastval = 0;
-    this_noerrexit = (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END);
+    if (WC_SUBLIST_TYPE(*end) != WC_SUBLIST_END)
+	this_noerrexit = noerrexit;
+    else
+	this_noerrexit = 1;
 
     return lastval;
 }
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index d38fbed74..939a11a7a 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -385,6 +385,113 @@
 1:ERR_RETURN with additional levels
 >Executed
 
+  (
+    setopt ERR_EXIT
+    { false } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from simple brace expression
+
+  (
+    setopt ERR_EXIT
+    { false && true } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from conditional brace expression
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    { if true; then false; true; fi } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "if" with simple body
+
+  (
+    setopt ERR_EXIT
+    { if true; then false && true; fi } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "if" with conditional body
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    { if false; then :; else false; true; fi } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "else" with simple body
+
+  (
+    setopt ERR_EXIT
+    { if false; then :; else false && true; fi } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "else" with conditional body
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    { case x in; (*) false;; esac } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "case" with simple body
+
+  (
+    setopt ERR_EXIT
+    { case x in; (*) false && true;; esac } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "case" with conditional body
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    { for _ in 1; do false; true; done } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "for" with simple body
+
+  (
+    setopt ERR_EXIT
+    { for _ in 1; do false && true; done } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "for" with conditional body
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    () { { while ((argv[1]--)); do false; true; done
+         } always { print INSIDE: $? } } 1
+    print OUTSIDE
+  )
+1:ERR_EXIT from "while" with simple body
+
+  (
+    setopt ERR_EXIT
+    () { { while ((argv[1]--)); do false && true; done
+         } always { print INSIDE: $? } } 1
+    print OUTSIDE
+  )
+1:ERR_EXIT from "while" with conditional body
+>INSIDE: 1
+
+  (
+    setopt ERR_EXIT
+    { repeat $; do false; done } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "repeat" with simple body
+
+  (
+    setopt ERR_EXIT
+    { repeat 1; do false && true; done } always { print INSIDE: $? }
+    print OUTSIDE
+  )
+1:ERR_EXIT from "repeat" with conditional body
+>INSIDE: 1
+
   (print before; setopt noexec; print after)
 0:NO_EXEC option
 >before