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

[Alan Mackenzie]

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).