Re: Redisplay hook error backtraces

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Thu, Jul 14, 2022 at 08:12:51 +0300, Eli Zaretskii wrote:
> > Date: Wed, 13 Jul 2022 20:12:42 +0000
> > Cc: larsi@gnus.org, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I still don't think I understand why testing redisplaying_p and the
> > > new optional variable would not be enough.

> > We've got to distinguish between the signals we want to generate
> > backtraces for and those we don't.  redisplaying_p is not relevant to
> > that, I think.  For example, we don't want to generate a backtrace
> > for a "failed" evaluation of the code generated by `c-safe'.

> You want to distinguish errors inside condition-case?

More, distinguish the different condition-cases in which errors might
occur.

> If they are not already marked in some way that allows you to do so, I
> think it's better to do it the other way around: bind some variable in
> internal_lisp_condition_case before invoking the body.

OK, I think I see what you're getting at.  But this would mean binding
that variable at _every_ condition_case, and needing some way of deciding
which way to bind it (an extra argument in every use of
internal_lisp_condition_case, perhaps?).  Why would this be better than
the way I have already implemented?

> > > > You mean, all the changes in eval.c and keyboard.c?  I think the
> > > > changes to internal_condition_case_n are essential to the patch,
> > > > and I honestly don't think it can be done much more elegantly,
> > > > but I'm open to suggestions.

> > > Can we discuss how to implement it without introducing a special
> > > handler and without adding new safe_run_hooks_* functions?

I think we need the new function safe_run_hooks_2_with_backtrace (see
below), since there is currently no "safe" function for hooks with two
arguments.  But some of the other ones could disappear (see below).

> > OK.  Perhaps with extra optional arguments, that kind of thing?

> Maybe, I don't know.  I still think the job of signal_or_quit and
> safe_run_hooks is almost the same when you want to collect backtrace,
> so too many differences strike me as unexpected.

OK, I have an idea.  I restore the variable redisplay_lisping back into
the code (I took it out last night), binding it to true (or Qt?) at every
place in xdisp.c where redisplay calls a Lisp hook.  I then test that
variable in internal_condition_case_n in place of having the extra bool
argument to that function.

That would then get rid of the new functions
safe_run_hooks_with_backtrace_funcall and safe_run_hooks_with_backtrace.
We could also rename safe_run_hooks_2_with_backtrace by removing
"_with_backtrace" from the name.

What do you think?

-- 
Alan Mackenzie (Nuremberg, Germany).