GNU bug report logs - #25289
Simplify calls to setjmp for condition case

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Chris Gregory <czipperz <at> gmail.com>
Subject: bug#25289: closed (Re: bug#25289: Simplify calls to setjmp for
 condition case)
Date: Sat, 31 Dec 2016 11:39:02 +0000

[Message part 1 (text/plain, inline)]

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 25289 <at> debbugs.gnu.org.

-- 
25289: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=25289
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Eli Zaretskii <eliz <at> gnu.org>
To: Chris Gregory <czipperz <at> gmail.com>
Cc: 25289-done <at> debbugs.gnu.org
Subject: Re: bug#25289: Simplify calls to setjmp for condition case
Date: Sat, 31 Dec 2016 13:38:26 +0200

> From: Chris Gregory <czipperz <at> gmail.com>
> 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.

[Message part 3 (message/rfc822, inline)]

From: Chris Gregory <czipperz <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Simplify calls to setjmp for condition case
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.

-- 
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 *

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.