Subject: bug#25289: closed (Re: bug#25289: Simplify calls to setjmp for condition case)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 31 Dec 2016 11:39:02 +0000

Your bug report

#25289: Simplify calls to setjmp for condition case

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
25289: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=25289
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#25289: Simplify calls to setjmp for condition case
From: Eli Zaretskii <[email-masked]>
To: Chris Gregory <[email-masked]>
Cc: [email-masked]
Date: Sat, 31 Dec 2016 13:38:26 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> From: Chris Gregory <[email-masked]>
> Date: Wed, 28 Dec 2016 18:32:18 -0600
> 
> This patch simplifies calls to setjmp, while keeping the semantics of
> setjmp valid (it must be called in an if statement or some loop with
> a comparison operator).
> 
> This takes the common bodies out of the branches.

Thanks, installed on the master branch.

I'm afraid this is the last change we can accept from you before your
legal paperwork is completed.

Subject: Simplify calls to setjmp for condition case
From: Chris Gregory <[email-masked]>
To: [email-masked]
Date: Wed, 28 Dec 2016 18:32:18 -0600
List-Id: <debbugs-submit.debbugs.gnu.org>

This patch simplifies calls to setjmp, while keeping the semantics of
setjmp valid (it must be called in an if statement or some loop with
a comparison operator).

This takes the common bodies out of the branches.

-- 
Chris Gregory

diff --git a/src/eval.c b/src/eval.c
index e50e26a..6d50b98 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1084,22 +1084,19 @@ 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))
-    {
-      Lisp_Object val = func (arg);
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return val;
-    }
+  // This if statement is not redundant. setjmp must be called in an
+  // if statement.
+  if (sys_setjmp (c->jmp) != 0)
+    /* Throw works by a longjmp that comes right here. */
+    val = handlerlist->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;
-    }
+    val = func (arg);
+
+  clobbered_eassert (handlerlist == c);
+  handlerlist = handlerlist->next;
+  return val;
 }
 
 /* Unwind the specbind, catch, and handler stacks back to CATCH, and
@@ -1313,20 +1310,24 @@ internal_condition_case (Lisp_Object (*bfun) (void), Lisp_Object handlers,
 			 Lisp_Object (*hfun) (Lisp_Object))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  if (sys_setjmp (c->jmp))
+  Lisp_Object val;
+  bool is_recursive;
+
+  // This if statement is not redundant. setjmp must be called in an
+  // if statement.
+  if (sys_setjmp (c->jmp) != 0)
     {
-      Lisp_Object val = handlerlist->val;
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return hfun (val);
+      is_recursive = true;
+      val = handlerlist->val;
     }
   else
     {
-      Lisp_Object val = bfun ();
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return val;
+      is_recursive = false;
+      val = bfun ();
     }
+  clobbered_eassert (handlerlist == c);
+  handlerlist = handlerlist->next;
+  return is_recursive ? hfun (val) : val;
 }
 
 /* Like internal_condition_case but call BFUN with ARG as its argument.  */
@@ -1337,20 +1338,24 @@ 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);
-  if (sys_setjmp (c->jmp))
+  Lisp_Object val;
+  bool is_recursive;
+
+  // This if statement is not redundant. setjmp must be called in an
+  // if statement.
+  if (sys_setjmp (c->jmp) != 0)
     {
-      Lisp_Object val = handlerlist->val;
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return hfun (val);
+      is_recursive = true;
+      val = handlerlist->val;
     }
   else
     {
-      Lisp_Object val = bfun (arg);
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return val;
+      is_recursive = false;
+      val = bfun (arg);
     }
+  clobbered_eassert (handlerlist == c);
+  handlerlist = handlerlist->next;
+  return is_recursive ? hfun (val) : val;
 }
 
 /* Like internal_condition_case_1 but call BFUN with ARG1 and ARG2 as
@@ -1364,20 +1369,24 @@ internal_condition_case_2 (Lisp_Object (*bfun) (Lisp_Object, Lisp_Object),
 			   Lisp_Object (*hfun) (Lisp_Object))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  if (sys_setjmp (c->jmp))
+  Lisp_Object val;
+  bool is_recursive;
+
+  // This if statement is not redundant. setjmp must be called in an
+  // if statement.
+  if (sys_setjmp (c->jmp) != 0)
     {
-      Lisp_Object val = handlerlist->val;
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return hfun (val);
+      is_recursive = true;
+      val = handlerlist->val;
     }
   else
     {
-      Lisp_Object val = bfun (arg1, arg2);
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return val;
+      is_recursive = false;
+      val = bfun (arg1, arg2);
     }
+  clobbered_eassert (handlerlist == c);
+  handlerlist = handlerlist->next;
+  return is_recursive ? hfun (val) : val;
 }
 
 /* Like internal_condition_case but call BFUN with NARGS as first,
@@ -1393,20 +1402,24 @@ internal_condition_case_n (Lisp_Object (*bfun) (ptrdiff_t, Lisp_Object *),
 						Lisp_Object *args))
 {
   struct handler *c = push_handler (handlers, CONDITION_CASE);
-  if (sys_setjmp (c->jmp))
+  Lisp_Object val;
+  bool is_recursive;
+
+  // This if statement is not redundant. setjmp must be called in an
+  // if statement.
+  if (sys_setjmp (c->jmp) != 0)
     {
-      Lisp_Object val = handlerlist->val;
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return hfun (val, nargs, args);
+      is_recursive = true;
+      val = handlerlist->val;
     }
   else
     {
-      Lisp_Object val = bfun (nargs, args);
-      clobbered_eassert (handlerlist == c);
-      handlerlist = handlerlist->next;
-      return val;
+      is_recursive = false;
+      val = bfun (nargs, args);
     }
+  clobbered_eassert (handlerlist == c);
+  handlerlist = handlerlist->next;
+  return is_recursive ? hfun (val, nargs, args) : val;
 }
 
 struct handler *