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

From: Alan Mackenzie <[email-masked]>
Date: Wed, 30 Oct 2024 21:52:13 +0000

Hello, Stefan.

On Wed, Oct 30, 2024 at 15:33:04 -0400, Stefan Monnier wrote:
> > year, in place of the current error output 

> >     test-byte-compile-errors.el:2:11: Error: Wrong type argument: listp, baz

> > the following line would appear:

> >     test-byte-compile-errors.el:2:11: While loading "test-byte-compile-errors-2.el" Error: Wrong type argument: listp, baz

> I can definitely live with this syntax, but maybe we should use
> something more like what GCC uses (e.g. for errors in #included files)
> which puts the "While loading" info on separate lines.

I thought about that, but seeing as how only one message at a time is
visible in the message area, we'd probably want to output one message
with embedded LFs, rather than several consecutive "While loading ..."s.

> > This could surely be achieved with something like the following in Fload:

> >     (handler-bind ((lambda (err)
> >                     (signal (car err)
> > 		            (combine-error-info "While loading " file
> > 			                         (cdr err)))))
> > 		  readevalloop (Qget_file_char, &input, hist_file_name,
> > 		      0, Qnil, Qnil, Qnil, Qnil);)

> That was my thinking as well (tho see below).

> > (where, obviously, the details need to be worked out).  It would need
> > augmenting with handling for (eq debug-on-error t), and probably a few
> > other things, too.

> `combine-error-info` is a bit problematic because we don't have clear
> rules about the content of (cdr err), other than the fact that it should
> be a list (tho we don't even enforce that very much).
> Most likely we could append elements to that list, but we'd have to
> worry about interactions with other libraries wanting to do similar
> things.

Do other libraries actually do such things?

> So I was thinking that we should go instead with:

>    (handler-bind ((error (lambda (err)
>                            (push file (gethash err our-table-of-error-source)))))
>      	  readevalloop (Qget_file_char, &input, hist_file_name,
>      	      0, Qnil, Qnil, Qnil, Qnil);)

> Where `our-table-of-error-source` would be a weak eq-hashtable.

Do we need a hash table when it's only going to have a few elements at
any time?  `require's rarely go more than 5 or 6 deep.  Why not just have
a simple dynamically bound list?  Or have I misunderstood what you mean?

> Emacs Lisp guarantees that the `err` we get here will be the exact same
> object that any subsequent `condition-case` will get when it finally
> handles the error so that it can use `gethash` to fetch our
> side information.

> Note that we don't `signal` the error again, instead we let the error
> handling code propagate it further, which is what `handler-bind` does
> when the handler returns normally (which should also eliminate the
> possible problems of interaction with `debug-on-error`).

The reason I suggested a signal call was so that the error information in
the successive ERRs would accumulate, rather than just being the fixed
ERR from the initial error.

And I think any call to the debugger on account of debug-on-error should
be in the innermost recursive `require', where the error is actually
signalled, so as to be of maximum use to the person debugging it.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).