Re: Fontification error backtrace [Was: Why is it so difficult to get a Lisp backtrace?]

[Alan Mackenzie]

Hello, Eli.

On Tue, Jun 28, 2022 at 14:57:44 +0300, Eli Zaretskii wrote:
> > Date: Mon, 27 Jun 2022 19:48:45 +0000
> > Cc: larsi@gnus.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > > The only two ways of having that, AFAIK, are:

> > > > >   . emit the backtrace int *Messages*
> > > > >   . add the backtrace to delayed-warnings-list

> > > > Why can't the backtrace be generated and stored, and only displayed at a
> > > > subsequent redisplay (probably the next one)?

> > > That's what the second alternative above already does.

> > OK, I've got some code.  :-)

> Sigh.  I suggested two alternatives, but you effectively
> ignored/rejected both of them.  Why?

*Messages* just doesn't feel the right place to put a backtrace.

delayed-warnings-list consists of a list of structured lists, rather than
the vanilla text output by debug-early.  So the backtrace would need some
massaging to fit into delayed-warnings-list.  Also these delayed warnings
get displayed between post-command-hook and redisplay, which is not an
optimal time to display a backtrace generated in redisplay.

However, the display of the backtrace was not the difficult bit in
writing this code - that was triggering the backtrace when there was an
error signalled, and only then.

> > What it does, on a fontification error during redisplay, is to write a
> > backtrace into the *Backtrace* buffer, attempts a pop-to-buffer (which
> > isn't working, not surprisingly), and aborts the redisplay.

> I don't understand why you needed to do it this way.

I copied it from the debug-on-error backtrace.  It's just my first try at
getting the thing displayed.  I'm not at all experienced in creating and
displaying windows.

> First, what's wrong with the two ways I suggested?  They are simple to
> implement, use existing infrastructure, don't require any changes in
> the APIs, and are safe.  Why didn't you use one of them?

See above.

> Second, if you _must_ roll out something new (but please explain why),
> why do you need a special handler? why not slightly modify the
> existing handler, safe_eval_handler?  Doing that would have avoided
> the need to change existing APIs.

That way hadn't occurred to me.  It would surely be better than
yesterday's patch.

> Third, why are you singling out the fontifications? they are not the
> only source of Lisp errors in code called by the display engine.
> Moreover, handle_fontified_prop is not the only way of calling
> expensive Lisp code that deals with faces.

All that's true, yet fontification errors are an important source of
difficult to debug errors.  We could certainly extend the new backtrace
mechanism to handle these other types of bug, too.

> Forth, why on Earth do you want to abort redisplay??  Once you
> collected the backtrace, there should be no need in such drastic
> measures.

There's really no need for this, thinking about it.  Given that the
condition-case has handled the error, there's nothing stopping the
redisplay from continuing successfully on the other visible windows.

> I've just finished adding a feature that _really_ needs to abort
> redisplay, and I can tell you: it's NOT safe!  It can easily crash
> Emacs within a few commands, and your patch didn't do what I needed to
> do to avoid that.  You just throw to top-level from the bowels of the
> display code, assuming that it's okay.  Well, it isn't, not if
> handle_fontified_prop is called from redisplay_internal and its
> subroutines!

OK.

> So basically, I'm extremely confused, to say the least, by the
> proposed changes and by your rationale, and am not happy at all with
> installing them.

My patch was more a proof of concept, a demonstration that a backtrace
from within redisplay is possible.  It wasn't intended to be finished
code, and clearly isn't in that state, yet.

> > As already noted, the *Backtrace* buffer doesn't appear on the screen,
> > yet.  Any hints on how to achieve this would be most welcome.

> Any reasons you'd want that?  Once the buffer exists, why do you need
> to actually pop it up (and thus potentially disrupt the very
> window/frame configuration which you are trying to debug)?

A debug-on-error backtrace gets displayed immediately, so it would help
consistency.  If the window where the error occurred isn't occupying its
entire frame, there should be room to put the backtrace somewhere else, I
think.  I'm not at all experienced with things like pop-to-buffer, but
they're supposed to be very flexible.

> > It seems I could really do with after-redisplay-hook, in which I
> > could call pop-to-buffer, then cause redisplay to be restarted.

> I think you misunderstand what such a hook could do, and why it could
> be useful, if at all, in the first place.  If all you want is to cause
> the main loop perform some command after redisplay cycle ends, we
> already have several mechanisms in place for doing that.

That's what I want, yes, but couldn't find any such mechanism anywhere
when I looked.

> But all that is secondary.  We should first agree on the correct
> design and implementation of the main part, which is how to handle
> these errors for collecting Lisp backtrace information.

Yes.  How about what you suggested above, namely creating the backtrace
in save_eval_handler rather than signal_or_quit, at least as a second
attempt.  Though I don't think this would handle errors in other Lisp
called from redisplay.

Thanks for such a full review and helpful criticism.

-- 
Alan Mackenzie (Nuremberg, Germany).