Re: Simplify internal_catch()

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1104 bytes --]

Stefan Monnier wrote:
> At least I know Richard made me undo some "Lisp_Object foo = ..." years
> ago, saying that we shouldn't do that because it fails to compile in
> some cases (presumably when you used some specific compiler and you also
> asked for Lisp_Object to be a union rather than an int).
> I did not heed his advice very eagerly, to tell you the truth.

Many pre-C89 compilers indeed did not support union initializers (i.e., 'union u 
x = { something };', so perhaps there was some confusion about what worked and 
what didn't. Anyway, that water passed under the bridge long ago.

In reviewing this patch, I noticed some other confusion in this area that I 
introduced in 2013 while pacifying gcc -Wclobbered, and which I think you asked 
about but I didn't understand your question at the time (sorry). I attempted to 
fix the problem just now by installing the attached patch, which undoes Chris's 
simplification and then reworks the setjmp calls to use a local rather than a 
global. This avoid a need to load from a static variable in the normal flow of 
execution.

[-- Attachment #2: 0001-Clarify-internal_catch-etc.txt --]
[-- Type: text/plain, Size: 5526 bytes --]

From 535ef18ed523862db405d22ec4bea0bbfd4172ce Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 31 Dec 2016 13:10:38 -0800
Subject: [PATCH] Clarify internal_catch etc.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The recent change to internal_catch and friends relied on some
confusion I introduced to the code in 2013.  Attempt to fix
the confusion by clarifying the code instead.  This saves an
instruction and a load dependency in the typical case.
* src/eval.c (internal_catch, internal_condition_case)
(internal_condition_case_1, internal_condition_case_2)
(internal_condition_case_n): Undo the previous change.  Instead,
use use ‘c’ rather than ‘handlerlist’ in the typical case.
Also, use ‘eassert’ rather than ‘clobbered_eassert’ when possible.
---
 src/eval.c | 91 +++++++++++++++++++++++++++++---------------------------------
 1 file changed, 43 insertions(+), 48 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 01ae3a1..b174738 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1085,19 +1085,22 @@ internal_catch (Lisp_Object tag,
 {
   /* This structure is made part of the chain `catchlist'.  */
   struct handler *c = push_handler (tag, CATCHER);
-  Lisp_Object val;
 
   /* Call FUNC.  */
   if (! sys_setjmp (c->jmp))
-    val = func (arg);
-  else
     {
-      /* Throw works by a longjmp that comes right here.  */
-      val = handlerlist->val;
+      Lisp_Object val = func (arg);
+      eassert (handlerlist == c);
+      handlerlist = c->next;
+      return val;
+    }
+  else
+    { /* Throw works by a longjmp that comes right here.  */
+      Lisp_Object val = handlerlist->val;
+      clobbered_eassert (handlerlist == c);
+      handlerlist = handlerlist->next;
+      return val;
     }
-  clobbered_eassert (handlerlist == c);
-  handlerlist = handlerlist->next;
-  return val;
 }
 
 /* Unwind the specbind, catch, and handler stacks back to CATCH, and
@@ -1311,22 +1314,20 @@ internal_condition_case (Lisp_Object (*bfun) (void), Lisp_Object handlers,
 			 Lisp_Object (*hfun) (Lisp_Object))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  Lisp_Object val;
-  bool call_hfun;
-
   if (sys_setjmp (c->jmp))
     {
-      val = handlerlist->val;
-      call_hfun = true;
+      Lisp_Object val = handlerlist->val;
+      clobbered_eassert (handlerlist == c);
+      handlerlist = handlerlist->next;
+      return hfun (val);
     }
   else
     {
-      val = bfun ();
-      call_hfun = false;
+      Lisp_Object val = bfun ();
+      eassert (handlerlist == c);
+      handlerlist = c->next;
+      return val;
     }
-  clobbered_eassert (handlerlist == c);
-  handlerlist = handlerlist->next;
-  return call_hfun ? hfun (val) : val;
 }
 
 /* Like internal_condition_case but call BFUN with ARG as its argument.  */
@@ -1337,22 +1338,20 @@ internal_condition_case_1 (Lisp_Object (*bfun) (Lisp_Object), Lisp_Object arg,
 			   Lisp_Object (*hfun) (Lisp_Object))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  Lisp_Object val;
-  bool call_hfun;
-
   if (sys_setjmp (c->jmp))
     {
-      val = handlerlist->val;
-      call_hfun = true;
+      Lisp_Object val = handlerlist->val;
+      clobbered_eassert (handlerlist == c);
+      handlerlist = handlerlist->next;
+      return hfun (val);
     }
   else
     {
-      val = bfun (arg);
-      call_hfun = false;
+      Lisp_Object val = bfun (arg);
+      eassert (handlerlist == c);
+      handlerlist = c->next;
+      return val;
     }
-  clobbered_eassert (handlerlist == c);
-  handlerlist = handlerlist->next;
-  return call_hfun ? hfun (val) : val;
 }
 
 /* Like internal_condition_case_1 but call BFUN with ARG1 and ARG2 as
@@ -1366,22 +1365,20 @@ internal_condition_case_2 (Lisp_Object (*bfun) (Lisp_Object, Lisp_Object),
 			   Lisp_Object (*hfun) (Lisp_Object))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  Lisp_Object val;
-  bool call_hfun;
-
   if (sys_setjmp (c->jmp))
     {
-      val = handlerlist->val;
-      call_hfun = true;
+      Lisp_Object val = handlerlist->val;
+      clobbered_eassert (handlerlist == c);
+      handlerlist = handlerlist->next;
+      return hfun (val);
     }
   else
     {
-      val = bfun (arg1, arg2);
-      call_hfun = false;
+      Lisp_Object val = bfun (arg1, arg2);
+      eassert (handlerlist == c);
+      handlerlist = c->next;
+      return val;
     }
-  clobbered_eassert (handlerlist == c);
-  handlerlist = handlerlist->next;
-  return call_hfun ? hfun (val) : val;
 }
 
 /* Like internal_condition_case but call BFUN with NARGS as first,
@@ -1397,22 +1394,20 @@ internal_condition_case_n (Lisp_Object (*bfun) (ptrdiff_t, Lisp_Object *),
 						Lisp_Object *args))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  Lisp_Object val;
-  bool call_hfun;
-
   if (sys_setjmp (c->jmp))
     {
-      val = handlerlist->val;
-      call_hfun = true;
+      Lisp_Object val = handlerlist->val;
+      clobbered_eassert (handlerlist == c);
+      handlerlist = handlerlist->next;
+      return hfun (val, nargs, args);
     }
   else
     {
-      val = bfun (nargs, args);
-      call_hfun = false;
+      Lisp_Object val = bfun (nargs, args);
+      eassert (handlerlist == c);
+      handlerlist = c->next;
+      return val;
     }
-  clobbered_eassert (handlerlist == c);
-  handlerlist = handlerlist->next;
-  return call_hfun ? hfun (val, nargs, args) : val;
 }
 
 struct handler *
-- 
2.7.4