GNU bug report logs - #24047
[PROPOSED PATCH] ‘signal’ no longer returns

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 24047 in the body.
You can then email your comments to 24047 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Cc: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: [PROPOSED PATCH]
 ‘signal’ no longer returns
Date: Thu, 21 Jul 2016 16:20:27 +0200

Although for decades ‘signal’ has been documented to not return,
a corner case in the Lisp debugger causes ‘signal’ to return.
Remove the corner case and adjust Emacs internals accordingly.
An alternative would be to document the corner case, but this
would complicate the Lisp API unnecessarily.
* src/eval.c (maybe_call_debugger): Now returns void, not bool.
Caller changed.
(Fsignal): Now _Noreturn.  Callers adjusted accordingly.
(xsignal): Move to lisp.h.
* src/lisp.h (process_quit_flag): Now _Noreturn.
(xsignal): Now an inline function, as it's now simply an alias
for Fsignal.
---
 src/charset.c  |    6 +++---
 src/coding.c   |    6 +++---
 src/eval.c     |   33 ++++++---------------------------
 src/keyboard.c |    4 ----
 src/lisp.h     |    8 ++++++--
 5 files changed, 18 insertions(+), 39 deletions(-)

diff --git a/src/charset.c b/src/charset.c
index 95a9c57..05469aa 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -843,9 +843,9 @@ range of code points (in CHARSET) of target characters.  */)
   int nchars;
 
   if (nargs != charset_arg_max)
-    return Fsignal (Qwrong_number_of_arguments,
-		    Fcons (intern ("define-charset-internal"),
-			   make_number (nargs)));
+    Fsignal (Qwrong_number_of_arguments,
+	     Fcons (intern ("define-charset-internal"),
+		    make_number (nargs)));
 
   attrs = Fmake_vector (make_number (charset_attr_max), Qnil);
 
diff --git a/src/coding.c b/src/coding.c
index 29c90f0..a8ddc81 100644
--- a/src/coding.c
+++ b/src/coding.c
@@ -10546,9 +10546,9 @@ assigned to each coding category (see `coding-category-list').
   return Qnil;
 
  short_args:
-  return Fsignal (Qwrong_number_of_arguments,
-		  Fcons (intern ("define-coding-system-internal"),
-			 make_number (nargs)));
+  Fsignal (Qwrong_number_of_arguments,
+	   Fcons (intern ("define-coding-system-internal"),
+		  make_number (nargs)));
 }
 
 
diff --git a/src/eval.c b/src/eval.c
index 72facd5..a850446 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1432,7 +1432,7 @@ struct handler *
 
 
 static Lisp_Object find_handler_clause (Lisp_Object, Lisp_Object);
-static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
+static void maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
 				 Lisp_Object data);
 
 void
@@ -1460,7 +1460,8 @@ static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
 See Info anchor `(elisp)Definition of signal' for some details on how this
 error message is constructed.
 If the signal is handled, DATA is made available to the handler.
-See also the function `condition-case'.  */)
+See also the function `condition-case'.  */
+       attributes: noreturn)
   (Lisp_Object error_symbol, Lisp_Object data)
 {
   /* When memory is full, ERROR-SYMBOL is nil,
@@ -1537,14 +1538,7 @@ static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
 	  /* Special handler that means "print a message and run debugger
 	     if requested".  */
 	  || EQ (h->tag_or_ch, Qerror)))
-    {
-      bool debugger_called
-	= maybe_call_debugger (conditions, error_symbol, data);
-      /* We can't return values to code which signaled an error, but we
-	 can continue code which has signaled a quit.  */
-      if (debugger_called && EQ (real_error_symbol, Qquit))
-	return Qnil;
-    }
+    maybe_call_debugger (conditions, error_symbol, data);
 
   if (!NILP (clause))
     {
@@ -1569,16 +1563,6 @@ static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
   fatal ("%s", SDATA (string));
 }
 
-/* Internal version of Fsignal that never returns.
-   Used for anything but Qquit (which can return from Fsignal).  */
-
-void
-xsignal (Lisp_Object error_symbol, Lisp_Object data)
-{
-  Fsignal (error_symbol, data);
-  emacs_abort ();
-}
-
 /* Like xsignal, but takes 0, 1, 2, or 3 args instead of a list.  */
 
 void
@@ -1700,7 +1684,7 @@ static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
     = SIG is the error symbol, and DATA is the rest of the data.
     = SIG is nil, and DATA is (SYMBOL . REST-OF-DATA).
       This is for memory-full errors only.  */
-static bool
+static void
 maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig, Lisp_Object data)
 {
   Lisp_Object combined_data;
@@ -1719,12 +1703,7 @@ static bool maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig,
       && ! skip_debugger (conditions, combined_data)
       /* RMS: What's this for?  */
       && when_entered_debugger < num_nonmacro_input_events)
-    {
-      call_debugger (list2 (Qerror, combined_data));
-      return 1;
-    }
-
-  return 0;
+    call_debugger (list2 (Qerror, combined_data));
 }
 
 static Lisp_Object
diff --git a/src/keyboard.c b/src/keyboard.c
index 8901ff0..6b5528d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -10402,13 +10402,9 @@ shows the events before all translations (except for input methods).
 	 then quit right away.  */
       if (immediate_quit && NILP (Vinhibit_quit))
 	{
-	  struct gl_state_s saved;
-
 	  immediate_quit = false;
 	  pthread_sigmask (SIG_SETMASK, &empty_mask, 0);
-	  saved = gl_state;
 	  Fsignal (Qquit, Qnil);
-	  gl_state = saved;
 	}
       else
         { /* Else request quit when it's safe.  */
diff --git a/src/lisp.h b/src/lisp.h
index 39877d7..50ee8ea 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3230,7 +3230,7 @@ struct handler
 extern void process_pending_signals (void);
 extern bool volatile pending_signals;
 
-extern void process_quit_flag (void);
+extern _Noreturn void process_quit_flag (void);
 #define QUIT						\
   do {							\
     if (!NILP (Vquit_flag) && NILP (Vinhibit_quit))	\
@@ -3879,7 +3879,11 @@ extern void map_obarray (Lisp_Object, void (*) (Lisp_Object, Lisp_Object),
 extern Lisp_Object run_hook_with_args (ptrdiff_t nargs, Lisp_Object *args,
 				       Lisp_Object (*funcall)
 				       (ptrdiff_t nargs, Lisp_Object *args));
-extern _Noreturn void xsignal (Lisp_Object, Lisp_Object);
+INLINE _Noreturn void
+xsignal (Lisp_Object error_symbol, Lisp_Object data)
+{
+  Fsignal (error_symbol, data);
+}
 extern _Noreturn void xsignal0 (Lisp_Object);
 extern _Noreturn void xsignal1 (Lisp_Object, Lisp_Object);
 extern _Noreturn void xsignal2 (Lisp_Object, Lisp_Object, Lisp_Object);
-- 
1.7.9.5

Message #8 received at 24047 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 24047 <at> debbugs.gnu.org
Subject: Re: Should 'signal' sometimes return?
Date: Fri, 22 Jul 2016 14:16:58 +0200

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

Attached is a more-conservative version of the patch, which keeps the 
debug-on-quit behavior when the user types C-g, so that 'signal' never 
returns but C-g might still "return". This is in response to Stefan's 
email at 
<http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00958.html>.

[0001-signal-no-longer-returns.patch (text/x-patch, attachment)]

Message #11 received at 24047 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 24047 <at> debbugs.gnu.org
Subject: Re: bug#24047: Should 'signal' sometimes return?
Date: Sat, 23 Jul 2016 14:37:36 -0400

> Attached is a more-conservative version of the patch, which keeps the
> debug-on-quit behavior when the user types C-g, so that 'signal' never
> returns but C-g might still "return". This is in response to Stefan's email
> at <http://lists.gnu.org/archive/html/emacs-devel/2016-07/msg00958.html>.

Haven't looked in detail, but it looks OK.

A further patch could change the new `quit' function so it checks
debug-on-quit, and either calls the debugger or calls Fsignal.  This way
we don't need the intermediate signal_or_quit function.


        Stefan

Message #16 received at 24047-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 24047-done <at> debbugs.gnu.org
Subject: Re: bug#24047: Should 'signal' sometimes return?
Date: Mon, 25 Jul 2016 00:42:27 +0200

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

On 07/23/2016 08:37 PM, Stefan Monnier wrote:
> Haven't looked in detail, but it looks OK.

Thanks, I installed it in 'master' and am marking this as done.

>
> A further patch could change the new `quit' function so it checks
> debug-on-quit, and either calls the debugger or calls Fsignal.  This way
> we don't need the intermediate signal_or_quit function.

I tried something along those lines (see attached), but wasn't convinced 
that the result was correct (there are a lot of flags flying around), 
and my patch would cause quits to traverse the handler list twice, which 
seems inelegant.

[attempt.diff (text/x-patch, attachment)]

Previous Next

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