bug#31090: 26.0.91; Edebug incorrectly instruments unquotes in nested backquotes

[Gemini Lasswell <gazally@runbox.com>]

Hi Alan,
> I think we've been here before, in bug #16184.  The problem is that the
> instrumented form hasn't called edebug-enter, for whatever reason, hence
> hasn't pushed a cons onto edebug-offset-indices, which is thus still
> nil.  The (setcar edebug-offset-indices ...) at the start of
> edebug-slow-before (to which edebug-before is aliased) thus fails.

Yes, whenever Edebug's instrumentation is malformed due to a bug,
edebug-before is where it fails. Consider what edebug-before does: if
you are stepping through code, or type a key while running instrumented
code, it brings up Edebug's recursive edit at the location the form was
defined, the correct buffer, function and offset within the function. In
order to do that it relies upon edebug-enter having looked up that
information and placed it in Edebug's dynamic variables.

> At the time, I committed a solution which involved initialising that
> variable to '(0) instead of nil.  You persuaded me to revert that
> change, saying [Date: Fri, 30 Dec 2016 15:27:37 -0800, Subject: Re:
> bug#16184: 24.3.50; edebug and eval-when-compiler don't work together]:
>
>   > I haven't able to reproduce the bug with cc-eval-when-compile, just
>   > eval-and-compile. But the thing that is supposed to make Edebug wrap a
>   > form in edebug-enter is the use of def-form or def-body in the Edebug
>   > spec. It works for eval-when-compile which has the Edebug spec (&rest
>   > def-form). The body of eval-and-compile doesn't get wrapped because
>   > its Edebug spec is t, so the bug happens there.
>
>   > cc-eval-when-compile has the same Edebug spec as eval-when-compile, so
>   > its body should get wrapped by edebug-enter. If that's not happening
>   > in your Emacs, it's a bug in Edebug which is different from the
>   > eval-and-compile Edebug spec bug.
>
> This, if true, implies that using an instrumented macro is liable to
> produce this error if that macro doesn't have an appropriate edebug
> spec.  This seems to be an unreasonable prerequisite - I think the
> typical work flow would be writing a macro first, testing it with the
> help of Edebug, and then, possibly writing an edebug spec.

If a macro, instrumented or not, has no Edebug spec, this error should
not happen. Without an Edebug spec, Edebug does not instrument any of
the forms that are arguments to the macro, so there shouldn't be any
instrumentation in the macro expansion. The fact that `(,,fn ,@,args) is
currently making instrumentation show up in the macro expansion is a bug
in Edebug.

If a macro has an incorrect Edebug spec, it can cause this error, in
particular if the spec uses 'form' instead of 'def-form' to describe one
of the macro's arguments, and then the macro wraps that form in a lambda
and saves it somewhere, and it later gets called outside of its original
context.

> Perhaps we should think again about my solution from December 2016,
> namely initialising edebug-offset-indices to a cons '(0).  I've just
> tried this, and got the error edebug-freq-count is unbound.  So perhaps
> we should give initial values to all these declared dynamic variables
> which are bound by edebug-enter, for the case when edebug-enter doesn't
> get called.

This isn't going to work for the reason I described above, because
Edebug won't know where to invoke its recursive edit. Changing
edebug-before to fail silently when its dynamic variables are unbound
would not solve the problem either because if mis-instrumented code is
called from a correctly instrumented function, then edebug-before will
find that the data in the dynamic variables is wrong instead of missing.

For an example of that, go back to the steps to reproduce this bug and
evaluate my-test with C-u C-M-x instead of C-M-x, and then run it. The
result will be an error message, "Args out of range: [20 31 38 39], 24",
because edebug-before is trying to use an index (form number) from
my-should in the arrays for my-test, which has fewer forms. If my-test
was a longer function with the same or more forms than my-should, then
the error would not happen but stepping through my-test with Edebug
would do some weird jumping around when it got to the erroneous
edebug-before.

While pondering this I found a comment in edebug.el saying that the
reason for the distinction between form and def-form in the edebug specs
for macro arguments was that using def-form everywhere would be
expensive. But that was written in 1994, and it doesn't sound
prohibitively expensive to me now. I'll give it a try and report back.