51076: fix ERR_EXIT when used with "eval" or "source"; documentary comments

ChangeLog

@@ -1,5 +1,8 @@
 2022-12-03  Bart Schaefer  <schaefer@zsh.org>
 
+	* Philippe Altherr: 51076: Src/exec.c, Test/C03traps.ztst: fix
+	ERR_EXIT when used with "eval" or "source"; documentary comments
+
 	* Philippe Altherr: 51071: Src/exec.c, Test/C03traps.ztst: fix
 	ERR_RETURN when a function using && / || is called within another
 	statement using && / ||

Src/exec.c

@@ -56,7 +56,17 @@ struct funcsave {
 typedef struct funcsave *Funcsave;
 
 /*
- * used to suppress ERREXIT and trapping of SIGZERR, SIGEXIT.
+ * Used to suppress ERREXIT and trapping of SIGZERR, SIGEXIT in the
+ * evaluation of sub-commands of the command under evaluation. The
+ * variable must be updated before the evaluation of the sub-commands
+ * starts and restored to its previous state right after that
+ * evaluation ends. The variable is read and acted upon in execlist.
+ *
+ * A good usage example can be found in execwhile in loop.c, which
+ * evaluates while statements. The variable is updated to disable
+ * ERREXIT just before evaluating the while's condition and restored
+ * to its previous state right after the evaluation of the condition.
+ *
  * Bits from noerrexit_bits.
  */
 
@@ -64,7 +74,36 @@ typedef struct funcsave *Funcsave;
 int noerrexit;
 
 /*
- * used to suppress ERREXIT and ERRRETURN for the command under evaluation.
+ * Used to suppress ERREXIT and ERRRETURN for the command under
+ * evaluation.  The variable must be enabled (set to 1) at the very
+ * end of the evaluation of the command. It must come after the
+ * evaluation of any sub-commands of the command under evaluation. The
+ * variable is read and acted upon in execlist, which also takes care
+ * of initialising and resetting it to 0.
+ *
+ * Unlike the variable noerrexit, whose state applies to the
+ * evaluation of whole sub-commands (and their direct and indirect
+ * sub-commands), the scope of the variable this_noerrexit is much
+ * more localized. ERREXIT and ERRRETURN are triggered at the end of
+ * the function execlist after the evaluation of some or all of the
+ * list's sub-commands. The role of the variable this_noerrexit is to
+ * give to the functions evaluating the list's sub-commands the
+ * possibility to tell the calling execlist not to trigger ERREXIT and
+ * ERRRETURN. In other words, the variable acts as an additional
+ * return value between the called evaluation functions and the
+ * calling execlist. For that reason the variable must always be set
+ * as late as possible and in particular after any sub-command
+ * evaluation. If the variable is set before the evaluation of a
+ * sub-command, if may affect the wrong execlist, if the sub-command
+ * evaluation involves another execlist call, and/or the variable may
+ * get modified by the sub-command evaluation and thus wouldn't return
+ * the desired value to the calling execlist.
+ *
+ * Good usage examples can be found in the exec functions in loop.c,
+ * which evaluate compound commands. The variable is enabled right
+ * before returning from the functions, after all the sub-commands of
+ * the compound commands have already been evaluated.
+ *
  * 0 or 1
  */
 
@@ -1427,6 +1466,7 @@ execlist(Estate state, int dont_change_job, int exiting)
 	    goto sublist_done;
 	}
 	while (wc_code(code) == WC_SUBLIST) {
+	    this_noerrexit = 0;
 	    int isandor = WC_SUBLIST_TYPE(code) != WC_SUBLIST_END;
 	    int isnot = WC_SUBLIST_FLAGS(code) & WC_SUBLIST_NOT;
 	    next = state->pc + WC_SUBLIST_SKIP(code);
@@ -1582,6 +1622,7 @@ sublist_done:
 	    break;
 	code = *state->pc++;
     }
+    this_noerrexit = 0;
     pline_level = old_pline_level;
     list_pipe = old_list_pipe;
     list_pipe_job = old_list_pipe_job;
@@ -5999,7 +6040,6 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
 	    trap_return++;
 	ret = lastval;
 	noerrexit = funcsave->noerrexit;
-	this_noerrexit = 0;
 	if (noreturnval) {
 	    lastval = funcsave->lastval;
 	    numpipestats = funcsave->numpipestats;

Test/C03traps.ztst

@@ -954,6 +954,47 @@ F:Must be tested with a top-level script rather than source or function
 1:ERR_EXIT triggered by status 1 at end of anon func
 >Still functioning
 
+  (setopt err_exit
+  loop=true; while print loop $? >&2; $loop; do loop=false; false && true; done
+  print done $? >&2
+  )
+0: ERR_EXIT neither triggered inside loop nor triggered by while statement
+?loop 0
+?loop 1
+?done 1
+
+  (setopt err_exit
+  { loop=true; while print loop $? >&2; $loop; do loop=false; false && true; done } || false
+  print done $? >&2
+  )
+1: ERR_EXIT not triggered inside loop but triggered by rhs of ||
+?loop 0
+?loop 1
+
+  (setopt err_exit
+  eval 'loop=true; while print loop $? >&2; $loop; do loop=false; false && true; done'
+  print done $? >&2
+  )
+1: ERR_EXIT not triggered inside loop but triggered by eval
+?loop 0
+?loop 1
+
+  (setopt err_exit
+  source <(echo 'loop=true; while print loop $? >&2; $loop; do loop=false; false && true; done')
+  print done $? >&2
+  )
+1: ERR_EXIT not triggered inside loop but triggered by source
+?loop 0
+?loop 1
+
+  (setopt err_exit
+  v=$(loop=true; while print loop $? >&2; $loop; do loop=false; false && true; done)
+  print done $? >&2
+  )
+1: ERR_EXIT not triggered inside loop but triggered by command substitution
+?loop 0
+?loop 1
+
   if zmodload zsh/system 2>/dev/null; then
   (
     trap 'echo TERM; exit 2' TERM