GNU bug report logs - #67196
M-: uses a wrong value of debug-on-error when it is nil.

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 67196 in the body.
You can then email your comments to 67196 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: Alan Mackenzie <acm <at> muc.de>
To: bug-gnu-emacs <at> gnu.org
Subject: M-: uses a wrong value of debug-on-error when it is nil.
Date: Wed, 15 Nov 2023 13:48:40 +0000

Hello, Emacs.

On the master branch (and probably any recent Emacs):

(i) emacs -Q
(ii) C-h v debug-on-error RET
(iii) M-: debug-on-error RET

From (ii) it will be seen that debug-on-error's value is nil.
(iii) wrongly reports that its value is t.  This is a bug.

#########################################################################

What is happening here is that eval-expression binds debug-on-error to
an uninterned symbol called "t", so that d-o-e will be set to non-nil
for the evaluation of the coming form.

This has the unwanted side effect of ignoring the actual value of d-o-e
in forms which use it.  For example, if the variable is set to a list of
error symbols, this value gets ignored on evaluating a form with M-:.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Wed, 15 Nov 2023 19:19:17 +0200

> Date: Wed, 15 Nov 2023 13:48:40 +0000
> From: Alan Mackenzie <acm <at> muc.de>
> 
> Hello, Emacs.
> 
> On the master branch (and probably any recent Emacs):
> 
> (i) emacs -Q
> (ii) C-h v debug-on-error RET
> (iii) M-: debug-on-error RET
> 
> >From (ii) it will be seen that debug-on-error's value is nil.
> (iii) wrongly reports that its value is t.  This is a bug.
> 
> #########################################################################
> 
> What is happening here is that eval-expression binds debug-on-error to
> an uninterned symbol called "t", so that d-o-e will be set to non-nil
> for the evaluation of the coming form.
> 
> This has the unwanted side effect of ignoring the actual value of d-o-e
> in forms which use it.  For example, if the variable is set to a list of
> error symbols, this value gets ignored on evaluating a form with M-:.

Aren't we doing this on purpose, Stefan?

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Wed, 15 Nov 2023 17:55:18 +0000

Hello, Eli.

On Wed, Nov 15, 2023 at 19:19:17 +0200, Eli Zaretskii wrote:
> > Date: Wed, 15 Nov 2023 13:48:40 +0000
> > From: Alan Mackenzie <acm <at> muc.de>

> > Hello, Emacs.

> > On the master branch (and probably any recent Emacs):

> > (i) emacs -Q
> > (ii) C-h v debug-on-error RET
> > (iii) M-: debug-on-error RET

> > >From (ii) it will be seen that debug-on-error's value is nil.
> > (iii) wrongly reports that its value is t.  This is a bug.

> > #########################################################################

> > What is happening here is that eval-expression binds debug-on-error to
> > an uninterned symbol called "t", so that d-o-e will be set to non-nil
> > for the evaluation of the coming form.

> > This has the unwanted side effect of ignoring the actual value of d-o-e
> > in forms which use it.  For example, if the variable is set to a list of
> > error symbols, this value gets ignored on evaluating a form with M-:.

> Aren't we doing this on purpose, Stefan?

Thinking about it more clearly, we're using one variable, debug-on-error,
for two conflicting purposes:
(i) The calling mechanism of execute-extended-command, including the
  invocation of the debugger, should an error be signalled.
(ii) Use in the form to be evaluated, where the variable's value outside
  of execute-extended-command should endure.

It would seem to me we really need two distinct variables for these two
purposes.  I think it's clear that purpose (ii) would be the one to
retain use of debug-on-error.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sun, 19 Nov 2023 12:19:49 -0500

> Thinking about it more clearly, we're using one variable, debug-on-error,
> for two conflicting purposes:
> (i) The calling mechanism of execute-extended-command, including the
>   invocation of the debugger, should an error be signalled.
> (ii) Use in the form to be evaluated, where the variable's value outside
>   of execute-extended-command should endure.

Yes, there's a conflicting need here: we want to control the way `eval`
works "from the outside", i.e. we want the `debug-on-error` to apply to
the evaluator but not to the code being evaluated.

But `eval` doesn't distinguish between the context of the evaluator and
the context of the code being evaluated because all ELisp code uses the
same evaluator.  I can't think of any way to split the variable into two
to solve the problem.  Instead it gives me the impression we're trying
some impossible feat like watching oneself sleep.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sun, 19 Nov 2023 19:33:48 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  67196 <at> debbugs.gnu.org
> Date: Sun, 19 Nov 2023 12:19:49 -0500
> 
> it gives me the impression we're trying some impossible feat like
> watching oneself sleep.

This is Emacs: the impossible we do today; miracles take a little
longer.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: dick <dick.r.chiang <at> gmail.com>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sun, 19 Nov 2023 14:30:40 -0500

Eli wrote:
> This is Emacs: the impossible we do today; miracles take
> a little longer.

To which dick wrote:
> I assure you your smug self-satisfaction is shared only by the
> unemployable programmers you compare yourself to.

Thank you Dick for the subtle comic relief.


        Stefan

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

From: Alan Mackenzie <acm <at> muc.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 17:10:47 +0000

Hello, Stefan.

On Sun, Nov 19, 2023 at 12:19:49 -0500, Stefan Monnier wrote:
> > Thinking about it more clearly, we're using one variable, debug-on-error,
> > for two conflicting purposes:
> > (i) The calling mechanism of execute-extended-command, including the
> >   invocation of the debugger, should an error be signalled.
> > (ii) Use in the form to be evaluated, where the variable's value outside
> >   of execute-extended-command should endure.

> Yes, there's a conflicting need here: we want to control the way `eval`
> works "from the outside", i.e. we want the `debug-on-error` to apply to
> the evaluator but not to the code being evaluated.

> But `eval` doesn't distinguish between the context of the evaluator and
> the context of the code being evaluated because all ELisp code uses the
> same evaluator.  I can't think of any way to split the variable into two
> to solve the problem.  Instead it gives me the impression we're trying
> some impossible feat like watching oneself sleep.

I can think of a way to do this (indeed, I've got a trial implementation
here which works).

Firstly, though, there is a bug in the doc string of
eval-expression-debug-on-error: rather than stating what the meaning of
the variable is, what it's for, it states the low level details of how
it achieves the desired effect.  This is needlessly restrictive.  I
propose changing that doc string from:

    If non-nil set `debug-on-error' to t in `eval-expression'.
    If nil, don't change the value of `debug-on-error'.

to something like:

    Non-nil means enter debugger on an error in a call from `eval-expression'.
    Does not apply to errors handled by `condition-case' or those
    matched by `debug-ignored-errors'.
    A nil value for this variable will not prevent an entry to
    the debugger caused by other variables such as `debug-on-error'.
    Like `debug-on-error', this variable's value can also be a list,
    with the same meaning as for `debug-on-error'.    

..  With this change, the mechanism for eval-expression-debug-on-error
can be changed such that it doesn't become confused with debug-on-error.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 20:48:40 +0200

> Date: Fri, 24 Nov 2023 17:10:47 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 67196 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> Firstly, though, there is a bug in the doc string of
> eval-expression-debug-on-error: rather than stating what the meaning of
> the variable is, what it's for, it states the low level details of how
> it achieves the desired effect.  This is needlessly restrictive.  I
> propose changing that doc string from:
> 
>     If non-nil set `debug-on-error' to t in `eval-expression'.
>     If nil, don't change the value of `debug-on-error'.
> 
> to something like:
> 
>     Non-nil means enter debugger on an error in a call from `eval-expression'.
>     Does not apply to errors handled by `condition-case' or those
>     matched by `debug-ignored-errors'.
>     A nil value for this variable will not prevent an entry to
>     the debugger caused by other variables such as `debug-on-error'.

First, the last two sentences above should be transposed, as the
second one is not related to the 1st one, but the 3rd one is.

And second, please try to reword so that the text is less complicated
and easier to understand.

Thanks.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 15:22:10 -0500

> I can think of a way to do this (indeed, I've got a trial implementation
> here which works).

Eager to see it,


        Stefan

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 20:54:45 +0000

Hello, Eli and Stefan.

On Fri, Nov 24, 2023 at 20:48:40 +0200, Eli Zaretskii wrote:
> > Date: Fri, 24 Nov 2023 17:10:47 +0000
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 67196 <at> debbugs.gnu.org
> > From: Alan Mackenzie <acm <at> muc.de>

> > Firstly, though, there is a bug in the doc string of
> > eval-expression-debug-on-error: rather than stating what the meaning of
> > the variable is, what it's for, it states the low level details of how
> > it achieves the desired effect.  This is needlessly restrictive.  I
> > propose changing that doc string from:

> >     If non-nil set `debug-on-error' to t in `eval-expression'.
> >     If nil, don't change the value of `debug-on-error'.

> > to something like:

> >     Non-nil means enter debugger on an error in a call from `eval-expression'.
> >     Does not apply to errors handled by `condition-case' or those
> >     matched by `debug-ignored-errors'.
> >     A nil value for this variable will not prevent an entry to
> >     the debugger caused by other variables such as `debug-on-error'.

> First, the last two sentences above should be transposed, as the
> second one is not related to the 1st one, but the 3rd one is.

Done.

> And second, please try to reword so that the text is less complicated
> and easier to understand.

I'm perhaps a bit too close to it.  Apart from the first line (for which
too much information needs squashing in), I can't really see much scope
for improvement.

> Thanks.

Anyway, here's the patch of the current state.  With it, M-:
debug-on-error RET shows nil, when that is the case.  M-: (foo) enters
the debugger when an error gets signalled, assuming
eval-expression-debug-on-error is t (or a suitable list), but doesn't
enter the debugger when e-e-d-o-error is nil.

There's a slight disadvantage to the approach, namely the introduction of
a new internal variable debug-from--eval-expression which is tested from
signal_or_quit.  On the other hand, eval-expression itself has been
noticeably simplified.



diff --git a/lisp/simple.el b/lisp/simple.el
index 02c68912dba..e8a9a795c0b 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1981,11 +1981,17 @@ eval-expression-print-length
   :version "21.1")
 
 (defcustom eval-expression-debug-on-error t
-  "If non-nil set `debug-on-error' to t in `eval-expression'.
-If nil, don't change the value of `debug-on-error'."
+  "Non-nil means enter debugger on error on a call from `eval-expression'.
+Does not apply to errors handled by `condition-case' or those
+matched by `debug-ignored-errors'.
+Like `debug-on-error', this variable's value can also be a list,
+with the same meaning as for `debug-on-error'.
+
+A nil value for this variable will not prevent an entry to
+the debugger caused by other variables such as `debug-on-error'."
   :group 'lisp
   :type 'boolean
-  :version "21.1")
+  :version "30.1")
 
 (defcustom eval-expression-print-maximum-character 127
   "The largest integer that will be displayed as a character.
@@ -2120,34 +2126,19 @@ eval-expression
    (cons (read--expression "Eval: ")
          (eval-expression-get-print-arguments current-prefix-arg)))
 
-  (let (result)
-    (if (null eval-expression-debug-on-error)
-        (setq result
-              (values--store-value
-               (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-      (let ((old-value (make-symbol "t")) new-value)
-        ;; Bind debug-on-error to something unique so that we can
-        ;; detect when evalled code changes it.
-        (let ((debug-on-error old-value))
-          (setq result
-	        (values--store-value
-                 (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-	  (setq new-value debug-on-error))
-        ;; If evalled code has changed the value of debug-on-error,
-        ;; propagate that change to the global binding.
-        (unless (eq old-value new-value)
-	  (setq debug-on-error new-value))))
-
-    (let ((print-length (unless no-truncate eval-expression-print-length))
-          (print-level  (unless no-truncate eval-expression-print-level))
-          (eval-expression-print-maximum-character char-print-limit)
-          (deactivate-mark))
-      (let ((out (if insert-value (current-buffer) t)))
-        (prog1
-            (prin1 result out)
-          (let ((str (and char-print-limit
-                          (eval-expression-print-format result))))
-            (when str (princ str out))))))))
+  (let* ((debug-from--eval-expression eval-expression-debug-on-error)
+         (result (values--store-value
+                  (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
+         (print-length (unless no-truncate eval-expression-print-length))
+         (print-level  (unless no-truncate eval-expression-print-level))
+         (eval-expression-print-maximum-character char-print-limit)
+         (deactivate-mark)
+         (out (if insert-value (current-buffer) t)))
+    (prog1
+        (prin1 result out)
+      (let ((str (and char-print-limit
+                      (eval-expression-print-format result))))
+        (when str (princ str out))))))
 
 (defun edit-and-eval-command (prompt command)
   "Prompting with PROMPT, let user edit COMMAND and eval result.
diff --git a/src/eval.c b/src/eval.c
index 12e811ce264..6cadda01efb 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2033,7 +2033,8 @@ maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig, Lisp_Object data)
       /* Does user want to enter debugger for this kind of error?  */
       && (signal_quit_p (sig)
 	  ? debug_on_quit
-	  : wants_debugger (Vdebug_on_error, conditions))
+	  : (wants_debugger (Vdebug_from__eval_expression, conditions)
+	     || wants_debugger (Vdebug_on_error, conditions)))
       && ! skip_debugger (conditions, combined_data)
       /* See commentary on definition of
          `internal-when-entered-debugger'.  */
@@ -4299,6 +4300,13 @@ syms_of_eval (void)
 See also the variable `debug-on-quit' and `inhibit-debugger'.  */);
   Vdebug_on_error = Qnil;
 
+  DEFVAR_LISP ("debug-from--eval-expression", Vdebug_from__eval_expression,
+	       doc: /* Non-nil means enter debugger if an error is signaled.
+This only applies in forms called by `eval-expression'.  This variable
+has the same semantics as `debug-on-error'.  It is an internal variable
+only.  */);
+  Vdebug_from__eval_expression = Qnil;
+
   DEFVAR_LISP ("debug-ignored-errors", Vdebug_ignored_errors,
     doc: /* List of errors for which the debugger should not be called.
 Each element may be a condition-name or a regexp that matches error messages.


-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 16:25:11 -0500

> +  (let* ((debug-from--eval-expression eval-expression-debug-on-error)
> +         (result (values--store-value
> +                  (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
> +         (print-length (unless no-truncate eval-expression-print-length))
> +         (print-level  (unless no-truncate eval-expression-print-level))
> +         (eval-expression-print-maximum-character char-print-limit)
> +         (deactivate-mark)
> +         (out (if insert-value (current-buffer) t)))
> +    (prog1
> +        (prin1 result out)
> +      (let ((str (and char-print-limit
> +                      (eval-expression-print-format result))))
> +        (when str (princ str out))))))

So you kicked the can a bit further down the road.
The next bug report will be that `M-: debug-from--eval-expression` does
not return the expected value.

FWIW, the other way I came up to circumvent the problem is to test the
shape of the expression to evaluate and only use
`eval-expression-debug-on-error` when the expression is not a mere
symbol (for which the backtrace would presumably not be interesting anyway).
But then we get the weird situation where `M-x debug-on-error` can
return nil but `M-x (list debug-on-error ...)` returns a list that
starts with t.

I think I'd rather keep the current code, whose semantics is
actually simpler.


        Stefan

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

From: Alan Mackenzie <acm <at> muc.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Fri, 24 Nov 2023 22:21:27 +0000

Hello, Stefan.

On Fri, Nov 24, 2023 at 16:25:11 -0500, Stefan Monnier wrote:
> > +  (let* ((debug-from--eval-expression eval-expression-debug-on-error)
> > +         (result (values--store-value
> > +                  (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
> > +         (print-length (unless no-truncate eval-expression-print-length))
> > +         (print-level  (unless no-truncate eval-expression-print-level))
> > +         (eval-expression-print-maximum-character char-print-limit)
> > +         (deactivate-mark)
> > +         (out (if insert-value (current-buffer) t)))
> > +    (prog1
> > +        (prin1 result out)
> > +      (let ((str (and char-print-limit
> > +                      (eval-expression-print-format result))))
> > +        (when str (princ str out))))))

> So you kicked the can a bit further down the road.
> The next bug report will be that `M-: debug-from--eval-expression` does
> not return the expected value.

Not at all.  debug-from--eval-expression is a purely internal variable,
unlike debug-on-error which is intended for user use.  It is bound in
exactly one place, and tested in exactly one place.

Anybody reporting such a "bug" would legitimately get the reply "not a
bug".

> FWIW, the other way I came up to circumvent the problem is to test the
> shape of the expression to evaluate and only use
> `eval-expression-debug-on-error` when the expression is not a mere
> symbol (for which the backtrace would presumably not be interesting anyway).
> But then we get the weird situation where `M-x debug-on-error` can
> return nil but `M-x (list debug-on-error ...)` returns a list that
> starts with t.

[ By M-x, I assume you're meaning M-:.]

Yes.  Such a patch wouldn't fix the bug.  The root of the bug is trying
to make debug-on-error do two different contradictory jobs, as we've
already discussed.  To fix this, you've either got to decide not to do
one of these jobs at all, or introduce a new variable.  My patch does
the second of these.

> I think I'd rather keep the current code, whose semantics is
> actually simpler.

No.  My patch fixes the bug, by introducing a separate variable to do
one of the two jobs that debug-on-error is currently trying to do.  This
is a simplification.  As a side effect, eval-expression has been reduced
from 59 to 44 lines; not counting the doc string this is a reduction of
around 50%.

For what it's worth, I lost about 10 hours of time trying to debug
a situation where I wasn't getting a backtrace, despite debug-on-error
being t.  The problem was that d-o-e wasn't t at all, it was nil.  M-:
had been lying.  

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 09:30:29 +0200

> Date: Fri, 24 Nov 2023 20:54:45 +0000
> Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > >     Non-nil means enter debugger on an error in a call from `eval-expression'.
> > >     Does not apply to errors handled by `condition-case' or those
> > >     matched by `debug-ignored-errors'.
> > >     A nil value for this variable will not prevent an entry to
> > >     the debugger caused by other variables such as `debug-on-error'.
> 
> > First, the last two sentences above should be transposed, as the
> > second one is not related to the 1st one, but the 3rd one is.
> 
> Done.
> 
> > And second, please try to reword so that the text is less complicated
> > and easier to understand.
> 
> I'm perhaps a bit too close to it.  Apart from the first line (for which
> too much information needs squashing in), I can't really see much scope
> for improvement.

Here:

   (defcustom eval-expression-debug-on-error t
    "If non-nil, enter debugger if `eval-expression' signals an error.
  A non-nil value means set `debug-on-error' to that value when calling
  `eval-expression'; this will enter a debugger if `eval-expression'
  signals an error.  A nil value means don't change the value of
  `debug-on-error' when calling `eval-expression'.
  The value can also be a list, with the same meaning as for `debug-on-error'.
  Like  `debug-on-error', this variable does not apply to errors handled
  by `condition-case' or those matched by `debug-ignored-errors'."

> +  DEFVAR_LISP ("debug-from--eval-expression", Vdebug_from__eval_expression,
> +	       doc: /* Non-nil means enter debugger if an error is signaled.
> +This only applies in forms called by `eval-expression'.  This variable
> +has the same semantics as `debug-on-error'.  It is an internal variable
> +only.  */);

The doc string should be just

  /* Internal use only, used by `eval-expression'.  */

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 09:59:45 +0200

> Date: Fri, 24 Nov 2023 22:21:27 +0000
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > So you kicked the can a bit further down the road.
> > The next bug report will be that `M-: debug-from--eval-expression` does
> > not return the expected value.
> 
> Not at all.  debug-from--eval-expression is a purely internal variable,
> unlike debug-on-error which is intended for user use.  It is bound in
> exactly one place, and tested in exactly one place.
> 
> Anybody reporting such a "bug" would legitimately get the reply "not a
> bug".

How about not exposing the internal variable to Lisp at all?

> For what it's worth, I lost about 10 hours of time trying to debug
> a situation where I wasn't getting a backtrace, despite debug-on-error
> being t.  The problem was that d-o-e wasn't t at all, it was nil.  M-:
> had been lying.  

You never described that situation, AFAICT.  I think you should, so
that we could assess how grave the problem is, which is an important
part of deciding whether the solution you propose is useful.  I don't
understand how could you NOT get a backtrace when debug-on-error is
non-nil.

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 10:32:23 +0000

Hello, Eli.

On Sat, Nov 25, 2023 at 09:59:45 +0200, Eli Zaretskii wrote:
> > Date: Fri, 24 Nov 2023 22:21:27 +0000
> > Cc: Eli Zaretskii <eliz <at> gnu.org>, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> > From: Alan Mackenzie <acm <at> muc.de>

> > > So you kicked the can a bit further down the road.
> > > The next bug report will be that `M-: debug-from--eval-expression` does
> > > not return the expected value.

> > Not at all.  debug-from--eval-expression is a purely internal variable,
> > unlike debug-on-error which is intended for user use.  It is bound in
> > exactly one place, and tested in exactly one place.

> > Anybody reporting such a "bug" would legitimately get the reply "not a
> > bug".

> How about not exposing the internal variable to Lisp at all?

That's a very good idea.  It would need little more than a new C function
which would bind that variable then call eval.  Maybe move
eval-expression-debug-on-error into eval.c, too.

> > For what it's worth, I lost about 10 hours of time trying to debug
> > a situation where I wasn't getting a backtrace, despite debug-on-error
> > being t.  The problem was that d-o-e wasn't t at all, it was nil.  M-:
> > had been lying.  

> You never described that situation, AFAICT.  I think you should, so
> that we could assess how grave the problem is, which is an important
> part of deciding whether the solution you propose is useful.  I don't
> understand how could you NOT get a backtrace when debug-on-error is
> non-nil.

Sorry, I wasn't clear enough.  During those 10 hours, I was under the
impression that debug-on-error was t, because M-: debug-on-error said so.
It actually was nil.  That's why I submitted this bug report.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 13:15:15 +0200

> Date: Sat, 25 Nov 2023 10:32:23 +0000
> Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > How about not exposing the internal variable to Lisp at all?
> 
> That's a very good idea.  It would need little more than a new C function
> which would bind that variable then call eval.  Maybe move
> eval-expression-debug-on-error into eval.c, too.

What I had in mind was a function exposed to Lisp that would set an
internal variable not exposed to Lisp.

> > > For what it's worth, I lost about 10 hours of time trying to debug
> > > a situation where I wasn't getting a backtrace, despite debug-on-error
> > > being t.  The problem was that d-o-e wasn't t at all, it was nil.  M-:
> > > had been lying.  
> 
> > You never described that situation, AFAICT.  I think you should, so
> > that we could assess how grave the problem is, which is an important
> > part of deciding whether the solution you propose is useful.  I don't
> > understand how could you NOT get a backtrace when debug-on-error is
> > non-nil.
> 
> Sorry, I wasn't clear enough.  During those 10 hours, I was under the
> impression that debug-on-error was t, because M-: debug-on-error said so.
> It actually was nil.  That's why I submitted this bug report.

So maybe instead of changing how this stuff works we should improve
how debug-on-error's value is reported by M-: and other eval commands?

Note that (AFAIU) your change doesn't just solve the problem you
bumped into, it also changes the value of debug-on-error inside
eval-expression etc., when eval-expression-debug-on-error's value is
non-nil, but not t.  I wonder what is the reason for that?

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 12:40:03 +0000

Hello, Eli.

On Sat, Nov 25, 2023 at 13:15:15 +0200, Eli Zaretskii wrote:
> > Date: Sat, 25 Nov 2023 10:32:23 +0000
> > Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org
> > From: Alan Mackenzie <acm <at> muc.de>

> > > How about not exposing the internal variable to Lisp at all?

> > That's a very good idea.  It would need little more than a new C function
> > which would bind that variable then call eval.  Maybe move
> > eval-expression-debug-on-error into eval.c, too.

> What I had in mind was a function exposed to Lisp that would set an
> internal variable not exposed to Lisp.

The would still require an unwind-protect somewhere.  I've implemented
sub-eval-expression in eval.c.  debug-from--eval-expression is no longer
visible from Lisp.  Perhaps this is good enough.  

> > > > For what it's worth, I lost about 10 hours of time trying to debug
> > > > a situation where I wasn't getting a backtrace, despite debug-on-error
> > > > being t.  The problem was that d-o-e wasn't t at all, it was nil.  M-:
> > > > had been lying.  

> > > You never described that situation, AFAICT.  I think you should, so
> > > that we could assess how grave the problem is, which is an important
> > > part of deciding whether the solution you propose is useful.  I don't
> > > understand how could you NOT get a backtrace when debug-on-error is
> > > non-nil.

> > Sorry, I wasn't clear enough.  During those 10 hours, I was under the
> > impression that debug-on-error was t, because M-: debug-on-error said so.
> > It actually was nil.  That's why I submitted this bug report.

> So maybe instead of changing how this stuff works we should improve
> how debug-on-error's value is reported by M-: and other eval commands?

Yes, but that might be complicated, and won't help the user trying to
debug something which depends on debug-on-error, who is using M-: to try
to test it.  I still say the bug is trying to make debug-on-error do too
much, more than it's capable of.

> Note that (AFAIU) your change doesn't just solve the problem you
> bumped into, it also changes the value of debug-on-error inside
> eval-expression etc., when eval-expression-debug-on-error's value is
> non-nil, but not t.  I wonder what is the reason for that?

I don't see that in my current version of the patch (below).  To test
this, I used the following:

(defun foo ()
  (interactive)
  (message "debug-on-error is %s" debug-on-error)
  (message "eval-expression-debug-on-error is %s" eval-expression-debug-on-error)
  (car 'foo))

, and called it with various settings of debug-on-error and
eval-expression-debug-on-error.  In particular, with

  (setq eval-expression-debug-on-error '(wrong-type-argument))

, I still see debug-on-error reported as nil.



diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 6d83aaf4d14..9d176c6c599 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -262,6 +262,13 @@ minibuffer-prompt-properties--setter
 					     :value (nil)
 					     (symbol :format "%v"))
 				     (const :tag "always" t)))
+             (eval-expression-debug-on-error debug
+                                             (choice (const :tag "off")
+                                                     (repeat :menu-tag "When"
+                                                             :value (nil)
+                                                             (symbol :format "%v"))
+                                                     (const :tag "always" t))
+                                             "30.1")
 	     (debug-ignored-errors debug (repeat (choice symbol regexp)))
 	     (debug-on-quit debug boolean)
 	     (debug-on-signal debug boolean)
diff --git a/lisp/simple.el b/lisp/simple.el
index 02c68912dba..f4c9873ceed 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -1980,13 +1980,6 @@ eval-expression-print-length
   :type '(choice (const :tag "No Limit" nil) integer)
   :version "21.1")
 
-(defcustom eval-expression-debug-on-error t
-  "If non-nil set `debug-on-error' to t in `eval-expression'.
-If nil, don't change the value of `debug-on-error'."
-  :group 'lisp
-  :type 'boolean
-  :version "21.1")
-
 (defcustom eval-expression-print-maximum-character 127
   "The largest integer that will be displayed as a character.
 This affects printing by `eval-expression' (via
@@ -2120,34 +2113,18 @@ eval-expression
    (cons (read--expression "Eval: ")
          (eval-expression-get-print-arguments current-prefix-arg)))
 
-  (let (result)
-    (if (null eval-expression-debug-on-error)
-        (setq result
-              (values--store-value
-               (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-      (let ((old-value (make-symbol "t")) new-value)
-        ;; Bind debug-on-error to something unique so that we can
-        ;; detect when evalled code changes it.
-        (let ((debug-on-error old-value))
-          (setq result
-	        (values--store-value
-                 (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-	  (setq new-value debug-on-error))
-        ;; If evalled code has changed the value of debug-on-error,
-        ;; propagate that change to the global binding.
-        (unless (eq old-value new-value)
-	  (setq debug-on-error new-value))))
-
-    (let ((print-length (unless no-truncate eval-expression-print-length))
-          (print-level  (unless no-truncate eval-expression-print-level))
-          (eval-expression-print-maximum-character char-print-limit)
-          (deactivate-mark))
-      (let ((out (if insert-value (current-buffer) t)))
-        (prog1
-            (prin1 result out)
-          (let ((str (and char-print-limit
-                          (eval-expression-print-format result))))
-            (when str (princ str out))))))))
+  (let* ((result (values--store-value
+                  (sub-eval-expression (macroexpand-all exp))))
+         (print-length (unless no-truncate eval-expression-print-length))
+         (print-level  (unless no-truncate eval-expression-print-level))
+         (eval-expression-print-maximum-character char-print-limit)
+         (deactivate-mark)
+         (out (if insert-value (current-buffer) t)))
+    (prog1
+        (prin1 result out)
+      (let ((str (and char-print-limit
+                      (eval-expression-print-format result))))
+        (when str (princ str out))))))
 
 (defun edit-and-eval-command (prompt command)
   "Prompting with PROMPT, let user edit COMMAND and eval result.
diff --git a/src/eval.c b/src/eval.c
index 12e811ce264..eccabf3a091 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -2033,7 +2033,8 @@ maybe_call_debugger (Lisp_Object conditions, Lisp_Object sig, Lisp_Object data)
       /* Does user want to enter debugger for this kind of error?  */
       && (signal_quit_p (sig)
 	  ? debug_on_quit
-	  : wants_debugger (Vdebug_on_error, conditions))
+	  : (wants_debugger (Vdebug_from__eval_expression, conditions)
+	     || wants_debugger (Vdebug_on_error, conditions)))
       && ! skip_debugger (conditions, combined_data)
       /* See commentary on definition of
          `internal-when-entered-debugger'.  */
@@ -2383,6 +2384,19 @@ DEFUN ("eval", Feval, Seval, 1, 2, 0,
   return unbind_to (count, eval_sub (form));
 }
 
+DEFUN ("sub-eval-expression", Fsub_eval_expression, Ssub_eval_expression,
+       1, 1, 0,
+       doc: /* Evaluate FORM and return its value.
+This function should be called only from `eval-expression'.
+It evaluates with `lexical-binding' non-nil, and handles
+`eval-expression-debug-on-error'.  */)
+  (Lisp_Object form)
+{
+  specpdl_ref count = SPECPDL_INDEX ();
+  specbind (Qdebug_from__eval_expression, Veval_expression_debug_on_error);
+  return unbind_to (count, Feval (form, Qt));
+}
+
 void
 grow_specpdl_allocation (void)
 {
@@ -4299,6 +4313,29 @@ syms_of_eval (void)
 See also the variable `debug-on-quit' and `inhibit-debugger'.  */);
   Vdebug_on_error = Qnil;
 
+  DEFSYM (Qeval_expression_debug_on_error, "eval-expression-debug-on-error");
+  DEFVAR_LISP ("eval-expression-debug-on-error",
+	       Veval_expression_debug_on_error,
+	       doc: /* Non-nil means enter debugger on error on a call from `eval-expression'.
+Does not apply to errors handled by `condition-case' or those
+matched by `debug-ignored-errors'.
+Like `debug-on-error', this variable's value can also be a list,
+with the same meaning as for `debug-on-error'.
+
+A nil value for this variable will not prevent an entry to
+the debugger caused by other variables such as `debug-on-error'.  */);
+  Veval_expression_debug_on_error = Qt;
+
+  DEFSYM (Qdebug_from__eval_expression, "debug-from--eval-expression");
+  DEFVAR_LISP ("debug-from--eval-expression", Vdebug_from__eval_expression,
+	       doc: /* Non-nil means enter debugger if an error is signaled.
+This only applies in forms called by `eval-expression'.  This variable
+has the same semantics as `debug-on-error'.  It is an internal variable
+only.  */);
+  Vdebug_from__eval_expression = Qnil;
+  /* debug-from--eval-expression should not be visible from Lisp.  */
+  Funintern (Qdebug_from__eval_expression, Qnil);
+
   DEFVAR_LISP ("debug-ignored-errors", Vdebug_ignored_errors,
     doc: /* List of errors for which the debugger should not be called.
 Each element may be a condition-name or a regexp that matches error messages.
@@ -4455,6 +4492,7 @@ syms_of_eval (void)
   defsubr (&Sautoload);
   defsubr (&Sautoload_do_load);
   defsubr (&Seval);
+  defsubr (&Ssub_eval_expression);
   defsubr (&Sapply);
   defsubr (&Sfuncall);
   defsubr (&Sfunc_arity);


-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 15:04:08 +0200

> Date: Sat, 25 Nov 2023 12:40:03 +0000
> Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > What I had in mind was a function exposed to Lisp that would set an
> > internal variable not exposed to Lisp.
> 
> The would still require an unwind-protect somewhere.

We can unwind-protect in C as well.

> > > Sorry, I wasn't clear enough.  During those 10 hours, I was under the
> > > impression that debug-on-error was t, because M-: debug-on-error said so.
> > > It actually was nil.  That's why I submitted this bug report.
> 
> > So maybe instead of changing how this stuff works we should improve
> > how debug-on-error's value is reported by M-: and other eval commands?
> 
> Yes, but that might be complicated, and won't help the user trying to
> debug something which depends on debug-on-error, who is using M-: to try
> to test it.

I don't see how that could happen.

> > Note that (AFAIU) your change doesn't just solve the problem you
> > bumped into, it also changes the value of debug-on-error inside
> > eval-expression etc., when eval-expression-debug-on-error's value is
> > non-nil, but not t.  I wonder what is the reason for that?
> 
> I don't see that in my current version of the patch (below).  To test
> this, I used the following:
> 
> (defun foo ()
>   (interactive)
>   (message "debug-on-error is %s" debug-on-error)
>   (message "eval-expression-debug-on-error is %s" eval-expression-debug-on-error)
>   (car 'foo))
> 
> , and called it with various settings of debug-on-error and
> eval-expression-debug-on-error.  In particular, with
> 
>   (setq eval-expression-debug-on-error '(wrong-type-argument))
> 
> , I still see debug-on-error reported as nil.

No, I meant with the current code debug-on-error is set to t whenever
eval-expression-debug-on-error is non-nil, and your change sets it to
the same value as eval-expression-debug-on-error instead.

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 14:14:59 +0000

Hello, Eli.

On Sat, Nov 25, 2023 at 15:04:08 +0200, Eli Zaretskii wrote:
> > Date: Sat, 25 Nov 2023 12:40:03 +0000
> > Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> > From: Alan Mackenzie <acm <at> muc.de>

[ .... ]

> > > Note that (AFAIU) your change doesn't just solve the problem you
> > > bumped into, it also changes the value of debug-on-error inside
> > > eval-expression etc., when eval-expression-debug-on-error's value is
> > > non-nil, but not t.  I wonder what is the reason for that?

> > I don't see that in my current version of the patch (below).  To test
> > this, I used the following:

> > (defun foo ()
> >   (interactive)
> >   (message "debug-on-error is %s" debug-on-error)
> >   (message "eval-expression-debug-on-error is %s" eval-expression-debug-on-error)
> >   (car 'foo))

> > , and called it with various settings of debug-on-error and
> > eval-expression-debug-on-error.  In particular, with

> >   (setq eval-expression-debug-on-error '(wrong-type-argument))

> > , I still see debug-on-error reported as nil.

> No, I meant with the current code debug-on-error is set to t whenever
> eval-expression-debug-on-error is non-nil, ....

Yes.

> .... and your change sets it to the same value as
> eval-expression-debug-on-error instead.

That is not what is meant to happen.  Instead, the new internal variable
debug-from--eval-expression gets set to e-e-debug-on-error.
debug-on-error should remain unchanged throughout a M-: call, leaving it
free for use by user code.  The subroutines of signal_or_quit test
debug-from--eval-expression in addition to testing debug-on-error.

Are you sure that isn't what you're seeing?

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 09:23:42 -0500

> That's a very good idea.  It would need little more than a new C function
> which would bind that variable then call eval.  Maybe move
> eval-expression-debug-on-error into eval.c, too.

Side note: there's recently been several things pointing to the need to
add something like Common Lisp's `handler-bind`.

`handler-bind` would provide a superset of the features that your
new C function would provide.  E.g. we could do something like:

    (defmacro with-debug-on-error (&rest body)
      `(handler-bind ((error #'debugger))
         ,@body))

which would mean that any error signal not caught within BODY itself
would call the debugger (and contrary to `condition-case`, that call is
performed before unwinding the stack, so we'd get the behavior we
expect from the debugger).

> Sorry, I wasn't clear enough.  During those 10 hours, I was under the
> impression that debug-on-error was t, because M-: debug-on-error said so.
> It actually was nil.  That's why I submitted this bug report.

I feel for you.  Personally I had a similar experience at some point but
instead of looking at the value of the var, I actually signaled
an error.

Seeing how I was getting into the debugger, I assumed that
`debug-on-error` was properly set (I had set it manually, so I (thought
I) knew that it was set, which is why I didn't even look at the var, but
apparently that was within a recursive edit that had it let-bound or
something, which is why it was actually unset).

The change you propose wouldn't have helped me directly in that case,
tho I think it probably would have circumvented the problem because the
var presumably wouldn't have been let-bound in the first place :-)

It took me less than 10 hours to figure out one of my many unjustified
assumptions, luckily.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 17:50:09 +0200

> Date: Sat, 25 Nov 2023 14:14:59 +0000
> Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > No, I meant with the current code debug-on-error is set to t whenever
> > eval-expression-debug-on-error is non-nil, ....
> 
> Yes.
> 
> > .... and your change sets it to the same value as
> > eval-expression-debug-on-error instead.
> 
> That is not what is meant to happen.

We are miscommunicating.

> Instead, the new internal variable
> debug-from--eval-expression gets set to e-e-debug-on-error.

Exactly.  And that value can be non-nil, but also not t.  That is the
change I see, and your doc string suggests that it is intentional.
Thus, we have a behavior change wrt to the value that the body of
eval-expression will see.  And non-nil, non-t values have special
meaning there.

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 16:40:40 +0000

Hello, Eli.

On Sat, Nov 25, 2023 at 17:50:09 +0200, Eli Zaretskii wrote:
> > Date: Sat, 25 Nov 2023 14:14:59 +0000
> > Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> > From: Alan Mackenzie <acm <at> muc.de>

> > > No, I meant with the current code debug-on-error is set to t whenever
> > > eval-expression-debug-on-error is non-nil, ....

> > Yes.

> > > .... and your change sets it to the same value as
> > > eval-expression-debug-on-error instead.

> > That is not what is meant to happen.

> We are miscommunicating.

> > Instead, the new internal variable
> > debug-from--eval-expression gets set to e-e-debug-on-error.

> Exactly.  And that value can be non-nil, but also not t.  That is the
> change I see, and your doc string suggests that it is intentional.
> Thus, we have a behavior change wrt to the value that the body of
> eval-expression will see.  And non-nil, non-t values have special
> meaning there.

OK, I'm with you, now.  The non-nil, non-t value in the internal variable
debug-from--e-e won't be seen by any Lisp code.  It can be seen by Lisp
code in eval-expression-debug-on-error.

This is an extension of e-e-debug-on-error to match what debug-on-error
does.  I wouldn't think that d-o-e gets set to a list of conditions very
often, but when it does, it's likely very useful.  It seemed logical to
extend the same facility to e-e-debug-on-error.  Do you think this is a
bad idea?  [Clearly, some entries in NEWS will be needed, as well as
amendments to the Elisp manual.]

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 18:46:05 +0200

> Date: Sat, 25 Nov 2023 16:40:40 +0000
> Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> From: Alan Mackenzie <acm <at> muc.de>
> 
> > > Instead, the new internal variable
> > > debug-from--eval-expression gets set to e-e-debug-on-error.
> 
> > Exactly.  And that value can be non-nil, but also not t.  That is the
> > change I see, and your doc string suggests that it is intentional.
> > Thus, we have a behavior change wrt to the value that the body of
> > eval-expression will see.  And non-nil, non-t values have special
> > meaning there.
> 
> OK, I'm with you, now.  The non-nil, non-t value in the internal variable
> debug-from--e-e won't be seen by any Lisp code.  It can be seen by Lisp
> code in eval-expression-debug-on-error.
> 
> This is an extension of e-e-debug-on-error to match what debug-on-error
> does.  I wouldn't think that d-o-e gets set to a list of conditions very
> often, but when it does, it's likely very useful.  It seemed logical to
> extend the same facility to e-e-debug-on-error.  Do you think this is a
> bad idea?  [Clearly, some entries in NEWS will be needed, as well as
> amendments to the Elisp manual.]

I don't know yet if it is a good idea, I just wanted to point out that
your change is not just a bugfix: it actually changes the behavior of
eval-expression.

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 16:57:21 +0000

Hello, Eli.

On Sat, Nov 25, 2023 at 18:46:05 +0200, Eli Zaretskii wrote:
> > Date: Sat, 25 Nov 2023 16:40:40 +0000
> > Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> > From: Alan Mackenzie <acm <at> muc.de>

> > > > Instead, the new internal variable
> > > > debug-from--eval-expression gets set to e-e-debug-on-error.

> > > Exactly.  And that value can be non-nil, but also not t.  That is the
> > > change I see, and your doc string suggests that it is intentional.
> > > Thus, we have a behavior change wrt to the value that the body of
> > > eval-expression will see.  And non-nil, non-t values have special
> > > meaning there.

> > OK, I'm with you, now.  The non-nil, non-t value in the internal variable
> > debug-from--e-e won't be seen by any Lisp code.  It can be seen by Lisp
> > code in eval-expression-debug-on-error.

> > This is an extension of e-e-debug-on-error to match what debug-on-error
> > does.  I wouldn't think that d-o-e gets set to a list of conditions very
> > often, but when it does, it's likely very useful.  It seemed logical to
> > extend the same facility to e-e-debug-on-error.  Do you think this is a
> > bad idea?  [Clearly, some entries in NEWS will be needed, as well as
> > amendments to the Elisp manual.]

> I don't know yet if it is a good idea, I just wanted to point out that
> your change is not just a bugfix: it actually changes the behavior of
> eval-expression.

OK.  It's a change that would be trivially easy to reverse.

My guess is that debug-on-error was originally a DEFVAR_BOOL, and at some
stage it was extended to handle a list of conditions.  But for some
reason eval-expression-debug-on-error was not extended in the same way.
Perhaps this was an oversight.  If so, now would be a good time to fix
this.

Maybe there is some evidence of what happened in the repository.  I'll
have a look.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Alan Mackenzie <acm <at> muc.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 17:36:37 +0000

Hello again, Eli.

On Sat, Nov 25, 2023 at 16:57:21 +0000, Alan Mackenzie wrote:
> On Sat, Nov 25, 2023 at 18:46:05 +0200, Eli Zaretskii wrote:
> > > Date: Sat, 25 Nov 2023 16:40:40 +0000
> > > Cc: monnier <at> iro.umontreal.ca, 67196 <at> debbugs.gnu.org, acm <at> muc.de
> > > From: Alan Mackenzie <acm <at> muc.de>

> > > > > Instead, the new internal variable
> > > > > debug-from--eval-expression gets set to e-e-debug-on-error.

> > > > Exactly.  And that value can be non-nil, but also not t.  That is the
> > > > change I see, and your doc string suggests that it is intentional.
> > > > Thus, we have a behavior change wrt to the value that the body of
> > > > eval-expression will see.  And non-nil, non-t values have special
> > > > meaning there.

> > > OK, I'm with you, now.  The non-nil, non-t value in the internal variable
> > > debug-from--e-e won't be seen by any Lisp code.  It can be seen by Lisp
> > > code in eval-expression-debug-on-error.

> > > This is an extension of e-e-debug-on-error to match what debug-on-error
> > > does.  I wouldn't think that d-o-e gets set to a list of conditions very
> > > often, but when it does, it's likely very useful.  It seemed logical to
> > > extend the same facility to e-e-debug-on-error.  Do you think this is a
> > > bad idea?  [Clearly, some entries in NEWS will be needed, as well as
> > > amendments to the Elisp manual.]

> > I don't know yet if it is a good idea, I just wanted to point out that
> > your change is not just a bugfix: it actually changes the behavior of
> > eval-expression.

> OK.  It's a change that would be trivially easy to reverse.

> My guess is that debug-on-error was originally a DEFVAR_BOOL, and at some
> stage it was extended to handle a list of conditions.  But for some
> reason eval-expression-debug-on-error was not extended in the same way.
> Perhaps this was an oversight.  If so, now would be a good time to fix
> this.

> Maybe there is some evidence of what happened in the repository.  I'll
> have a look.

eval-expression-debug-on-error came into existence with this commit:

commit b49df39ddcfc578234530208eba8e288f604db1b
Author: Richard M. Stallman <rms <at> gnu.org>
Date:   Tue Sep 14 07:00:04 1999 +0000

    (eval-expression-print-level): New variable.
    (eval-expression-print-length): New variable.
    (eval-expression-debug-on-error): New variable.
    (eval-expression): Bind print-level, print-length and
    debug-on-error from those vars.

..  debug-on-error got its list value with this commit:

commit 128c0f667926dd4296411b9189125ee8c5b78b79
Author: Roland McGrath <roland <at> gnu.org>
Date:   Thu Jun 4 04:33:43 1992 +0000

    *** empty log message ***

..  So debug-on-error had its list of conditions value 7 years before
eval-expression-debug-on-error existed.  It's not clear why this new
variable didn't take over the enhanced semantics from debug-on-error.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 25 Nov 2023 19:12:58 +0100

On Nov 25 2023, Alan Mackenzie wrote:

> My guess is that debug-on-error was originally a DEFVAR_BOOL, and at some
> stage it was extended to handle a list of conditions.  But for some
> reason eval-expression-debug-on-error was not extended in the same way.

There is no need to extend eval-expression-debug-on-error.  Just set it
to nil to let eval-expression use the value of debug-on-error as usual.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Alan Mackenzie <acm <at> muc.de>
Cc: 67196 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sat, 16 Dec 2023 23:23:31 -0500

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

> Side note: there's recently been several things pointing to the need to
> add something like Common Lisp's `handler-bind`.

BTW, I now have working code implementing `handler-bind` (see bug#67862
for a preview of the code).  With it I can fix this bug with the
patch below (well, plus a tweak in `debug.el` plus some docstring
adjustements).


        Stefan

[eval-expression.patch (text/x-diff, inline)]

diff --git a/lisp/simple.el b/lisp/simple.el
index cee1ddac52f..da07a249081 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2120,23 +2120,17 @@ eval-expression
    (cons (read--expression "Eval: ")
          (eval-expression-get-print-arguments current-prefix-arg)))
 
-  (let (result)
+  (let* (result
+         (runfun
+          (lambda ()
+            (setq result
+                  (values--store-value
+                   (eval (let ((lexical-binding t)) (macroexpand-all exp))
+                         t))))))
     (if (null eval-expression-debug-on-error)
-        (setq result
-              (values--store-value
-               (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-      (let ((old-value (make-symbol "t")) new-value)
-        ;; Bind debug-on-error to something unique so that we can
-        ;; detect when evalled code changes it.
-        (let ((debug-on-error old-value))
-          (setq result
-	        (values--store-value
-                 (eval (let ((lexical-binding t)) (macroexpand-all exp)) t)))
-	  (setq new-value debug-on-error))
-        ;; If evalled code has changed the value of debug-on-error,
-        ;; propagate that change to the global binding.
-        (unless (eq old-value new-value)
-	  (setq debug-on-error new-value))))
+        (funcall runfun)
+      (handler-bind ((error debugger))
+        (funcall runfun)))
 
     (let ((print-length (unless no-truncate eval-expression-print-length))
           (print-level  (unless no-truncate eval-expression-print-level))

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

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, eliz <at> gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Mon, 18 Dec 2023 22:54:39 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > Side note: there's recently been several things pointing to the need to
  > > add something like Common Lisp's `handler-bind`.

Could someone explain the `handler-bind' construct and show its
documentation?  It might be a good idea, but it could be that we can
do better,

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Stallman <rms <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, eliz <at> gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Tue, 19 Dec 2023 00:05:02 -0500

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

>   > > Side note: there's recently been several things pointing to the need to
>   > > add something like Common Lisp's `handler-bind`.
>
> Could someone explain the `handler-bind' construct and show its
> documentation?  It might be a good idea, but it could be that we can
> do better,

Here's my current handler-bind patch, which includes some doc.


        Stefan

[handler-bind.patch (text/x-diff, inline)]

commit 7252f6b48944e22980f4030c98a6c7b05243e19e
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date:   Tue Dec 19 00:03:25 2023 -0500

    tramp.el: Use `handler-bind` instead of `signal-hook-function`
    
    * lisp/net/tramp.el (tramp--trace-errors): Rename from
    `tramp-signal-hook-function` and change calling convention for
    `handler-bind`.
    (tramp-unknown-id-string, tramp-unknown-id-integer):
    Don't use `defconst` for vars which we let-bind (in tramp-tests.el).
    (tramp-file-name-handler): Use `handler-bind` instead of let-binding
    `signal-hook-function`.
    (tramp-handle-lock-file, tramp-handle-load, tramp-add-hops)
    (tramp-run-real-handler):
    * lisp/net/tramp-sshfs.el (tramp-sshfs-handle-insert-file-contents):
    * lisp/net/tramp-sh.el (tramp-sh-get-signal-strings):
    (tramp-send-command-and-read):
    * lisp/net/tramp-message.el (tramp-debug-message, tramp-error):
    * lisp/net/tramp-adb.el (tramp-do-parse-file-attributes-with-ls): Don't
    let-bind `signal-hook-function`.

diff --git a/lisp/net/tramp-adb.el b/lisp/net/tramp-adb.el
index e4d3ba8c74b..0b68e7384b2 100644
--- a/lisp/net/tramp-adb.el
+++ b/lisp/net/tramp-adb.el
@@ -297,9 +297,7 @@ tramp-do-parse-file-attributes-with-ls
 		 (cons uid tramp-unknown-id-integer)
 		 (cons gid tramp-unknown-id-integer)
 		 tramp-time-dont-know   ; atime
-		 ;; `date-to-time' checks `iso8601-parse', which might fail.
-		 (let (signal-hook-function)
-		   (date-to-time date))	; mtime
+		 (date-to-time date)	; mtime
 		 tramp-time-dont-know   ; ctime
 		 size
 		 mod-string
diff --git a/lisp/net/tramp-message.el b/lisp/net/tramp-message.el
index e05357f1f4f..924f4d15ad2 100644
--- a/lisp/net/tramp-message.el
+++ b/lisp/net/tramp-message.el
@@ -207,8 +207,7 @@ tramp-debug-message
 ARGUMENTS to actually emit the message (if applicable)."
   (declare (tramp-suppress-trace t))
   (let ((inhibit-message t)
-	create-lockfiles file-name-handler-alist message-log-max
-	signal-hook-function)
+	create-lockfiles file-name-handler-alist message-log-max)
     (with-current-buffer (tramp-get-debug-buffer vec)
       (goto-char (point-max))
       (let ((point (point)))
@@ -364,7 +363,7 @@ tramp-error
 signal identifier to be raised, remaining arguments passed to
 `tramp-message'.  Finally, signal SIGNAL is raised with
 FMT-STRING and ARGUMENTS."
-  (let (signal-hook-function)
+  (let ()
     (tramp-backtrace vec-or-proc)
     (unless arguments
       ;; FMT-STRING could be just a file name, as in
diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
index a7ead1f2997..77ab5b0a399 100644
--- a/lisp/net/tramp-sh.el
+++ b/lisp/net/tramp-sh.el
@@ -3168,12 +3168,11 @@ tramp-sh-get-signal-strings
 	(setcdr signals (cddr signals)))
       ;; Sanity check.  "kill -l" shall have returned just the signal
       ;; names.  Some shells don't, like the one in "docker alpine".
-      (let (signal-hook-function)
-	(condition-case nil
-	    (dolist (sig (cdr signals))
-	      (unless (string-match-p (rx bol (+ (any "+-" alnum)) eol) sig)
-		(error nil)))
-	  (error (setq signals '(0)))))
+      (condition-case nil
+	  (dolist (sig (cdr signals))
+	    (unless (string-match-p (rx bol (+ (any "+-" alnum)) eol) sig)
+	      (error nil)))
+	(error (setq signals '(0))))
       (dotimes (i 128)
 	(push
 	 (cond
@@ -5536,9 +5535,7 @@ tramp-send-command-and-read
       ;; Read the expression.
       (condition-case nil
 	  (prog1
-	      (let ((signal-hook-function
-		     (unless noerror signal-hook-function)))
-		(read (current-buffer)))
+	      (read (current-buffer))
 	    ;; Error handling.
 	    (when (search-forward-regexp (rx (not space)) (line-end-position) t)
 	      (error nil)))
diff --git a/lisp/net/tramp-sshfs.el b/lisp/net/tramp-sshfs.el
index 102ba637f55..afc2033ee6e 100644
--- a/lisp/net/tramp-sshfs.el
+++ b/lisp/net/tramp-sshfs.el
@@ -242,7 +242,7 @@ tramp-sshfs-handle-insert-file-contents
   (filename &optional visit beg end replace)
   "Like `insert-file-contents' for Tramp files."
   (setq filename (expand-file-name filename))
-  (let (signal-hook-function result)
+  (let (result)
     (unwind-protect
         (setq result
 	      (insert-file-contents
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 88cbfa2d88c..ebdc8bf97ef 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -1085,10 +1085,10 @@ tramp-postfix-host-regexp
 (defconst tramp-localname-regexp (rx (* (not (any "\r\n"))) eos)
   "Regexp matching localnames.")
 
-(defconst tramp-unknown-id-string "UNKNOWN"
+(defvar tramp-unknown-id-string "UNKNOWN"
   "String used to denote an unknown user or group.")
 
-(defconst tramp-unknown-id-integer -1
+(defvar tramp-unknown-id-integer -1
   "Integer used to denote an unknown user or group.")
 
 ;;;###tramp-autoload
@@ -1994,16 +1994,16 @@ tramp-barf-if-file-missing
 
 ;; This function provides traces in case of errors not triggered by
 ;; Tramp functions.
-(defun tramp-signal-hook-function (error-symbol data)
-  "Function to be called via `signal-hook-function'."
+(defun tramp--trace-errors (err)
+  "Function to be called via `handler-bind'."
   ;; `custom-initialize-*' functions provoke `void-variable' errors.
   ;; We don't want to see them in the backtrace.
   (declare (tramp-suppress-trace t))
-  (unless (eq error-symbol 'void-variable)
+  (unless (eq (car err) 'void-variable)
     (let ((inhibit-message t))
       (tramp-error
-       (car tramp-current-connection) error-symbol
-       (mapconcat (lambda (x) (format "%s" x)) data " ")))))
+       (car tramp-current-connection) (car err)
+       (mapconcat (lambda (x) (format "%s" x)) (cdr err) " ")))))
 
 (defmacro with-parsed-tramp-file-name (filename var &rest body)
   "Parse a Tramp filename and make components available in the body.
@@ -2081,7 +2081,7 @@ with-tramp-progress-reporter
 (defmacro with-tramp-timeout (list &rest body)
   "Like `with-timeout', but allow SECONDS to be nil.
 
-(fn (SECONDS TIMEOUT-FORMS...) BODY)"
+\(fn (SECONDS TIMEOUT-FORMS...) BODY)"
   (declare (indent 1) (debug ((form body) body)))
   (let ((seconds (car list))
 	(timeout-forms (cdr list)))
@@ -2248,8 +2248,7 @@ tramp-run-real-handler
 	    ,(and (eq inhibit-file-name-operation operation)
 		  inhibit-file-name-handlers)))
 	 (inhibit-file-name-operation operation)
-	 (args (if (tramp-file-name-p (car args)) (cons nil (cdr args)) args))
-	 signal-hook-function)
+	 (args (if (tramp-file-name-p (car args)) (cons nil (cdr args)) args)))
     (apply operation args)))
 
 ;; We handle here all file primitives.  Most of them have the file
@@ -2386,8 +2385,8 @@ tramp-file-name-handler
             (let ((current-connection tramp-current-connection)
 		  (foreign
 		   (tramp-find-foreign-file-name-handler v operation))
-		  (signal-hook-function #'tramp-signal-hook-function)
 		  result)
+	      (handler-bind ((error #'tramp--trace-errors))
 	      ;; Set `tramp-current-connection'.
 	      (unless
 		  (tramp-file-name-equal-p v (car tramp-current-connection))
@@ -2453,7 +2452,7 @@ tramp-file-name-handler
 		(unless
 		    (tramp-file-name-equal-p
 		     (car current-connection) (car tramp-current-connection))
-		  (setq tramp-current-connection current-connection))))))
+		  (setq tramp-current-connection current-connection)))))))
 
       ;; When `tramp-mode' is not enabled, or the file name is quoted,
       ;; we don't do anything.
@@ -4584,7 +4583,7 @@ tramp-handle-lock-file
 
 	;; Do the lock.
         (let ((tramp-verbose 0)
-              create-lockfiles signal-hook-function)
+              create-lockfiles)
 	  (condition-case nil
 	      (make-symbolic-link info lockname 'ok-if-already-exists)
 	    (error
@@ -4632,8 +4631,7 @@ tramp-handle-load
       (tramp-error v 'file-missing file))
     (if (not (file-exists-p file))
 	nil
-      (let ((signal-hook-function (unless noerror signal-hook-function))
-	    (inhibit-message (or inhibit-message nomessage)))
+      (let ((inhibit-message (or inhibit-message nomessage)))
 	(with-tramp-progress-reporter v 0 (format "Loading %s" file)
 	  (let ((local-copy (file-local-copy file)))
 	    (unwind-protect
@@ -4651,7 +4649,7 @@ tramp-add-hops
   "Add ad-hoc proxy definitions to `tramp-default-proxies-alist'."
   (when-let ((hops (tramp-file-name-hop vec))
 	     (item vec))
-    (let (signal-hook-function changed)
+    (let (changed)
       (dolist
 	  (proxy (reverse (split-string hops tramp-postfix-hop-regexp 'omit)))
 	(let* ((host-port (tramp-file-name-host-port item))

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

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, eliz <at> gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Sun, 24 Dec 2023 22:41:44 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Here's my current handler-bind patch, which includes some doc.

Thanks, but I don't see any documentation of handler-bind in that patch.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Richard Stallman <rms <at> gnu.org>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, eliz <at> gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Mon, 25 Dec 2023 21:39:26 -0500

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

>   > Here's my current handler-bind patch, which includes some doc.
> Thanks, but I don't see any documentation of handler-bind in that patch.

Hmm... indeed it wasn't the right patch.  Don't know how that happened.
Hopefully, this one is better,


        Stefan

[handler-bind.patch (text/x-diff, inline)]

commit 034f453c17683bfc4e55e8a9bf9402556c4cb1ae
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date:   Mon Dec 18 23:45:05 2023 -0500

    New special form `handler-bind`
    
    AFAIK, this provides the same semantics as Common Lisp's `handler-bind`.
    
    * lisp/subr.el (handler-bind): New macro.
    
    * src/eval.c (pop_handler): New function.
    (Fhandler_Bind_1): New function.
    (signal_or_quit): Handle new handlertypes `HANDLER` and `SKIP_CONDITIONS`.
    (find_handler_clause): Simplify.
    (syms_of_eval): Defsubr `Fhandler_bind_1`.
    
    * doc/lispref/control.texi (Handling Errors): Add `handler-bind`.
    
    * test/src/eval-tests.el (eval-tests--handler-bind): New test.

diff --git a/doc/lispref/control.texi b/doc/lispref/control.texi
index d4bd8c14ae3..4107963eed5 100644
--- a/doc/lispref/control.texi
+++ b/doc/lispref/control.texi
@@ -2293,6 +2293,44 @@ Handling Errors
 @code{condition-case-unless-debug} rather than @code{condition-case}.
 @end defmac
 
+Occasionally, we want to catch some errors and record some information
+about the conditions in which they occurred, such as the full
+backtrace, or the current buffer.  This kinds of information is sadly
+not available in the handlers of a @code{condition-case} because the
+stack is unwound before running that handler, so the handler is run in
+the dynamic context of the @code{condition-case} rather than that of
+the place where the error was signaled.  For those circumstances, you
+can use the following form:
+
+@defmac handler-bind handlers body <at> dots{}
+This special form runs @var{body} and if it executes without error,
+the value it returns becomes the value of the @code{handler-bind}
+form.  In this case, the @code{handler-bind} has no effect.
+
+@var{handlers} should be a list of elements of the form
+@code{(@var{conditions} @var{handler})} where @var{conditions} is an
+error condition name to be handled, or a list of condition names, and
+@var{handler} should be a form whose evaluation should return a function.
+
+Before running @var{body}, @code{handler-bind} evaluates all the
+@var{handler} forms and installs those handlers to be active during
+the evaluation of @var{body}.  These handlers are searched together
+with those installed by @code{condition-case}.  When the innermost
+matching handler is one installed by @code{handler-bind}, the
+@var{handler} function is called with a single argument holding the
+error description.
+
+@var{handler} is called in the dynamic context where the error
+happened, without first unwinding the stack, meaning that all the
+dynamic bindings are still in effect, except that all the error
+handlers between the code that signaled the error and the
+@code{handler-bind} are temporarily suspended.  Like any normal
+function, @var{handler} can exit non-locally, typically via
+@code{throw}, or it can return normally.  If @var{handler} returns
+normally, it means the handler @emph{declined} to handle the error and
+the search for an error handler is continued where it left off.
+@end defmac
+
 @node Error Symbols
 @subsubsection Error Symbols and Condition Names
 @cindex error symbol
diff --git a/etc/NEWS b/etc/NEWS
index 90ff23b7937..8e71b0a903d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1326,6 +1326,11 @@ values.
 
 * Lisp Changes in Emacs 30.1
 
+** New special form 'handler-bind'.
+Provides a functionality similar to `condition-case` except it runs the
+handler code without unwinding the stack, such that we can record the
+backtrace and other dynamic state at the point of the error.
+
 ** New 'pop-up-frames' action alist entry for 'display-buffer'.
 This has the same effect as the variable of the same name and takes
 precedence over the variable when present.
diff --git a/lisp/subr.el b/lisp/subr.el
index 93428c4a518..c53dd333303 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7497,6 +7497,36 @@ match-buffers
         (push buf bufs)))
     bufs))
 
+(defmacro handler-bind (handlers &rest body)
+  "Setup error HANDLERS around execution of BODY.
+HANDLERS is a list of (CONDITIONS HANDLER) where
+CONDITIONS should be a list of condition names (symbols) or
+a single condition name and HANDLER is a form whose evaluation
+returns a function.
+When an error is signaled during execution of BODY, if that
+error matches CONDITIONS, then the associated HANDLER
+function is called with the error as argument.
+HANDLERs can either transfer the control via a non-local exit,
+or return normally.  If they return normally the search for an
+error handler continues from where it left off."
+  ;; FIXME: Completion support as in `condition-case'?
+  (declare (indent 1) (debug ((&rest (sexp form)) body)))
+  (let ((args '())
+        (bindings '()))
+    (dolist (cond+handler (reverse handlers))
+      (let ((handler (car (cdr cond+handler)))
+            (conds (car cond+handler))
+            (handlersym (gensym "handler")))
+        (push (list handlersym handler) bindings)
+        (if (not (listp conds))
+            (progn
+              (push handlersym args)
+              (push `',conds args))
+          (dolist (cond conds)
+            (push handlersym args)
+            (push `',cond args)))))
+    `(let ,bindings (handler-bind-1 (lambda () ,@body) ,@args))))
+
 (defmacro with-memoization (place &rest code)
   "Return the value of CODE and stash it in PLACE.
 If PLACE's value is non-nil, then don't bother evaluating CODE
diff --git a/src/eval.c b/src/eval.c
index 419285eb694..e00886afbc8 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1192,6 +1192,12 @@ DEFUN ("catch", Fcatch, Scatch, 1, UNEVALLED, 0,
 
 #define clobbered_eassert(E) verify (sizeof (E) != 0)
 
+static void
+pop_handler (void)
+{
+  handlerlist = handlerlist->next;
+}
+
 /* Set up a catch, then call C function FUNC on argument ARG.
    FUNC should return a Lisp_Object.
    This is how catches are done from within C code.  */
@@ -1355,6 +1361,37 @@ or (:success BODY...), where the BODY is made of Lisp expressions.
   return internal_lisp_condition_case (var, bodyform, handlers);
 }
 
+DEFUN ("handler-bind-1", Fhandler_bind_1, Shandler_bind_1, 1, MANY, 0,
+       doc: /* Setup error handlers around execution of BODYFUN.
+BODYFUN be a function and it is called with no arguments.
+CONDITION should be a condition name (symbol).
+When an error is signaled during executon of BODYFUN, if that
+error matches CONDITION, then the associated HANDLER is
+called with the error as argument.
+HANDLER should either transfer the control via a non-local exit,
+or return normally.
+If it returns normally, the search for an error handler continues
+from where it left off.
+
+usage: (handler-bind BODYFUN [CONDITION HANDLER]...)  */)
+  (ptrdiff_t nargs, Lisp_Object *args)
+{
+  eassert (nargs >= 1);
+  Lisp_Object bodyfun = args[0];
+  Lisp_Object map = Qnil;
+  ptrdiff_t i = 2;
+  while (i < nargs)
+    {
+      Lisp_Object condition = args[i - 1], handler = args[i];
+      map = Fcons (Fcons (condition, handler), map);
+      i += 2;
+    }
+  push_handler (Fnreverse (map), HANDLER);
+  Lisp_Object ret = call0 (bodyfun);
+  pop_handler ();
+  return ret;
+}
+
 /* Like Fcondition_case, but the args are separate
    rather than passed in a list.  Used by Fbyte_code.  */
 
@@ -1731,6 +1768,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
     = (NILP (error_symbol) ? Fcar (data) : error_symbol);
   Lisp_Object clause = Qnil;
   struct handler *h;
+  int skip;
 
   if (gc_in_progress || waiting_for_input)
     emacs_abort ();
@@ -1753,6 +1791,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
       /* Edebug takes care of restoring these variables when it exits.  */
       max_ensure_room (&max_lisp_eval_depth, lisp_eval_depth, 20);
 
+      /* FIXME: 'handler-bind' makes `signal-hook-function' obsolete?  */
       call2 (Vsignal_hook_function, error_symbol, data);
     }
 
@@ -1772,16 +1811,53 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 	Vsignaling_function = backtrace_function (pdl);
     }
 
-  for (h = handlerlist; h; h = h->next)
+  for (skip = 0, h = handlerlist; h; skip++, h = h->next)
     {
-      if (h->type == CATCHER_ALL)
+      switch (h->type)
         {
+        case CATCHER_ALL:
           clause = Qt;
           break;
-        }
-      if (h->type != CONDITION_CASE)
-	continue;
-      clause = find_handler_clause (h->tag_or_ch, conditions);
+	case CATCHER:
+	  continue;
+        case CONDITION_CASE:
+          clause = find_handler_clause (h->tag_or_ch, conditions);
+	  break;
+	case HANDLER:
+	  {
+	    Lisp_Object handlers = h->tag_or_ch;
+	    for (; CONSP (handlers); handlers = XCDR (handlers))
+	      {
+	        Lisp_Object handler = XCAR (handlers);
+	        if (CONSP (handler)
+	            && !NILP (Fmemq (XCAR (handler), conditions)))
+	          {
+	            Lisp_Object error_data
+	              = (NILP (error_symbol)
+	                 ? data : Fcons (error_symbol, data));
+	            push_handler (make_fixnum (skip), SKIP_CONDITIONS);
+	            Lisp_Object retval = call1 (XCDR (handler), error_data);
+	            pop_handler ();
+	            if (CONSP (retval))
+	              {
+	                error_symbol = XCAR (retval);
+	                data = XCDR (retval);
+	                conditions = Fget (error_symbol, Qerror_conditions);
+	              }
+	          }
+	      }
+	    continue;
+	  }
+	case SKIP_CONDITIONS:
+	  {
+	    int toskip = XFIXNUM (h->tag_or_ch);
+	    while (toskip-- >= 0)
+	      h = h->next;
+	    continue;
+	  }
+	default:
+	  abort ();
+	}
       if (!NILP (clause))
 	break;
     }
@@ -1798,7 +1874,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 	  || (CONSP (clause) && !NILP (Fmemq (Qdebug, clause)))
 	  /* Special handler that means "print a message and run debugger
 	     if requested".  */
-	  || EQ (h->tag_or_ch, Qerror)))
+	  || EQ (clause, Qerror)))
     {
       debugger_called
 	= maybe_call_debugger (conditions, error_symbol, data);
@@ -1812,8 +1888,9 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
      with debugging.  Make sure to use `debug-early' unconditionally
      to not interfere with ERT or other packages that install custom
      debuggers.  */
+  /* FIXME: This could be turned into a `handler-bind` at toplevel?  */
   if (!debugger_called && !NILP (error_symbol)
-      && (NILP (clause) || EQ (h->tag_or_ch, Qerror))
+      && (NILP (clause) || EQ (clause, Qerror))
       && noninteractive && backtrace_on_error_noninteractive
       && NILP (Vinhibit_debugger)
       && !NILP (Ffboundp (Qdebug_early)))
@@ -1827,6 +1904,7 @@ signal_or_quit (Lisp_Object error_symbol, Lisp_Object data, bool keyboard_quit)
 
   /* If an error is signaled during a Lisp hook in redisplay, write a
      backtrace into the buffer *Redisplay-trace*.  */
+  /* FIXME: Turn this into a `handler-bind` installed during redisplay?  */
   if (!debugger_called && !NILP (error_symbol)
       && backtrace_on_redisplay_error
       && (NILP (clause) || h == redisplay_deep_handler)
@@ -2052,13 +2130,10 @@ find_handler_clause (Lisp_Object handlers, Lisp_Object conditions)
   register Lisp_Object h;
 
   /* t is used by handlers for all conditions, set up by C code.  */
-  if (EQ (handlers, Qt))
-    return Qt;
-
   /* error is used similarly, but means print an error message
      and run the debugger if that is enabled.  */
-  if (EQ (handlers, Qerror))
-    return Qt;
+  if (!CONSP (handlers))
+    return handlers;
 
   for (h = handlers; CONSP (h); h = XCDR (h))
     {
@@ -4461,6 +4536,7 @@ syms_of_eval (void)
   defsubr (&Sthrow);
   defsubr (&Sunwind_protect);
   defsubr (&Scondition_case);
+  defsubr (&Shandler_bind_1);
   DEFSYM (QCsuccess, ":success");
   defsubr (&Ssignal);
   defsubr (&Scommandp);
diff --git a/src/lisp.h b/src/lisp.h
index df6cf1df544..ee7ceb8e250 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3595,7 +3595,8 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs)
 }
 
 /* This structure helps implement the `catch/throw' and `condition-case/signal'
-   control structures.  A struct handler contains all the information needed to
+   control structures as well as 'handler-bind'.
+   A struct handler contains all the information needed to
    restore the state of the interpreter after a non-local jump.
 
    Handler structures are chained together in a doubly linked list; the `next'
@@ -3616,9 +3617,23 @@ record_in_backtrace (Lisp_Object function, Lisp_Object *args, ptrdiff_t nargs)
    state.
 
    Members are volatile if their values need to survive _longjmp when
-   a 'struct handler' is a local variable.  */
-
-enum handlertype { CATCHER, CONDITION_CASE, CATCHER_ALL };
+   a 'struct handler' is a local variable.
+
+   For the HANDLER and SKIP_CONDITIONS cases, we only make use of the
+   `tag_or_ch` field and none of the rest, because there's no longjmp
+   to jump to.
+   [ Maybe we should split the handler-list into a list of restart point
+     (for CATCHERs and CONDITION_CASEs) and a list of conditions handlers
+     (for HANDLERs and CONDITION_CASEs)?  ]
+
+   When running the HANDLER of a 'handler-bind', we need to
+   temporarily "mute" the CONDITION_CASEs and HANDLERs that are "below"
+   the current handler, but without hiding any CATCHERs.  We do that by
+   installing a SKIP_CONDITIONS which tells the search to skip the
+   N next conditions.  */
+
+enum handlertype { CATCHER, CONDITION_CASE, CATCHER_ALL,
+                   HANDLER, SKIP_CONDITIONS };
 
 enum nonlocal_exit
 {
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index 4589763b2f5..71049d927a9 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -282,4 +282,41 @@ eval-tests-defvaralias
   (should-error (defvaralias 'eval-tests--my-c 'eval-tests--my-d)
                 :type 'cyclic-variable-indirection))
 
+(ert-deftest eval-tests--handler-bind ()
+  ;; A `handler-bind' has no effect if no error is signaled.
+  (should (equal (catch 'tag
+                   (handler-bind ((error (lambda (err) (throw 'tag 'wow))))
+                     'noerror))
+                 'noerror))
+  ;; The handler is called from within the dynamic extent where the
+  ;; error is signaled, unlike `condition-case'.
+  (should (equal (catch 'tag
+                   (handler-bind ((error (lambda (_err) (throw 'tag 'err))))
+                     (list 'inner-catch
+                           (catch 'tag
+                             (user-error "hello")))))
+                 '(inner-catch err)))
+  ;; But inner condition handlers are temporarily muted.
+  (should (equal (condition-case nil
+                     (handler-bind
+                         ((error (lambda (_err)
+                                   (signal 'wrong-type-argument nil))))
+                       (list 'result
+                             (condition-case nil
+                                 (user-error "hello")
+                               (wrong-type-argument 'inner-handler))))
+                   (wrong-type-argument 'wrong-type-argument))
+                 'wrong-type-argument))
+  ;; Handlers do not apply to the code run within the handlers.
+  (should (equal (condition-case nil
+                     (handler-bind
+                         ((error (lambda (_err)
+                                   (signal 'wrong-type-argument nil)))
+                          (wrong-type-argument
+                           (lambda (_err) (user-error "wrong-type-argument"))))
+                       (user-error "hello"))
+                   (wrong-type-argument 'wrong-type-argument)
+                   (error 'plain-error))
+                 'wrong-type-argument)))
+
 ;;; eval-tests.el ends here

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

From: Richard Stallman <rms <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67196 <at> debbugs.gnu.org, acm <at> muc.de, eliz <at> gnu.org
Subject: Re: bug#67196: M-: uses a wrong value of debug-on-error when it is
 nil.
Date: Tue, 26 Dec 2023 23:54:07 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

When I first implemented Emacs Lisp -- before there was an editor to
fit it into -- I was desperately trying to keep it small.
Thus, when implementing error handling, I did not try to represent
error daat in such a way that a handler could analyze the situation
or continue.  So I implemented condition-case and that's all.

handler-bind is a natural step forward, but it might be desirable to make
a way to handle an error and continue from where the error happened.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

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

From: Alan Mackenzie <acm <at> muc.de>
To: 67196-done <at> debbugs.gnu.org
Cc: acm <at> muc.de
Subject: Re: M-: uses a wrong value of debug-on-error when it is nil.
Date: Thu, 24 Oct 2024 21:33:41 +0000

On Wed, Nov 15, 2023 at 13:48:40 +0000, Alan Mackenzie wrote:
> Hello, Emacs.

> On the master branch (and probably any recent Emacs):

> (i) emacs -Q
> (ii) C-h v debug-on-error RET
> (iii) M-: debug-on-error RET

> From (ii) it will be seen that debug-on-error's value is nil.
> (iii) wrongly reports that its value is t.  This is a bug.

> #########################################################################

> What is happening here is that eval-expression binds debug-on-error to
> an uninterned symbol called "t", so that d-o-e will be set to non-nil
> for the evaluation of the coming form.

> This has the unwanted side effect of ignoring the actual value of d-o-e
> in forms which use it.  For example, if the variable is set to a list of
> error symbols, this value gets ignored on evaluating a form with M-:.

This bug was fixed by the following commit:

commit 7959a63ce258c90eb3c7947ab3318c5531eb37d9
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
Date:   Mon Dec 18 23:47:56 2023 -0500

    (eval-expression): Fix bug#67196

by using the newly written handler-bind.

-- 
Alan Mackenzie (Nuremberg, Germany).

Previous Next

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