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

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

[Alan Mackenzie]

Hello, Emacs.

When byte compiling file1, and file1 requires file2, should there be an
error in file2, it is reported as being in file1, at the requires line.
This is entirely unhelpful; the reported position should be that of the
error in file2.  Things get even more unhelpful if there is a nesting of
required files, and the error occurs in a deeply nested file.

For an example, create the files ~/test-byte-compile-errors.el:

#########################################################################
;; -*- lexical-binding:t -*-
(require 'test-byte-compile-errors-2 "~/test-byte-compile-errors-2") 
#########################################################################

, and ~/test-byte-compile-errors-2.el:

#########################################################################
;; -*- lexical-binding:t -*-
(defvar foo nil)
(defun bar ()
  (setq foo)) ; <==================
(provide 'test-byte-compile-errors-2)
#########################################################################

..  From an Emacs session, do

    M-x byte-compile-file RET ~/test-byte-compile-errors.el RET

..  This will report the error as

Compiling file /home/acm/test-byte-compile-errors.el at Fri Nov  3 10:14:40 2023
test-byte-compile-errors.el:2:2: Error: Wrong number of arguments: setq, 1

..  This is not the error location.

#########################################################################

Preliminary analysis:

The pertinent error information is discarded by one of two
condition-cases in the macro displaying-byte-compile-warnings in
emacs-lisp/bytecomp.el.

If these condition-case's are disabled (for example by spiking the
enclosing `if' forms) and the necessary defuns recompiled, there instead
appears an error message in the display area.  On setting debug-on-error
to t and repeating the compilation, one gets a backtrace, which whilst
not ideal, is considerably more helpful than the original error message.

This appears to be a fundamental problem with condition-case.  When an
error occurs, the stack gets unwound before the error handlers have a
chance to analyse it.

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The pertinent error information is discarded by one of two
> condition-cases in the macro displaying-byte-compile-warnings in
> emacs-lisp/bytecomp.el.

Kind of, yes.  But the info available there is hard to use: basically at
that point we could get the error message and the backtrace, which is
what we get when we set `byte-compile-debug`.  In the case where
`byte-compile-debug` is not set, we don't really want to display
a backtrace, so we'd have to "analyze" that backtrace to try and extract
a useful error message from it, which is hard&brittle.

> If these condition-case's are disabled (for example by spiking the
> enclosing `if' forms)

By "spiking" do you mean setting `byte-compile-debug`?

> This appears to be a fundamental problem with condition-case.  When an
> error occurs, the stack gets unwound before the error handlers have a
> chance to analyse it.

[ This is going a bit on a tangent, but I think it would be good to add
  some support for something like Common-Lisp's `handler-bind`,
  i.e. error handlers that are run before unwinding the stack (so they
  can capture the stack trace as well as the value of dynbound vars at
  the moment the error occurs, for example).

  It would make it possible for ERT to refrain from activating
  `debug-on-error`, for example (which it does in order to capture the
  backtrace at the time of the error).  ]

I think for this specific problem being discussed (which is indeed
a fairly common occurrence in my experience), the better solution is to
change `load` so it adds the "context" (i.e. filename and ideally also
the approximate file position info) to errors.
[ This may require something like `handler-bind`.  ]


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Fri, Nov 03, 2023 at 12:09:03 -0400, Stefan Monnier wrote:
> > The pertinent error information is discarded by one of two
> > condition-cases in the macro displaying-byte-compile-warnings in
> > emacs-lisp/bytecomp.el.

> Kind of, yes.  But the info available there is hard to use: basically at
> that point we could get the error message and the backtrace, which is
> what we get when we set `byte-compile-debug`.  In the case where
> `byte-compile-debug` is not set, we don't really want to display
> a backtrace, ....

Why not?  We're not in the compilation any more, we're loading a file.
Some error has prevented that file loading, so we want a backtrace just
as we would get with M-x load-file foo.elc RET.

> .... so we'd have to "analyze" that backtrace to try and extract a
> useful error message from it, which is hard&brittle.

Indeed, so.

> > If these condition-case's are disabled (for example by spiking the
> > enclosing `if' forms)

> By "spiking" do you mean setting `byte-compile-debug`?

I actually commented out byte-compile-debug, replacing it with t.  But
same idea, yes.

> > This appears to be a fundamental problem with condition-case.  When an
> > error occurs, the stack gets unwound before the error handlers have a
> > chance to analyse it.

> [ This is going a bit on a tangent, but I think it would be good to add
>   some support for something like Common-Lisp's `handler-bind`,
>   i.e. error handlers that are run before unwinding the stack (so they
>   can capture the stack trace as well as the value of dynbound vars at
>   the moment the error occurs, for example).

>   It would make it possible for ERT to refrain from activating
>   `debug-on-error`, for example (which it does in order to capture the
>   backtrace at the time of the error).  ]

That sounds kind of useful.

> I think for this specific problem being discussed (which is indeed
> a fairly common occurrence in my experience), the better solution is to
> change `load` so it adds the "context" (i.e. filename and ideally also
> the approximate file position info) to errors.
> [ This may require something like `handler-bind`.  ]

Another solution would be to dispense with
display-byte-compile-warnings, just letting compiler errors generate
backtraces.

The problem here is that there is no distinction in bytecomp.el between
"external" errors (such as from require) and errors detected by the
compiler in the source file being compiled.  The first decidedly want a
backtrace, the second probably not.  All these errors are handled as
though they were "internal" errors detected by the compiler.  This is
suboptimal.

Perhaps we should report the second type of error (detected by the
compiler) by calling a warning function, as we do for warnings, and
removing the damaging condition-case's as suggested two paragraphs back.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> Why not?  We're not in the compilation any more, we're loading a file.
> Some error has prevented that file loading, so we want a backtrace just
> as we would get with M-x load-file foo.elc RET.

Hmm... that's a good point.

>> I think for this specific problem being discussed (which is indeed
>> a fairly common occurrence in my experience), the better solution is to
>> change `load` so it adds the "context" (i.e. filename and ideally also
>> the approximate file position info) to errors.
>> [ This may require something like `handler-bind`.  ]
> Another solution would be to dispense with
> display-byte-compile-warnings, just letting compiler errors generate
> backtraces.
>
> The problem here is that there is no distinction in bytecomp.el between
> "external" errors (such as from require) and errors detected by the
> compiler in the source file being compiled.

These are two fairly "clear" cases, admittedly.

But there are also cases in between where it's less clear, mostly with
errors during macro-expansion where the internal/external distinction is
not always that clear since some macros come from outside but others
come from the very file we're compiling, and where we can't easily tell
if an error is due to a bug in the macro definition or a bug in the use
of the macro.

> The first decidedly want a backtrace, the second probably not.
> All these errors are handled as though they were "internal" errors
> detected by the compiler.  This is suboptimal.

Also there are 2 questions:

- whether to give a backtrace (and/or enter the debugger).
- when we don't show a backtrace, what info do we put in the error message.

For the first, the current "solution" is to set `byte-compile-debug`.
It's not ideal, and we should improve it, but at least we do have
a solution for it.

For the second we currently don't show a good enough info and in my
previous response I focused on that part.

> Perhaps we should report the second type of error (detected by the
> compiler) by calling a warning function, as we do for warnings, and
> removing the damaging condition-case's as suggested two paragraphs back.

If the user is not asking to see backtraces, the current treatment seems
cleaner than without any `condition-case`.  So maybe those
`condition-case` should be turned into `condition-case-unless-debug`?


        Stefan

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

[Alan Mackenzie]

Hello, Stefan.

On Sun, Nov 12, 2023 at 12:28:13 -0500, Stefan Monnier wrote:
> > Why not?  We're not in the compilation any more, we're loading a file.
> > Some error has prevented that file loading, so we want a backtrace just
> > as we would get with M-x load-file foo.elc RET.

> Hmm... that's a good point.

> >> I think for this specific problem being discussed (which is indeed
> >> a fairly common occurrence in my experience), the better solution is to
> >> change `load` so it adds the "context" (i.e. filename and ideally also
> >> the approximate file position info) to errors.
> >> [ This may require something like `handler-bind`.  ]
> > Another solution would be to dispense with
> > display-byte-compile-warnings, just letting compiler errors generate
> > backtraces.

> > The problem here is that there is no distinction in bytecomp.el between
> > "external" errors (such as from require) and errors detected by the
> > compiler in the source file being compiled.

> These are two fairly "clear" cases, admittedly.

> But there are also cases in between where it's less clear, mostly with
> errors during macro-expansion where the internal/external distinction is
> not always that clear since some macros come from outside but others
> come from the very file we're compiling, and where we can't easily tell
> if an error is due to a bug in the macro definition or a bug in the use
> of the macro.

Question: will the user be able to identify the macro and its source
file if we just print the bare error message as enforced by
displaying-byte-compile-warnings?  It the answer is no or not really, we
should give her the backtrace to get started on.

> > The first decidedly want a backtrace, the second probably not.
> > All these errors are handled as though they were "internal" errors
> > detected by the compiler.  This is suboptimal.

> Also there are 2 questions:

> - whether to give a backtrace (and/or enter the debugger).
> - when we don't show a backtrace, what info do we put in the error message.

> For the first, the current "solution" is to set `byte-compile-debug`.
> It's not ideal, and we should improve it, but at least we do have
> a solution for it.

I suspect byte-compile-debug isn't widely known.  Its name is also a bit
discordant - it's not necessarily about debugging byte-compile, it's
just to get sensible error messages when something goes wrong,
especially when that something is not part of the byte compiler.

> For the second we currently don't show a good enough info and in my
> previous response I focused on that part.

Indeed, for the error message which provoked this bug report, the
current information is poor indeed.  Considering that require's can be
nested, we only tell the user the identity of the outermost one.

> > Perhaps we should report the second type of error (detected by the
> > compiler) by calling a warning function, as we do for warnings, and
> > removing the damaging condition-case's as suggested two paragraphs back.

> If the user is not asking to see backtraces, the current treatment seems
> cleaner than without any `condition-case`.

It's "neat and tidy", but at the cost of discarding all useful
information.  There are other common situations in Emacs where
the debugger is entered, or a backtrace output without debug-on-error
having to be set.  Perhaps this one should join them.

> So maybe those `condition-case` should be turned into
> `condition-case-unless-debug`?

I think this would be a very useful first step.  I think it likely a
user will set debug-on-error on encountering any unhelpful error
message.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> But there are also cases in between where it's less clear, mostly with
>> errors during macro-expansion where the internal/external distinction is
>> not always that clear since some macros come from outside but others
>> come from the very file we're compiling, and where we can't easily tell
>> if an error is due to a bug in the macro definition or a bug in the use
>> of the macro.
>
> Question: will the user be able to identify the macro and its source
> file if we just print the bare error message as enforced by
> displaying-byte-compile-warnings?

I think we print a "bare" error message (together with the location of
the macro call).  Often it's good enough (e.g. when the error is really
in the macro call itself).  Sometimes it's very perplexing :-(

> It the answer is no or not really, we should give her the backtrace to
> get started on.

Says the one who claimed earlier that backtraces are stressful :-)

Dumping the backtrace is a kind of cop-out.

Don't get me wrong, I love backtraces, but I don't think we should
blissfully throw backtraces at unsuspecting users (unlike Python, say).
IOW, we should first work harder to provide better error messages.

>> For the first, the current "solution" is to set `byte-compile-debug`.
>> It's not ideal, and we should improve it, but at least we do have
>> a solution for it.
>
> I suspect byte-compile-debug isn't widely known.  Its name is also a bit
> discordant - it's not necessarily about debugging byte-compile, it's
> just to get sensible error messages when something goes wrong,
> especially when that something is not part of the byte compiler.

Agreed.

>> For the second we currently don't show a good enough info and in my
>> previous response I focused on that part.
> Indeed, for the error message which provoked this bug report, the
> current information is poor indeed.  Considering that require's can be
> nested, we only tell the user the identity of the outermost one.

We don't even give that info.  We just give the line number of the
`require`.  It's almost as good as the outermost file name, but not quite.

> It's "neat and tidy", but at the cost of discarding all useful
> information.  There are other common situations in Emacs where
> the debugger is entered, or a backtrace output without debug-on-error
> having to be set.

Hmm... I can't think of such a situation.  When/where do we show
a backtrace without the user's explicit request?

>> So maybe those `condition-case` should be turned into
>> `condition-case-unless-debug`?
> I think this would be a very useful first step.  I think it likely a
> user will set debug-on-error on encountering any unhelpful error
> message.

AFAICT this would basically be equivalent to aliasing
`byte-compile-debug` to `debug-on-error`.

It may turn out to be annoying occasionally, but I think it's worth
a try (I've never found it useful to have `byte-compile-debug` set to
t without also setting `debug-on-error` to t).


        Stefan

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

[Drew Adams]

> Dumping the backtrace is a kind of cop-out.
> 
> Don't get me wrong, I love backtraces, but I don't think we should
> blissfully throw backtraces at unsuspecting users (unlike Python, say).
> IOW, we should first work harder to provide better error messages.

OT, so ... sorry.

It just occurred to me that instead of just having
a Boolean `debug-on-error', which turns use of the
debugger on/off for an error, there might be a
third possibility: show an error message and let
you then decide whether to open the backtrace --
maybe click (or `RET') the error msg, or in some
other way make the choice.

IOW, we could perhaps prepare a backtrace buffer
without actually entering its recursive edit etc.
If a user doesn't ask to see/use it then it just
sits there (buried) as an unused buffer.

Or display of the error msg could allow for (1)
activating/entering the backtrace buffer, (2)
leaving it buried, or (3) deleting it.

No idea about implementation or reasonableness.
Just something that occurred to me.  Yes, first
priority should be a good error msg.  Yes, users
should be able to optionally open the debugger.
But could we maybe  give them that possibility
after showing the error msg?

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

[Alan Mackenzie]

Hello, Stefan.

It's been almost a year since this bug got lost in the storm of other
things which needed doing.  I'd like to come back to it now.  To refresh
your memory, the problem was about not getting any information about foo
when an error occurred during (require 'foo).

And no apologies for the top posting, which just seems appropriate,
since new possibilities (handler-bind in particular, thanks!) have
appeared in the last year.

My latest idea is to put (the C equivalent of) handler-bind around the
readevalloop call near the end of Fload.  So with my test files from last
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

..  The "While loading "foo.el" " bit would be repeated as needed for
nested (require 'foo)s.  OK, the error line could be quite long, but the
information would at least be there.

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);)

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

What do you think of the idea?

-- 
Alan Mackenzie (Nuremberg, Germany).


On Sun, Nov 12, 2023 at 16:19:36 -0500, Stefan Monnier wrote:
> >> But there are also cases in between where it's less clear, mostly with
> >> errors during macro-expansion where the internal/external distinction is
> >> not always that clear since some macros come from outside but others
> >> come from the very file we're compiling, and where we can't easily tell
> >> if an error is due to a bug in the macro definition or a bug in the use
> >> of the macro.

> > Question: will the user be able to identify the macro and its source
> > file if we just print the bare error message as enforced by
> > displaying-byte-compile-warnings?

> I think we print a "bare" error message (together with the location of
> the macro call).  Often it's good enough (e.g. when the error is really
> in the macro call itself).  Sometimes it's very perplexing :-(

> > It the answer is no or not really, we should give her the backtrace to
> > get started on.

> Says the one who claimed earlier that backtraces are stressful :-)

> Dumping the backtrace is a kind of cop-out.

> Don't get me wrong, I love backtraces, but I don't think we should
> blissfully throw backtraces at unsuspecting users (unlike Python, say).
> IOW, we should first work harder to provide better error messages.

> >> For the first, the current "solution" is to set `byte-compile-debug`.
> >> It's not ideal, and we should improve it, but at least we do have
> >> a solution for it.

> > I suspect byte-compile-debug isn't widely known.  Its name is also a bit
> > discordant - it's not necessarily about debugging byte-compile, it's
> > just to get sensible error messages when something goes wrong,
> > especially when that something is not part of the byte compiler.

> Agreed.

> >> For the second we currently don't show a good enough info and in my
> >> previous response I focused on that part.
> > Indeed, for the error message which provoked this bug report, the
> > current information is poor indeed.  Considering that require's can be
> > nested, we only tell the user the identity of the outermost one.

> We don't even give that info.  We just give the line number of the
> `require`.  It's almost as good as the outermost file name, but not quite.

> > It's "neat and tidy", but at the cost of discarding all useful
> > information.  There are other common situations in Emacs where
> > the debugger is entered, or a backtrace output without debug-on-error
> > having to be set.

> Hmm... I can't think of such a situation.  When/where do we show
> a backtrace without the user's explicit request?

> >> So maybe those `condition-case` should be turned into
> >> `condition-case-unless-debug`?
> > I think this would be a very useful first step.  I think it likely a
> > user will set debug-on-error on encountering any unhelpful error
> > message.

> AFAICT this would basically be equivalent to aliasing
> `byte-compile-debug` to `debug-on-error`.

> It may turn out to be annoying occasionally, but I think it's worth
> a try (I've never found it useful to have `byte-compile-debug` set to
> t without also setting `debug-on-error` to t).


>         Stefan

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

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

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.
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`).


        Stefan

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

[Alan Mackenzie]

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

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

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

I don't have an opinion on that.  I only care about the case where that
info ends up in a file or buffer, along with other warnings and errors,
such as when I do `make` or `byte-compile-file`.  Ideally I'd like to be
able to click on each "While loading" to be brought to the place of the
corresponding `require`.  And ideally this would work with the existing
entries of `compilation-error-regexp-alist`.

>> `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?

Currently, this would be the first, but since I added `handler-bind`
I've already felt like doing such things on a few occasions, so it's
only a question of time.

>> 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?

A hashtable is not the only solution, indeed.  But a weak hashtable
makes it possible to skip the need to use something that's "dynamically
bound", and hence to have to think about where we do the dynamic binding
and what to do if it's nested etc...
IOW, it seems simpler to me.

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

In my suggestion I also accumulate them, but I put them in the side-info
hashtable instead of inside the error object.  I think it is important to
preserve the `eq`ness of the error object (since it embodies the fact
that we're still handling the same error), so if we don't use a side
table we would probably want to "combine" by mutating the error object.

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

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.

More concretely, with your code, I think the debugger will be called
with a stack that looks like

     <calling the debugger>
     signal(...)
     ...
     <calling the handler of the outer handler-bind>
     signal(...)
     ...
     <calling the handler of the inner handler-bind>
     error("the actual error")
     ...
     handler-bind(...)    ; the inner one
     require(...)
     ...
     handler-bind(...)    ; the outer one
     require(...)

whereas with the code I suggest the stack should look like

     <calling the debugger>
     error("the actual error")
     ...
     handler-bind(...)    ; the inner one
     require(...)
     ...
     handler-bind(...)    ; the outer one
     require(...)

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.


        Stefan