bug#66912: With `require', the byte compiler reports the wrong file for errors.

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Wed, Nov 06, 2024 at 13:51:05 -0500, Stefan Monnier wrote:
> >> I was thinking we wouldn't bother, just like the we don't bother
> >> emptying the match-data after we used it.
> > I'm getting problems when debug-on-error is non-nil, and a *Backtrace*
> > is on the screen.  At this stage, Vloads_in_progress is still non-nil.

> Right, because we're still in the process of loading those files.

> > So when another error occurs, even a trivial one like "next-line: End of
> > buffer", its error message gets prefixed by the "While loading ..."
> > strings.

> Of course.

Of course?  You foresaw this and didn't tell me about it.  :-)

> This is an instance of the problem I described as follows:

>     But I don't think it would be correct in all cases: if file A loads
>     file B which compiles file C which loads file D which signals an
>     error we want the compiler error message to say "error in D loaded
>     from C" and not "error in D loaded from C loaded from B loaded from A".

No, I don't think so.  Loading a file which compiles another file is
somewhat unusual, to say the least.  I don't recall seeing it anywhere
in Emacs.  What the message would say in such a circumstance is "While
loading A... While loading B... While loading D... \n(Wrong type
argument listp baz)".  This is accurate, though maybe not the whole
story in the case where there's a compilation in the middle of all that
lot.  This case is probably rare enough not to matter.

> > This is a Bad Thing.  Do you have any suggestions for a fix?

> Other than changing to another approach (e.g. one that incrementally
> adds the context info via `handler-bind`s so you only get the context
> that found "between" the error and the place where we actually handle
> the error), ....

I don't really understand that at the moment.  I'll need to think about
it.

> .... I think the easy solution is to consider not the whole
> `Vloads_in_progress_of_last_error` but only the part which is not
> still present in `Vloads_in_progress`.

In either case, the simplicity of the approach, which was meant to
recommend it, has disappeared, being replaced by appreciable complexity.

> Something like

>     (while (not (equal Vloads_in_progress_of_last_error Vloads_in_progress))
>       (setq msg (concat msg " while loading "
>                         (pop Vloads_in_progress_of_last_error))))

This doesn't make sense in the current implementation (sorry for not
sending you a patch), because Vloads_in_progress_at_error is copied from
Vloads_in_progress in signal_or_quit.

> > I've tried it, and the above problem seems definitely to make it less
> > simple than the approach I originally took (which currently works).

> AFAIK, your previous approach suffered from the exact same problem, tho
> maybe in fewer cases.

No, it doesn't.  There, Vloads_still_in_progress is kept in synch with
Vloads_in_progress when Fload operations start and end.  When the
debugger or error handler outputs a message using Vl_still_i_p, it
resets that variable to nil, preventing it getting output again.

That strategem isn't available in the newer approach, where
Vl_i_p_at_error gets "refreshed" at each error occurring in the
debugger's recursive edit.

At the moment, I think my currently working solution is the best way to
go.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).