From: Paul Eggert <eggert@cs.ucla.edu>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Chris Gregory <czipperz@gmail.com>, emacs-devel@gnu.org
Subject: Re: Simplify internal_catch()
Date: Sat, 31 Dec 2016 13:29:06 -0800 [thread overview]
Message-ID: <c50b84a5-a13d-f675-f251-d744a6e69cb7@cs.ucla.edu> (raw)
In-Reply-To: <jwvh95n4zzq.fsf-monnier+emacs@gnu.org>
[-- 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
next prev parent reply other threads:[~2016-12-31 21:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-28 6:33 Simplify internal_catch() Chris Gregory
2016-12-28 8:48 ` Andreas Schwab
2016-12-28 16:45 ` Stefan Monnier
2016-12-28 19:24 ` Paul Eggert
2016-12-28 19:28 ` Stefan Monnier
2016-12-31 21:29 ` Paul Eggert [this message]
-- strict thread matches above, loose matches on Subject: below --
2016-12-28 7:34 [no subject] Chris Gregory
2016-12-28 7:42 ` Simplify internal_catch () Paul Eggert
2016-12-28 1:54 Simplify internal_catch() Chris Gregory
2016-12-28 2:50 ` Stefan Monnier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c50b84a5-a13d-f675-f251-d744a6e69cb7@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=czipperz@gmail.com \
--cc=emacs-devel@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.