Re: Exception handling and "trap" vs. TRAPNAL()

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Oct 2,  8:09pm, Peter Stephenson wrote:
}
} I'm not sure there's any code change I would feel comfortable with.
} [...] I don't see anything that's [...] obviously inconsistent
} provided the manual is written correctly... except for the
} odd case of the error in the TRAP function, which I'm currently too
} confused even to think about straight.

It's really pretty simple.  The whole problem is the "else if (errflag)"
branch in dotrapargs().  It's a no-op for inline traps because of later
tests of (isfunc).

If we want to suppress errors/interrupts from trap functions, so they
behave the same as inline traps, we simply delete that branch:

-------------------------------------------------------------------
Index: Src/signals.c
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/signals.c,v
retrieving revision 1.13
diff -c -r1.13 signals.c
--- Src/signals.c	28 May 2005 04:32:49 -0000	1.13
+++ Src/signals.c	2 Oct 2005 19:31:00 -0000
@@ -1097,8 +1097,7 @@
 	 * execrestore.
 	 */
 	trapret = trapreturn + 1;
-    } else if (errflag)
-	trapret = 1;
+    }
     execrestore();
     lexrestore();
 
-------------------------------------------------------------------

Note that an effect of suppressing errors in this way is that it
becomes difficult to send a keyboard interrupt to the shell, because
interrupts that arrive during execution of a trap will be ignored.

If we want to propagate errors that occur inside traps, and thereby
make it possible for inline traps to "throw exceptions" in "always"
blocks, we do this instead (more discussion below patch):

-------------------------------------------------------------------
Index: Src/signals.c
===================================================================
diff -c -r1.13 signals.c
--- Src/signals.c	28 May 2005 04:32:49 -0000	1.13
+++ Src/signals.c	2 Oct 2005 19:22:57 -0000
@@ -1003,6 +1003,7 @@
     int trapret = 0;
     int obreaks = breaks;
     int isfunc;
+    int traperr;
 
     /* if signal is being ignored or the trap function		      *
      * is NULL, then return					      *
@@ -1097,8 +1098,8 @@
 	 * execrestore.
 	 */
 	trapret = trapreturn + 1;
-    } else if (errflag)
-	trapret = 1;
+    }
+    traperr = errflag;
     execrestore();
     lexrestore();
 
@@ -1110,6 +1111,7 @@
 	    lastval = trapret-1;
 	}
     } else {
+	errflag |= traperr;
 	breaks += obreaks;
 	if (breaks > loops)
 	    breaks = loops;
-------------------------------------------------------------------

The |= there is to retain any true value of errflag from the context
restored by lexrestore().  That may be impossible in the first place,
but I'm not certain.

Note that the "if" for the "else" in that third hunk is:

    if (trapret > 0) {

With the change in the second hunk, (trapret > 0) is true if and only
if an explicit "return" statement was executed within the trap (either
inline or function).  When (traperr) is true an error/interrupt must
have occurred in the trap, and it therefore did not call "return",
and therefore (trapret > 0) must be false.