Subject: bug#46573: 28.0.50; Error when edebugging setting unbound place

From: Stefan Monnier <[email-masked]>
Date: Tue, 16 Feb 2021 19:00:45 -0500

>>> (put 'gv-place 'edebug-form-spec '(form)) ;So-called "indirect spec".
>>>
>>> That's certainly not correct for the simplest forms like
>>
>> This has been there since the introduction of `gv`, so I think it
>> *is* correct.  The problem is elsewhere (likely introduced by some of
>> my recent changes to Edebug).
>
> Darn!  I thought I had finally learned how to read edebug specs.  :-/  I
> though `form' meant that it's going to be instrumented?  Hm...  but it's
> `(form)' which means, er, uhm.

Yes, it means Edebug rewrites (setf x 5) to something like:

    (edebug-after (edebug-before 1) 3 (setf (edebug-after 0 2 x) 5))

Whose behavior then depends on the definition of (edebug-after N1 N2 EXP)
as a "place", which is here:

    (put 'edebug-after 'gv-expander
         (lambda (do before index place)
           (gv-letplace (getter setter) place
             (funcall do `(edebug-after ,before ,index ,getter)
                      (lambda (store)
                        `(progn (edebug-after ,before ,index ,getter)
                                ,(funcall setter store)))))))

and indeed, there's the bug, introduced by Gemini's commit
d79cf638f278e50c22feb53d6ba556f5ce9d7853 which does (among various other
things):

    [...]
    * lisp/emacs-lisp/gv.el: Modify edebug-after's gv-expander to
    instrument in the setter as well as the getter.
    [...]

    diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
    --- a/lisp/emacs-lisp/gv.el
    +++ b/lisp/emacs-lisp/gv.el
    @@ -302,5 +302,7 @@
     (put 'edebug-after 'gv-expander
          (lambda (do before index place)
            (gv-letplace (getter setter) place
              (funcall do `(edebug-after ,before ,index ,getter)
    -                  setter))))
    +                  (lambda (store)
    +                    `(progn (edebug-after ,before ,index ,getter)
    +                            ,(funcall setter store)))))))

Gemini, how important is it to instrument the setter?  It is definitely
undesirable for Edebug, which you end up seeing the result of
computations which don't take place at all during
un-instrumented execution.  How 'bout using something like

    `(edebug-after ,before ,index ,(funcall setter store))

instead?

        Stefan