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: Eli Zaretskii <eliz <at> gnu.org>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#25289: closed (Simplify calls to setjmp for condition case)
Date: Sat, 31 Dec 2016 11:39:02 +0000

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

Your message dated Sat, 31 Dec 2016 13:38:26 +0200
with message-id <83r34ogwgd.fsf <at> gnu.org>
and subject line Re: bug#25289: Simplify calls to setjmp for condition case
has caused the debbugs.gnu.org bug report #25289,
regarding Simplify calls to setjmp for condition case
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> 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: 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 *

[Message part 3 (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.

Previous Next

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