bug#56596: 29.0.50; void-variable cl--nm

bug#56596: 29.0.50; void-variable cl--nm

[Pierre L. Nageoire]

Hi

Caused by following code

(require 'eieio)

(defclass raw-mother () ())

(cl-defmethod I ((m raw-mother))
  (format "the mother"))

(cl-defmethod hello ((m raw-mother))
  (message "I am %s" (I m)))

(hello (raw-mother))

(defclass raw-daughter (raw-mother) ())

(cl-defmethod I ((d raw-daughter))
  (format "the daughter of %s"
	  (cl-call-next-method)))


(hello (raw-daughter))



But only since last update from git repos. With an emacs version updated
in march approximately no problem.


Regards 

bug#56596: 29.0.50; void-variable cl--nm

[Lars Ingebrigtsen]

"Pierre L. Nageoire" <devel@pollock-nageoire.net> writes:

> (hello (raw-daughter))
>
> But only since last update from git repos. With an emacs version updated
> in march approximately no problem.

I can reproduce this -- but only when using dynamic binding.  When using
lexical binding, things work fine.

I've added Stefan to the CCs; perhaps he has some comments.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#56596: 29.0.50; void-variable cl--nm

[Pierre L. Nageoire]

Hi,

Thank you for your answer 

Lars Ingebrigtsen <larsi@gnus.org> writes:

> "Pierre L. Nageoire" <devel@pollock-nageoire.net> writes:
>
>> (hello (raw-daughter))
>>
>> But only since last update from git repos. With an emacs version updated
>> in march approximately no problem.
>
> I can reproduce this -- but only when using dynamic binding.  When using
> lexical binding, things work fine.
  This is certainly the key point !


>
> I've added Stefan to the CCs; perhaps he has some comments.


  I suspect that cl-generic implementation has recently changed and that
  it should not be used nowadays exactly like it has been. Any small
  explanations from Stefan would be greatly appreciated !

  Regards 

bug#56596: 29.0.50; void-variable cl--nm

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

>> I can reproduce this -- but only when using dynamic binding.  When using
>> lexical binding, things work fine.

Indeed, `cl-defmethod` (and `cl-defgeneric`) aren't guaranteed to work in
dynbind code.  They often do, admittedly.

>> I've added Stefan to the CCs; perhaps he has some comments.
>
>   I suspect that cl-generic implementation has recently changed and that
>   it should not be used nowadays exactly like it has been. Any small
>   explanations from Stefan would be greatly appreciated !

The "next method" is not known when we compile `cl-defmethod` but it is
known when we build the "effective method", which is expected to be done
much less often than actual calls to that method.

The old code for `cl-defmethod` turned

    (cl-defmethod I ((d raw-daughter))
      (format "the daughter of %s"
              (cl-call-next-method)))

into something like

    (cl-..register (raw-daughter)
      (lambda (cnm d)
        (format "the daughter of %s"
                (apply cnm))))

forcing the caller to build a `cnm` closure which captures the args list
containing the value of the `d` argument.  Also it made it difficult to
implement `next-method-p` since that requires digging into this `cnm`
closure to see if it's one of those that would signal no-next-method.

The new code instead is a bit like:

    (cl-..register (raw-daughter)
      (lambda (nm)
        (lambda (&rest args)
          (let ((cnm (lambda (&rest cnmargs) (apply nm (or cnmargs args)))))
            (destructive-bind (d) args
              (format "the daughter of %s"
                      (apply cnm)))))))

This basically moves some of the code from the caller to here, which
doesn't seem to buy us very much but:
- it makes it much easier to implement `next-method-p` because now we
  have access to the actual "next method", rather than to a closure that
  combines the next methods with the saved arg list, so it's much easier
  to tell if the next method is the one that signals no-next-method.
- we occasionally get to skip building the `cnm` closure because we can
  use λ-lifting instead (basically the byte-compiler gets to see both
  parts of the code together and can thus change it: with this new code
  we could actually improve the code generated by the byte-compiler
  even further).

Problem is that the new code relies on the use of currying (see how
`nm` and `args` are now passed in a curried fashion), which is only
possible with lexical scoping.


        Stefan

bug#56596: 29.0.50; void-variable cl--nm

[Pierre L. Nageoire]

Hi Stefan,

Thanks for these detailed explanations; I think I will be able to
modify my codes to make them work with this new cl-generic
implementation.

Best regards 

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> I can reproduce this -- but only when using dynamic binding.  When using
>>> lexical binding, things work fine.
>
> Indeed, `cl-defmethod` (and `cl-defgeneric`) aren't guaranteed to work in
> dynbind code.  They often do, admittedly.
>
>>> I've added Stefan to the CCs; perhaps he has some comments.
>>
>>   I suspect that cl-generic implementation has recently changed and that
>>   it should not be used nowadays exactly like it has been. Any small
>>   explanations from Stefan would be greatly appreciated !
>
> The "next method" is not known when we compile `cl-defmethod` but it is
> known when we build the "effective method", which is expected to be done
> much less often than actual calls to that method.
>
> The old code for `cl-defmethod` turned
>
>     (cl-defmethod I ((d raw-daughter))
>       (format "the daughter of %s"
>               (cl-call-next-method)))
>
> into something like
>
>     (cl-..register (raw-daughter)
>       (lambda (cnm d)
>         (format "the daughter of %s"
>                 (apply cnm))))
>
> forcing the caller to build a `cnm` closure which captures the args list
> containing the value of the `d` argument.  Also it made it difficult to
> implement `next-method-p` since that requires digging into this `cnm`
> closure to see if it's one of those that would signal no-next-method.
>
> The new code instead is a bit like:
>
>     (cl-..register (raw-daughter)
>       (lambda (nm)
>         (lambda (&rest args)
>           (let ((cnm (lambda (&rest cnmargs) (apply nm (or cnmargs args)))))
>             (destructive-bind (d) args
>               (format "the daughter of %s"
>                       (apply cnm)))))))
>
> This basically moves some of the code from the caller to here, which
> doesn't seem to buy us very much but:
> - it makes it much easier to implement `next-method-p` because now we
>   have access to the actual "next method", rather than to a closure that
>   combines the next methods with the saved arg list, so it's much easier
>   to tell if the next method is the one that signals no-next-method.
> - we occasionally get to skip building the `cnm` closure because we can
>   use λ-lifting instead (basically the byte-compiler gets to see both
>   parts of the code together and can thus change it: with this new code
>   we could actually improve the code generated by the byte-compiler
>   even further).
>
> Problem is that the new code relies on the use of currying (see how
> `nm` and `args` are now passed in a curried fashion), which is only
> possible with lexical scoping.
>
>
>         Stefan

bug#56596: 29.0.50; void-variable cl--nm

[Stefan Kangas]

"Pierre L. Nageoire" <devel@pollock-nageoire.net> writes:

> Caused by following code
>
> (require 'eieio)
>
> (defclass raw-mother () ())
>
> (cl-defmethod I ((m raw-mother))
>   (format "the mother"))
>
> (cl-defmethod hello ((m raw-mother))
>   (message "I am %s" (I m)))
>
> (hello (raw-mother))
>
> (defclass raw-daughter (raw-mother) ())
>
> (cl-defmethod I ((d raw-daughter))
>   (format "the daughter of %s"
> 	  (cl-call-next-method)))
>
>
> (hello (raw-daughter))
>
>
>
> But only since last update from git repos. With an emacs version updated
> in march approximately no problem.

6f973faa912a5ac1ba643c6f5deb0c02baa0ba6d is the first bad commit
commit 6f973faa912a5ac1ba643c6f5deb0c02baa0ba6d
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date:   Wed Mar 30 13:54:56 2022 -0400

    cl-generic: Use OClosures for `cl--generic-isnot-nnm-p`

    Rewrite the handling of `cl-no-next-method` to get rid of the hideous
    hack used in `cl--generic-isnot-nnm-p` and also to try and move
    some of the cost to the construction of the effective method rather
    than its invocation.  This speeds up method calls measurably when
    there's a `cl-call-next-method` in the body.