unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65620: void function edebug-after
@ 2023-08-30 12:57 Alan Mackenzie
  2023-08-30 23:09 ` Michael Heerdegen
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2023-08-30 12:57 UTC (permalink / raw)
  To: 65620

Hello, Emacs.

On a recent master branch Emacs:
(i) emacs -Q
(ii) Insert the following into *scratch*:

(defmacro hash-if (condition then-form &rest else-forms)
  "A conditional compilation macro analogous to C's #if.
Evaluate CONDITION at macro-expansion time.  If it is non-nil,
expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
enclosed in a `progn' form.  ELSE-FORMS may be empty."
  (declare (indent 2)
           (debug (form sexp &rest sexp)))
  (if (eval condition lexical-binding)
      then-form
    (cons 'progn else-forms)))

(defun foo (bar)
  (hash-if (< emacs-major-version 19)
      (car bar)
    (cons bar bar)))

(iii) Evaluate hash-if by putting point after it and doing C-x C-e.
(iv) Attempt to instrument foo for edebug by putting point inside foo and
  doing C-u C-M-x.  This throws the error: "Ignoring macroexpansion
  error: (void-function edebug-after)".  This attempt to evaluate
  edebug-after is a bug.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-30 12:57 bug#65620: void function edebug-after Alan Mackenzie
@ 2023-08-30 23:09 ` Michael Heerdegen
  2023-08-31  7:55   ` Gerd Möllmann
  0 siblings, 1 reply; 14+ messages in thread
From: Michael Heerdegen @ 2023-08-30 23:09 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 65620

Alan Mackenzie <acm@muc.de> writes:

> (defmacro hash-if (condition then-form &rest else-forms)
>   "A conditional compilation macro analogous to C's #if.
> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
> enclosed in a `progn' form.  ELSE-FORMS may be empty."
>   (declare (indent 2)
>            (debug (form sexp &rest sexp)))
>   (if (eval condition lexical-binding)
>       then-form
>     (cons 'progn else-forms)))

Dunno if someone is able to fix this (I'm not).  Until then using
`def-form` `or `sexp` instead of `form` works in a better way (the
former edebugs CONDITION when instrumenting, the latter would omit
edebugging the CONDITION entirely).

Anyway, the key point in the above example is that macroexpanding (while
instrumenting) combined with the `eval' call seems to lead to the
evaluation of instrumented code outside of an Edebug session when
CONDITION is instrumented using `form`.  `eval-when-compile' uses
`def-form` for example - I guess using `form` in this case doesn't work
as one might expect.


Michael.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-30 23:09 ` Michael Heerdegen
@ 2023-08-31  7:55   ` Gerd Möllmann
  2023-08-31  8:02     ` Gerd Möllmann
  2023-08-31 13:50     ` Alan Mackenzie
  0 siblings, 2 replies; 14+ messages in thread
From: Gerd Möllmann @ 2023-08-31  7:55 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: Alan Mackenzie, 65620

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Alan Mackenzie <acm@muc.de> writes:
>
>> (defmacro hash-if (condition then-form &rest else-forms)
>>   "A conditional compilation macro analogous to C's #if.
>> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
>> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
>> enclosed in a `progn' form.  ELSE-FORMS may be empty."
>>   (declare (indent 2)
>>            (debug (form sexp &rest sexp)))
>>   (if (eval condition lexical-binding)
>>       then-form
>>     (cons 'progn else-forms)))
>
> Dunno if someone is able to fix this (I'm not).  Until then using
> `def-form` `or `sexp` instead of `form` works in a better way (the
> former edebugs CONDITION when instrumenting, the latter would omit
> edebugging the CONDITION entirely).
>
> Anyway, the key point in the above example is that macroexpanding (while
> instrumenting) combined with the `eval' call seems to lead to the
> evaluation of instrumented code outside of an Edebug session when
> CONDITION is instrumented using `form`.  `eval-when-compile' uses
> `def-form` for example - I guess using `form` in this case doesn't work
> as one might expect.

I think what's happening here is like this:

By using 'form' for condition, we're telling edebug to instruments it.
That is, the argument eval sees when foo is instrumented is whatever
edebug wraps around the condition (< ...), and that contains the
eval-after.  Using sexp for the condition doesn't instrument the condition.

One can follow that in the backtrace.

So, I guess there's nothing to fix here.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-31  7:55   ` Gerd Möllmann
@ 2023-08-31  8:02     ` Gerd Möllmann
  2023-08-31 13:50     ` Alan Mackenzie
  1 sibling, 0 replies; 14+ messages in thread
From: Gerd Möllmann @ 2023-08-31  8:02 UTC (permalink / raw)
  To: Michael Heerdegen; +Cc: Alan Mackenzie, 65620

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> Alan Mackenzie <acm@muc.de> writes:
>>
>>> (defmacro hash-if (condition then-form &rest else-forms)
>>>   "A conditional compilation macro analogous to C's #if.
>>> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
>>> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
>>> enclosed in a `progn' form.  ELSE-FORMS may be empty."
>>>   (declare (indent 2)
>>>            (debug (form sexp &rest sexp)))
>>>   (if (eval condition lexical-binding)
>>>       then-form
>>>     (cons 'progn else-forms)))
>>
>> Dunno if someone is able to fix this (I'm not).  Until then using
>> `def-form` `or `sexp` instead of `form` works in a better way (the
>> former edebugs CONDITION when instrumenting, the latter would omit
>> edebugging the CONDITION entirely).
>>
>> Anyway, the key point in the above example is that macroexpanding (while
>> instrumenting) combined with the `eval' call seems to lead to the
>> evaluation of instrumented code outside of an Edebug session when
>> CONDITION is instrumented using `form`.  `eval-when-compile' uses
>> `def-form` for example - I guess using `form` in this case doesn't work
>> as one might expect.
>
> I think what's happening here is like this:
>
> By using 'form' for condition, we're telling edebug to instruments it.
> That is, the argument eval sees when foo is instrumented is whatever

Sorry, "sees" is midleading: eval has as argument ...





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-31  7:55   ` Gerd Möllmann
  2023-08-31  8:02     ` Gerd Möllmann
@ 2023-08-31 13:50     ` Alan Mackenzie
  2023-08-31 14:41       ` Gerd Möllmann
  1 sibling, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2023-08-31 13:50 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Michael Heerdegen, acm, 65620

Hello, Gerd.

On Thu, Aug 31, 2023 at 09:55:18 +0200, Gerd Möllmann wrote:
> Michael Heerdegen <michael_heerdegen@web.de> writes:

> > Alan Mackenzie <acm@muc.de> writes:

> >> (defmacro hash-if (condition then-form &rest else-forms)
> >>   "A conditional compilation macro analogous to C's #if.
> >> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
> >> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
> >> enclosed in a `progn' form.  ELSE-FORMS may be empty."
> >>   (declare (indent 2)
> >>            (debug (form sexp &rest sexp)))
> >>   (if (eval condition lexical-binding)
> >>       then-form
> >>     (cons 'progn else-forms)))

> > Dunno if someone is able to fix this (I'm not).  Until then using
> > `def-form` `or `sexp` instead of `form` works in a better way (the
> > former edebugs CONDITION when instrumenting, the latter would omit
> > edebugging the CONDITION entirely).

> > Anyway, the key point in the above example is that macroexpanding (while
> > instrumenting) combined with the `eval' call seems to lead to the
> > evaluation of instrumented code outside of an Edebug session when
> > CONDITION is instrumented using `form`.  `eval-when-compile' uses
> > `def-form` for example - I guess using `form` in this case doesn't work
> > as one might expect.

> I think what's happening here is like this:

> By using 'form' for condition, we're telling edebug to instruments it.
> That is, the argument eval sees when foo is instrumented is whatever
> edebug wraps around the condition (< ...), and that contains the
> eval-after.  Using sexp for the condition doesn't instrument the condition.

Or, put a different way, edebug has instrumented CONDITION, then tries to
evaluate this.  This fails because there is no call to
edebug-make-enter-wrapper around the thing, which would defalias
edebug-after and edebug-before, and set up several lists that edebug
needs.

> One can follow that in the backtrace.

> So, I guess there's nothing to fix here.

I don't think I agree.  eval (and probably apply and funcall and its
variants) should somehow generate an "optional" edebug-make-enter-wrapper
around them.  This is currently not done.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-31 13:50     ` Alan Mackenzie
@ 2023-08-31 14:41       ` Gerd Möllmann
  2023-09-01  9:23         ` Alan Mackenzie
  0 siblings, 1 reply; 14+ messages in thread
From: Gerd Möllmann @ 2023-08-31 14:41 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Michael Heerdegen, 65620

Alan Mackenzie <acm@muc.de> writes:

> Hello, Gerd.

Hallo Alan, Grüße nach Nürnberg :-).

>
> On Thu, Aug 31, 2023 at 09:55:18 +0200, Gerd Möllmann wrote:
>> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> > Alan Mackenzie <acm@muc.de> writes:
>
>> >> (defmacro hash-if (condition then-form &rest else-forms)
>> >>   "A conditional compilation macro analogous to C's #if.
>> >> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
>> >> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
>> >> enclosed in a `progn' form.  ELSE-FORMS may be empty."
>> >>   (declare (indent 2)
>> >>            (debug (form sexp &rest sexp)))
>> >>   (if (eval condition lexical-binding)
>> >>       then-form
>> >>     (cons 'progn else-forms)))
>
>> > Dunno if someone is able to fix this (I'm not).  Until then using
>> > `def-form` `or `sexp` instead of `form` works in a better way (the
>> > former edebugs CONDITION when instrumenting, the latter would omit
>> > edebugging the CONDITION entirely).
>
>> > Anyway, the key point in the above example is that macroexpanding (while
>> > instrumenting) combined with the `eval' call seems to lead to the
>> > evaluation of instrumented code outside of an Edebug session when
>> > CONDITION is instrumented using `form`.  `eval-when-compile' uses
>> > `def-form` for example - I guess using `form` in this case doesn't work
>> > as one might expect.
>
>> I think what's happening here is like this:
>
>> By using 'form' for condition, we're telling edebug to instruments it.
>> That is, the argument eval sees when foo is instrumented is whatever
>> edebug wraps around the condition (< ...), and that contains the
>> eval-after.  Using sexp for the condition doesn't instrument the condition.
>
> Or, put a different way, edebug has instrumented CONDITION, then tries to
> evaluate this.  This fails because there is no call to
> edebug-make-enter-wrapper around the thing, which would defalias
> edebug-after and edebug-before, and set up several lists that edebug
> needs.

I think that's correct, but I wouldn't say Edebug evaluates CONDITION,
but we probably mean the same thing: CONDITION is instrumented and
HASH-IF then gets that as argument when FOO is macroexpanded.  Then the
execution of HASH-IF tries to evaluate the instrumented condition etc.

>> One can follow that in the backtrace.
>
>> So, I guess there's nothing to fix here.
>
> I don't think I agree.  eval (and probably apply and funcall and its
> variants) should somehow generate an "optional" edebug-make-enter-wrapper
> around them.  This is currently not done.

That would be one way.  On the other hand, the instrumentation of
CONDITION is actually kind of pointless, because nothing will be
left of it in the fully macroexpanded FOO.  So, one cannot step through
CONDITION with Edebug anyway.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-08-31 14:41       ` Gerd Möllmann
@ 2023-09-01  9:23         ` Alan Mackenzie
  2023-09-01 12:27           ` Gerd Möllmann
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2023-09-01  9:23 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Michael Heerdegen, acm, 65620

Hallo, Gerd,

Grüß aus Nürnberg!

On Thu, Aug 31, 2023 at 16:41:21 +0200, Gerd Möllmann wrote:
> Alan Mackenzie <acm@muc.de> writes:

> Hallo Alan, Grüße nach Nürnberg :-).


> > On Thu, Aug 31, 2023 at 09:55:18 +0200, Gerd Möllmann wrote:
> >> Michael Heerdegen <michael_heerdegen@web.de> writes:

> >> > Alan Mackenzie <acm@muc.de> writes:

> >> >> (defmacro hash-if (condition then-form &rest else-forms)
> >> >>   "A conditional compilation macro analogous to C's #if.
> >> >> Evaluate CONDITION at macro-expansion time.  If it is non-nil,
> >> >> expand the macro to THEN-FORM.  Otherwise expand it to ELSE-FORMS
> >> >> enclosed in a `progn' form.  ELSE-FORMS may be empty."
> >> >>   (declare (indent 2)
> >> >>            (debug (form sexp &rest sexp)))
> >> >>   (if (eval condition lexical-binding)
> >> >>       then-form
> >> >>     (cons 'progn else-forms)))

> >> > Dunno if someone is able to fix this (I'm not).  Until then using
> >> > `def-form` `or `sexp` instead of `form` works in a better way (the
> >> > former edebugs CONDITION when instrumenting, the latter would omit
> >> > edebugging the CONDITION entirely).

> >> > Anyway, the key point in the above example is that macroexpanding (while
> >> > instrumenting) combined with the `eval' call seems to lead to the
> >> > evaluation of instrumented code outside of an Edebug session when
> >> > CONDITION is instrumented using `form`.  `eval-when-compile' uses
> >> > `def-form` for example - I guess using `form` in this case doesn't work
> >> > as one might expect.

> >> I think what's happening here is like this:

> >> By using 'form' for condition, we're telling edebug to instruments it.
> >> That is, the argument eval sees when foo is instrumented is whatever
> >> edebug wraps around the condition (< ...), and that contains the
> >> eval-after.  Using sexp for the condition doesn't instrument the condition.

> > Or, put a different way, edebug has instrumented CONDITION, then tries to
> > evaluate this.  This fails because there is no call to
> > edebug-make-enter-wrapper around the thing, which would defalias
> > edebug-after and edebug-before, and set up several lists that edebug
> > needs.

> I think that's correct, but I wouldn't say Edebug evaluates CONDITION,
> but we probably mean the same thing: CONDITION is instrumented and
> HASH-IF then gets that as argument when FOO is macroexpanded.  Then the
> execution of HASH-IF tries to evaluate the instrumented condition etc.

I think I've got it, now.  Considering that hash-if evaluates CONDITION,
it is an error for edebug to evaluate CONDITION as an argument first;
there would be a sort of "double evaluation".  So I need to use sexp
rather than form in the edebug spec, like Michael and you have been
telling me all along.  :-)

> >> One can follow that in the backtrace.

> >> So, I guess there's nothing to fix here.

> > I don't think I agree.  eval (and probably apply and funcall and its
> > variants) should somehow generate an "optional" edebug-make-enter-wrapper
> > around them.  This is currently not done.

> That would be one way.  On the other hand, the instrumentation of
> CONDITION is actually kind of pointless, because nothing will be
> left of it in the fully macroexpanded FOO.  So, one cannot step through
> CONDITION with Edebug anyway.

Yes.  This is a shame, since it would be nice to step through CONDITION,
particularly if it is complicated.

I think a better way of handling this would be to have a "base function"
for edebug-after (and for edebug-before), as opposed to the nil that each
of these currently has.  These functions would throw an error asking the
user to check the edebug spec.  Something like (untested):

    (defun edebug-after (before-index after-index form)
      "Version of `edebug-after' to call when edebug is not yet set up.
    This function gets temporarily replaced by a real function when
    edebug becomes active."
      (error "Invalid call to `edebug-after' for %S: Is your debug spec \
    correct?" form))

..  What do you think?

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-01  9:23         ` Alan Mackenzie
@ 2023-09-01 12:27           ` Gerd Möllmann
  2023-09-01 21:27             ` Alan Mackenzie
  0 siblings, 1 reply; 14+ messages in thread
From: Gerd Möllmann @ 2023-09-01 12:27 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Michael Heerdegen, 65620

On 01.09.23 11:23, Alan Mackenzie wrote:

> I think a better way of handling this would be to have a "base function"
> for edebug-after (and for edebug-before), as opposed to the nil that each
> of these currently has.  These functions would throw an error asking the
> user to check the edebug spec.  Something like (untested):
> 
>      (defun edebug-after (before-index after-index form)
>        "Version of `edebug-after' to call when edebug is not yet set up.
>      This function gets temporarily replaced by a real function when
>      edebug becomes active."
>        (error "Invalid call to `edebug-after' for %S: Is your debug spec \
>      correct?" form))
> 
> ..  What do you think?

I find that an excellent idea!  The error "void function edebug-after" 
might indeed be considered a bit unhelpful by some :-).  Haven't tested 
anything either, though...





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-01 12:27           ` Gerd Möllmann
@ 2023-09-01 21:27             ` Alan Mackenzie
  2023-09-02  4:27               ` Gerd Möllmann
  2023-09-03  4:29               ` Michael Heerdegen
  0 siblings, 2 replies; 14+ messages in thread
From: Alan Mackenzie @ 2023-09-01 21:27 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Michael Heerdegen, acm, 65620

Hello, Gerd.

On Fri, Sep 01, 2023 at 14:27:42 +0200, Gerd Möllmann wrote:
> On 01.09.23 11:23, Alan Mackenzie wrote:

> > I think a better way of handling this would be to have a "base function"
> > for edebug-after (and for edebug-before), as opposed to the nil that each
> > of these currently has.  These functions would throw an error asking the
> > user to check the edebug spec.  Something like (untested):
> > 
> >      (defun edebug-after (before-index after-index form)
> >        "Version of `edebug-after' to call when edebug is not yet set up.
> >      This function gets temporarily replaced by a real function when
> >      edebug becomes active."
> >        (error "Invalid call to `edebug-after' for %S: Is your debug spec \
> >      correct?" form))
> > 
> > ..  What do you think?

> I find that an excellent idea!  The error "void function edebug-after" 
> might indeed be considered a bit unhelpful by some :-).  Haven't tested 
> anything either, though...

Here's a working patch with a slight improvement: the error message
identifies the macro suspected of having an erroneous edebug spec.




diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 9a06807bcdc..a6153d599ac 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -2469,12 +2469,50 @@ edebug-run-fast
   (setf (cdr (assq 'edebug edebug-behavior-alist))
         '(edebug-default-enter edebug-fast-before edebug-fast-after)))
 
-(defalias 'edebug-before nil
+;; The following versions of `edebug-before' and `edebug-after' exist
+;; to handle the error which occurs if either of them gets called
+;; without an enclosing `edebug-enter'.  This can happen, for example,
+;; when a macro mistakenly has a `form' element in its edebug spec,
+;; and it additionally, at macro-expansion time, calls `eval',
+;; `apply', or `funcall' (etc.) on the corresponding argument.  This
+;; is intended to fix bug#65620.
+
+(defun edebug-b/a-error (func)
+  "Throw an error for an invalid call of FUNC.
+FUNC is expected to be `edebug-before' or `edebug-after'."
+  (let (this-macro
+        (n 0)
+        bt-frame)
+    (while (and (setq bt-frame (backtrace-frame n))
+                (not (and (car bt-frame)
+                          (eq (cadr bt-frame) 'macroexpand-1))))
+      (setq n (1+ n)))
+    (when bt-frame
+      (setq this-macro (caaddr bt-frame)))
+
+    (error
+     (concat "Invalid call to `" (symbol-name func) "'"
+             (if this-macro
+                 (concat ".  Is the edebug spec for `"
+                         (symbol-name this-macro)
+                         "' correct?")
+               "")))))
+
+(defun edebug-before (_before-index)
   "Function called by Edebug before a form is evaluated.
-See `edebug-behavior-alist' for implementations.")
-(defalias 'edebug-after nil
+See `edebug-behavior-alist' for other implementations.  This
+version of `edebug-before' gets called when edebug is not yet set
+up.  `edebug-enter' binds the function cell to a real function
+when edebug becomes active."
+  (edebug-b/a-error 'edebug-before))
+
+(defun edebug-after (_before-index _after-index _form)
   "Function called by Edebug after a form is evaluated.
-See `edebug-behavior-alist' for implementations.")
+See `edebug-behavior-alist' for other implementations.  This
+version of `edebug-after' gets called when edebug is not yet set
+up.  `edebug-enter' binds the function cell to a real function
+when edebug becomes active."
+  (edebug-b/a-error 'edebug-after))
 
 (defun edebug--update-coverage (after-index value)
   (let ((old-result (aref edebug-coverage after-index)))


-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-01 21:27             ` Alan Mackenzie
@ 2023-09-02  4:27               ` Gerd Möllmann
  2023-09-02 13:10                 ` Alan Mackenzie
  2023-09-03  4:29               ` Michael Heerdegen
  1 sibling, 1 reply; 14+ messages in thread
From: Gerd Möllmann @ 2023-09-02  4:27 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Michael Heerdegen, 65620

Alan Mackenzie <acm@muc.de> writes:

> Here's a working patch with a slight improvement: the error message
> identifies the macro suspected of having an erroneous edebug spec.

Maybe we could also add to the comment for edebug-before that basically
any of the instrumented form in the context you describe can lead to
errors?

I believe, if IFORM is such an instrumented form, something like

   (let ((x IFORM))
     ...)

in some macro will also error. 

Otherwise, LGTM.  Thanks for doing this!





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-02  4:27               ` Gerd Möllmann
@ 2023-09-02 13:10                 ` Alan Mackenzie
  2023-09-02 13:15                   ` Gerd Möllmann
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Mackenzie @ 2023-09-02 13:10 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Michael Heerdegen, 65620

Hello again, Gerd.

On Sat, Sep 02, 2023 at 06:27:32 +0200, Gerd Möllmann wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Here's a working patch with a slight improvement: the error message
> > identifies the macro suspected of having an erroneous edebug spec.

> Maybe we could also add to the comment for edebug-before that basically
> any of the instrumented form in the context you describe can lead to
> errors?

> I believe, if IFORM is such an instrumented form, something like

>    (let ((x IFORM))
>      ...)

> in some macro will also error. 

I've not been able to produce an error at macro-exansion time with a
form like that.  So I haven't amended that comment, yet.  However,
edebugging through a function which invoked such a macro can produce
errors.  This is all caused by having a `form' element in the edebug
spec where there should be `sexp'.

To try and ameliorate this, I propose adding a sentence to the
description of `sexp' in doc/lispref/edebug.texi:



diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
index c5be3a40d2c..a64ebda6803 100644
--- a/doc/lispref/edebug.texi
+++ b/doc/lispref/edebug.texi
@@ -1289,6 +1289,8 @@ Specification List
 @item sexp
 A single unevaluated Lisp object, which is not instrumented.
 @c an "expression" is not necessarily intended for evaluation.
+If the macro evaluates an argument at macro-expansion time, you should
+use @code{sexp} for it, not @code{form}.
 
 @item form
 A single evaluated expression, which is instrumented.  If your macro


> Otherwise, LGTM.  Thanks for doing this!

Thanks!  I'm seriously considering committing this soon.  ;-)

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-02 13:10                 ` Alan Mackenzie
@ 2023-09-02 13:15                   ` Gerd Möllmann
  2023-09-02 13:57                     ` Alan Mackenzie
  0 siblings, 1 reply; 14+ messages in thread
From: Gerd Möllmann @ 2023-09-02 13:15 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Michael Heerdegen, 65620

Alan Mackenzie <acm@muc.de> writes:

> Hello again, Gerd.
>
> On Sat, Sep 02, 2023 at 06:27:32 +0200, Gerd Möllmann wrote:
>> Alan Mackenzie <acm@muc.de> writes:
>
>> > Here's a working patch with a slight improvement: the error message
>> > identifies the macro suspected of having an erroneous edebug spec.
>
>> Maybe we could also add to the comment for edebug-before that basically
>> any of the instrumented form in the context you describe can lead to
>> errors?
>
>> I believe, if IFORM is such an instrumented form, something like
>
>>    (let ((x IFORM))
>>      ...)
>
>> in some macro will also error. 
>
> I've not been able to produce an error at macro-exansion time with a
> form like that.  

Ok.

> So I haven't amended that comment, yet.  However, edebugging through a
> function which invoked such a macro can produce errors.  This is all
> caused by having a `form' element in the edebug spec where there
> should be `sexp'.
>
> To try and ameliorate this, I propose adding a sentence to the
> description of `sexp' in doc/lispref/edebug.texi:
>
>
> diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
> index c5be3a40d2c..a64ebda6803 100644
> --- a/doc/lispref/edebug.texi
> +++ b/doc/lispref/edebug.texi
> @@ -1289,6 +1289,8 @@ Specification List
>  @item sexp
>  A single unevaluated Lisp object, which is not instrumented.
>  @c an "expression" is not necessarily intended for evaluation.
> +If the macro evaluates an argument at macro-expansion time, you should
> +use @code{sexp} for it, not @code{form}.
>  
>  @item form
>  A single evaluated expression, which is instrumented.  If your macro
>

Yes, that's helpful.






^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-02 13:15                   ` Gerd Möllmann
@ 2023-09-02 13:57                     ` Alan Mackenzie
  0 siblings, 0 replies; 14+ messages in thread
From: Alan Mackenzie @ 2023-09-02 13:57 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: Michael Heerdegen, acm, 65620-done

Hello, Gerd.

On Sat, Sep 02, 2023 at 15:15:55 +0200, Gerd Möllmann wrote:
> Alan Mackenzie <acm@muc.de> writes:

[ .... ]

> > .... However, edebugging through a function which invoked such a
> > macro can produce errors.  This is all caused by having a `form'
> > element in the edebug spec where there should be `sexp'.

> > To try and ameliorate this, I propose adding a sentence to the
> > description of `sexp' in doc/lispref/edebug.texi:


> > diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
> > index c5be3a40d2c..a64ebda6803 100644
> > --- a/doc/lispref/edebug.texi
> > +++ b/doc/lispref/edebug.texi
> > @@ -1289,6 +1289,8 @@ Specification List
> >  @item sexp
> >  A single unevaluated Lisp object, which is not instrumented.
> >  @c an "expression" is not necessarily intended for evaluation.
> > +If the macro evaluates an argument at macro-expansion time, you should
> > +use @code{sexp} for it, not @code{form}.

> >  @item form
> >  A single evaluated expression, which is instrumented.  If your macro


> Yes, that's helpful.

Thanks!  I've committed the patch to the two files, and I'm now closing
the bug.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#65620: void function edebug-after
  2023-09-01 21:27             ` Alan Mackenzie
  2023-09-02  4:27               ` Gerd Möllmann
@ 2023-09-03  4:29               ` Michael Heerdegen
  1 sibling, 0 replies; 14+ messages in thread
From: Michael Heerdegen @ 2023-09-03  4:29 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Gerd Möllmann, 65620

Hello Alan,

> -(defalias 'edebug-before nil
> +;; The following versions of `edebug-before' and `edebug-after' exist
> +;; to handle the error which occurs if either of them gets called
> +;; without an enclosing `edebug-enter'.  This can happen, for example,
> +;; when a macro mistakenly has a `form' element in its edebug spec,
> +;; and it additionally, at macro-expansion time, calls `eval',
> +;; `apply', or `funcall' (etc.)

I wonder whether what you say about `apply', or `funcall' is true: What
you can "call" in the macro expander is either a symbol or a function
_form_, i.e., a quoted lambda.  Maybe this quoted lambda is instrumented
by Edebug when forcing it in the debug spec, dunno, but specifying a
lambda expression in a macro call that is then called by the macro at
expansion time as quoted lambda makes no sense, one would rather make
the macro accept a form, or eval the function form in the expansion at
run-time.

So I'm not sure whether `apply' and `funcall' are really like `eval' in
this case.

Or - if the argument to funcall is a symbol - my question is what would
happen when macro expansion calls instrumented functions the normal way
(F . ARGS).  This works correctly, right?

Michael.





^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-09-03  4:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-30 12:57 bug#65620: void function edebug-after Alan Mackenzie
2023-08-30 23:09 ` Michael Heerdegen
2023-08-31  7:55   ` Gerd Möllmann
2023-08-31  8:02     ` Gerd Möllmann
2023-08-31 13:50     ` Alan Mackenzie
2023-08-31 14:41       ` Gerd Möllmann
2023-09-01  9:23         ` Alan Mackenzie
2023-09-01 12:27           ` Gerd Möllmann
2023-09-01 21:27             ` Alan Mackenzie
2023-09-02  4:27               ` Gerd Möllmann
2023-09-02 13:10                 ` Alan Mackenzie
2023-09-02 13:15                   ` Gerd Möllmann
2023-09-02 13:57                     ` Alan Mackenzie
2023-09-03  4:29               ` Michael Heerdegen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).