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

From: Stefan Monnier <[email-masked]>
Date: Sat, 02 Nov 2024 10:51:46 -0400

> I don't think we need either.  In lread.c, there is already a static
> variable Vloads_in_progress (which despite the name, is not visible in
> Lisp).  This variable is a stack of the file names currently being
> loaded.  We could surely just use this, and avoid code duplication.

In that case we don't need `handler-bind` at all, and we just tack the
info when we build the error object in `Fsignal`.

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

With `handler-bind`s which add only the "current" file info, the error
will be labeled only with the relevant info, i.e. the intervening loads
between the `condition-case` (or `handler-bind`s) and the actual error.

> I've been working out details in the last two or three days.  I actually
> think the handler should exit with a (throw 'exit t).  That's because ...

What/where would be the matching `catch`?
I don't think the `handler-bind` we'd add to `load` should make
assumptions about how/where the errors will be handled.

>> I think I agree tho "be in the innermost recursive `require'" seems
>> quite vague.  But in any case the handlers of `handler-bind` are run
>> before we unwind the stack (e.g. if your nesting looks like "require =>
>> require => error" the two handlers of your two `require`s will be run
>> before we get to the debugger but the debugger will still show the full
>> stack.  Tho with your use of "resignaling" within the handlers, the
>> stack will tend to be even "more full", with the two handlers nested
>> and still active), so no matter how we do it, I think we will indeed get
>> the behavior that I believe you describe.
>
> We should respect any user setting of debug-on-error to anything non-nil.
> If non-nil, we should enter the debugger within the handler-bind's
> handler, so as to have access to Emacs's state when the error occurred.

I think we should do nothing special about `debug-on-error` and let it
be handled by the existing code: it should "just work".

> This mechanism is used in eval-expression.

That's because `eval-expression` doesn't want to obey `debug-on-error`.

> Also, this will prevent the byte compiler having (eq
> byte-compile-debug nil) subverting the call of the debugger.
> After all, when loading a file, we're not actually in the byte
> compiler, so byte-compile-debug shouldn't have an effect.

That would be nice.  Not sure how easy to do it, OTOH.

> On leaving the debugger after an error, we definitely don't want to
> enter the debugger again for the next file on the loading stack.
> Hence we should exit the handler with (throw 'exit t), or something
> like it.

If we're careful to preserve the `eq`ness of the error object, then the
debugger will know it has already been triggered for this error.

>> In any case, it should be easy to try out and change from one to the
>> other with very local changes (I'd expect that the code of the handlers
>> will be written in ELisp rather than C, right?).  So either way is fine.
> No, I think the handler code should be in C.  The function handler-bind-1
> seems very clumsy for use from C code.  It requires a function with no
> parameters, so this would likely involve creating a closure in the C
> code.  This isn't good.

I was talking about the code of the handlers.  I.e. the one we push
along with the HANDLER_BIND entry.   I'd build the handler by calling to
ELisp code (indeed, building closures from C is not good).

        Stefan