bug#58563: 29.0.50; Generic functions and advertised-calling-convention

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Further to https://bugs.gnu.org/58531#25, generic functions do not
currently support advertised-calling-convention very well (or vice
versa).

For example, starting with:

  (cl-defgeneric my-foo (x &optional _y)
    "Frobnicate X."
    (declare (advertised-calling-convention (x) "29.1"))
    x)

Any code that calls my-foo with two arguments correctly gives rise to a
warning during byte-compilation.

C-h f also shows the expected arglist, but not for methods:

  my-foo is a Lisp closure.
  (my-foo X)
  Frobnicate X.
  This is a generic function.
  Implementations:
  (my-foo X &optional _Y)
  Undocumented

More importantly, if we now do:

  (cl-defmethod my-foo ((x symbol) &optional _y)
    "Frobnicate X the symbol."
    (declare (advertised-calling-convention (x) "29.1"))
    x)

Then my-foo's symbol-function is overwritten and its entry in
advertised-signature-table is no longer found, so byte-compilation no
longer warns about incorrect usage, and C-h f regresses to displaying:

  my-foo is a byte-compiled Lisp function.
  (my-foo X &optional Y)
  Frobnicate X.
  This is a generic function.
  Implementations:
  (my-foo (X symbol) &optional _Y)
  Frobnicate X the symbol.
  (my-foo X &optional _Y)
  Undocumented

Note that, unlike with cl-defgeneric, the declare form in cl-defmethod
does not expand to a call to set-advertised-calling-convention.  If
set-advertised-calling-convention is called after the cl-defmethod, then
the advertised-calling-convention is preserved (or rather reinstated),
but only until the next cl-defmethod is defined (which could happen in
third-party code).

I guess either advertised-signature-table should be extended to allow
for the nature of generic functions, or cl-defmethod should be taught to
preserve such function properties (or both).

I wonder if cl-defgeneric should be the single source of this function
property, or whether any cl-defmethod should be able to overload it.
Thoughts?

Thanks,

-- 
Basil

In GNU Emacs 29.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2022-10-16 built on tia
Repository revision: 07222447b6c9e75b713fe3b3954952fbb0e40c71
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Debian GNU/Linux bookworm/sid

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Lars Ingebrigtsen]

"Basil L. Contovounesios" via "Bug reports for GNU Emacs, the Swiss army
knife of text editors" <bug-gnu-emacs@gnu.org> writes:

> I wonder if cl-defgeneric should be the single source of this function
> property, or whether any cl-defmethod should be able to overload it.
> Thoughts?

I think...  that this belongs in the cl-defgeneric only, and not in the
cl-defmethod.  (Unless there isn't a cl-defgeneric, which is sometimes
the case.)

That is, if a cl-defgeneric has defined a advertised-calling-convention,
it should be an error for the cl-defmethod to define one.

Perhaps.

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Basil L. Contovounesios [2022-10-16 13:24 +0300] wrote:

> Further to https://bugs.gnu.org/58531#25, generic functions do not
> currently support advertised-calling-convention very well (or vice
> versa).
>
> For example, starting with:
>
>   (cl-defgeneric my-foo (x &optional _y)
>     "Frobnicate X."
>     (declare (advertised-calling-convention (x) "29.1"))
>     x)
>
> Any code that calls my-foo with two arguments correctly gives rise to a
> warning during byte-compilation.

Except inside ert-deftest, actually.  I wonder why?

-- 
Basil

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

> I think...  that this belongs in the cl-defgeneric only, and not in the
> cl-defmethod.

Agreed.

> (Unless there isn't a cl-defgeneric, which is sometimes the case.)

I wouldn't bother special casing this.
I think it's OK to say that if you want an
`advertised-calling-convention` you need to add a `cl-defgeneric`.
This likely applies to most other `declare`ations, actually.


        Stefan

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Lars Ingebrigtsen]

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

> I wouldn't bother special casing this.
> I think it's OK to say that if you want an
> `advertised-calling-convention` you need to add a `cl-defgeneric`.
> This likely applies to most other `declare`ations, actually.

Yes, good point.

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> I wouldn't bother special casing this.
>> I think it's OK to say that if you want an
>> `advertised-calling-convention` you need to add a `cl-defgeneric`.
>> This likely applies to most other `declare`ations, actually.
>
> Yes, good point.

On the other hand, thinking about this a bit more -- it's not an
uncommon thing to just use defmethod to without a defgeneric to just
ensure that you're calling the function with arguments of the right
type.  Making people add a fallback method (to error out on) would just
be pointless noise for them.

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

>>> I wouldn't bother special casing this.
>>> I think it's OK to say that if you want an
>>> `advertised-calling-convention` you need to add a `cl-defgeneric`.
>>> This likely applies to most other `declare`ations, actually.
>>
>> Yes, good point.
>
> On the other hand, thinking about this a bit more -- it's not an
> uncommon thing to just use defmethod to without a defgeneric to just
> ensure that you're calling the function with arguments of the right
> type.  Making people add a fallback method (to error out on) would just
> be pointless noise for them.

A defgeneric without a body is *not* a fallback method.
It's just a declaration, really.


        Stefan

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Lars Ingebrigtsen]

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

> A defgeneric without a body is *not* a fallback method.
> It's just a declaration, really.

Er, right.  Geez, it's been too long since I've written Common Lisp...

But still, having to use a defgeneric to do `declare's is a bit
cumbersome.

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

Lars Ingebrigtsen [2022-10-16 17:16:27] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> A defgeneric without a body is *not* a fallback method.
>> It's just a declaration, really.
> Er, right.  Geez, it's been too long since I've written Common Lisp...
> But still, having to use a defgeneric to do `declare's is a bit
> cumbersome.

The need for `declare` is infrequent.
The use of a single `cl-defmethod` is infrequent.

The intersection of the two should be small enough that I don't see
a good reason to care if it's slightly less convenient than ideal.

But I'm only arguing to defend my choice not to implement it: don't let
that stop you from doing the work :-)


        Stefan

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

> Further to https://bugs.gnu.org/58531#25, generic functions do not
> currently support advertised-calling-convention very well (or vice
> versa).
>
> For example, starting with:
>
>   (cl-defgeneric my-foo (x &optional _y)
>     "Frobnicate X."
>     (declare (advertised-calling-convention (x) "29.1"))
>     x)
>
> Any code that calls my-foo with two arguments correctly gives rise to a
> warning during byte-compilation.

> More importantly, if we now do:
>
>   (cl-defmethod my-foo ((x symbol) &optional _y)
>     "Frobnicate X the symbol."
>     (declare (advertised-calling-convention (x) "29.1"))
>     x)
>
> Then my-foo's symbol-function is overwritten and its entry in
> advertised-signature-table is no longer found, so byte-compilation no
> longer warns about incorrect usage, and C-h f regresses to displaying:

I believe this is now fixed in `master`.

> C-h f also shows the expected arglist, but not for methods:
>
>   my-foo is a Lisp closure.
>   (my-foo X)
>   Frobnicate X.
>   This is a generic function.
>   Implementations:
>   (my-foo X &optional _Y)
>   Undocumented

Not this, OTOH.

> I wonder if cl-defgeneric should be the single source of this function
> property, or whether any cl-defmethod should be able to overload it.
> Thoughts?

The `declare` form on `cl-defmethod`s has been silently ignored until
now.  So I added a compilation warning when we find such a `declare`.


        Stefan

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 3045 bytes --]

tags 58563 + patch
quit

Stefan Monnier [2022-10-16 12:05 -0400] wrote:

>> Then my-foo's symbol-function is overwritten and its entry in
>> advertised-signature-table is no longer found, so byte-compilation no
>> longer warns about incorrect usage, and C-h f regresses to displaying:
>
> I believe this is now fixed in `master`.
>
>> C-h f also shows the expected arglist, but not for methods:
>>
>>   my-foo is a Lisp closure.
>>   (my-foo X)
>>   Frobnicate X.
>>   This is a generic function.
>>   Implementations:
>>   (my-foo X &optional _Y)
>>   Undocumented
>
> Not this, OTOH.

Do we want this fixed?

Currently C-h f map-contains-key RET says:

  map-contains-key is a byte-compiled Lisp function in ‘map.el’.
  (map-contains-key MAP KEY)
  Return non-nil if and only if MAP contains KEY.
  TESTFN is deprecated.  Its default depends on MAP.
  The default implementation delegates to ‘map-some’.
    Probably introduced at or before Emacs version 27.1.
  This is a generic function.
  Implementations:
  (map-contains-key (MAP hash-table) KEY &optional TESTFN) in ‘map.el’.
  Return non-nil if MAP contains KEY, ignoring TESTFN.
  (map-contains-key (MAP array) KEY &optional TESTFN) in ‘map.el’.
  Return non-nil if KEY is an index of MAP, ignoring TESTFN.
  (map-contains-key (MAP list) KEY &optional TESTFN) in ‘map.el’.
  Return non-nil if MAP contains KEY.
  If MAP is an alist, TESTFN defaults to ‘equal’.
  If MAP is a plist, TESTFN defaults to ‘eq’.
  (map-contains-key MAP KEY &optional TESTFN) in ‘map.el’.
  Undocumented

So the generic docstring shows the advertised signature,
and the method docstrings show the actual signature.

Whereas with a patch like that following my signature, we'd have:

  map-contains-key is a byte-compiled Lisp function in ‘map.el’.
  (map-contains-key MAP KEY)
  Return non-nil if and only if MAP contains KEY.
  TESTFN is deprecated.  Its default depends on MAP.
  The default implementation delegates to ‘map-some’.
    Probably introduced at or before Emacs version 27.1.
  This is a generic function.
  Implementations:
  (map-contains-key (MAP hash-table) KEY) in ‘map.el’.
  Return non-nil if MAP contains KEY, ignoring TESTFN.
  (map-contains-key (MAP array) KEY) in ‘map.el’.
  Return non-nil if KEY is an index of MAP, ignoring TESTFN.
  (map-contains-key (MAP list) KEY) in ‘map.el’.
  Return non-nil if MAP contains KEY.
  If MAP is an alist, TESTFN defaults to ‘equal’.
  If MAP is a plist, TESTFN defaults to ‘eq’.
  (map-contains-key MAP KEY) in ‘map.el’.
  Undocumented

I don't know whether that's better or worse, with all the references to
the seemingly nonexistent TESTFN.  Then again, we could just update the
docstrings to mention it less.

WDYT?

P.S. There's a call to cl--generic-method-info also in elisp-mode.el.
     I wasn't sure whether the advertised signature is of use there,
     so I left it alone.

-- 
Basil


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: signature.diff --]
[-- Type: text/x-diff, Size: 3793 bytes --]

diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index 7b6d43e572..db1ad64874 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -484,18 +484,18 @@ cl-generic-define-context-rewriter
 (defun cl--generic-make-defmethod-docstring ()
   ;; FIXME: Copy&paste from pcase--make-docstring.
   (let* ((main (documentation (symbol-function 'cl-defmethod) 'raw))
-         (ud (help-split-fundoc main 'cl-defmethod)))
+         (ud (help-split-fundoc main 'cl-defmethod))
+         (generic (cl--generic 'cl-generic-generalizers)))
     ;; So that eg emacs -Q -l cl-lib --eval "(documentation 'pcase)" works,
     ;; where cl-lib is anything using pcase-defmacro.
     (require 'help-fns)
     (with-temp-buffer
       (insert (or (cdr ud) main))
       (insert "\n\n\tCurrently supported forms for TYPE:\n\n")
-      (dolist (method (reverse (cl--generic-method-table
-                                (cl--generic 'cl-generic-generalizers))))
-        (let* ((info (cl--generic-method-info method)))
+      (dolist (method (reverse (cl--generic-method-table generic)))
+        (let ((info (cl--generic-method-info method generic)))
           (when (nth 2 info)
-          (insert (nth 2 info) "\n\n"))))
+            (insert (nth 2 info) "\n\n"))))
       (let ((combined-doc (buffer-string)))
         (if ud (help-add-fundoc-usage combined-doc (car ud)) combined-doc)))))
 
@@ -1096,15 +1096,19 @@ cl--generic-find-defgeneric-regexp
   (add-to-list 'find-function-regexp-alist
                '(cl-defgeneric . cl--generic-find-defgeneric-regexp)))
 
-(defun cl--generic-method-info (method)
+(defun cl--generic-method-info (method &optional generic)
   (let* ((specializers (cl--generic-method-specializers method))
          (qualifiers   (cl--generic-method-qualifiers method))
          (call-con     (cl--generic-method-call-con method))
          (function     (cl--generic-method-function method))
-         (args (help-function-arglist (if (not (eq call-con 'curried))
-                                          function
-                                        (funcall function #'ignore))
-                                      'names))
+         (signature    (and generic
+                            (get-advertised-calling-convention
+                             (symbol-function (cl--generic-name generic)))))
+         (args (if (and generic (listp signature)) signature
+                 (help-function-arglist (if (not (eq call-con 'curried))
+                                            function
+                                          (funcall function #'ignore))
+                                        'names)))
          (docstring (documentation function))
          (qual-string
           (if (null qualifiers) ""
@@ -1154,8 +1158,8 @@ cl--generic-describe
         (insert (propertize "Implementations:\n\n" 'face 'bold))
         ;; Loop over fanciful generics
         (dolist (method (cl--generic-method-table generic))
-          (pcase-let*
-              ((`(,qualifiers ,args ,doc) (cl--generic-method-info method)))
+          (pcase-let ((`(,qualifiers ,args ,doc)
+                       (cl--generic-method-info method generic)))
             ;; FIXME: Add hyperlinks for the types as well.
             (let ((print-quoted nil)
                   (quals (if (length> qualifiers 0)
@@ -1227,7 +1231,7 @@ cl--generic-method-documentation
       (dolist (method (cl--generic-method-table generic))
         (when (cl--generic-specializers-apply-to-type-p
                (cl--generic-method-specializers method) type)
-          (push (cl--generic-method-info method) docs))))
+          (push (cl--generic-method-info method generic) docs))))
     docs))
 
 (defun cl--generic-method-files (method)

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

>> Not this, OTOH.
> Do we want this fixed?

Maybe.

> Currently C-h f map-contains-key RET says:
>
>   map-contains-key is a byte-compiled Lisp function in ‘map.el’.
>   (map-contains-key MAP KEY)
>   Return non-nil if and only if MAP contains KEY.
>   TESTFN is deprecated.  Its default depends on MAP.
>   The default implementation delegates to ‘map-some’.
>     Probably introduced at or before Emacs version 27.1.
>   This is a generic function.
>   Implementations:
>   (map-contains-key (MAP hash-table) KEY &optional TESTFN) in ‘map.el’.
>   Return non-nil if MAP contains KEY, ignoring TESTFN.
>   (map-contains-key (MAP array) KEY &optional TESTFN) in ‘map.el’.
>   Return non-nil if KEY is an index of MAP, ignoring TESTFN.
>   (map-contains-key (MAP list) KEY &optional TESTFN) in ‘map.el’.
>   Return non-nil if MAP contains KEY.
>   If MAP is an alist, TESTFN defaults to ‘equal’.
>   If MAP is a plist, TESTFN defaults to ‘eq’.
>   (map-contains-key MAP KEY &optional TESTFN) in ‘map.el’.
>   Undocumented

The current code also allows things like a (A B Rest C) in the
defgeneric and then more precise args in the methods,

> I don't know whether that's better or worse, with all the references to
> the seemingly nonexistent TESTFN.

Exactly.  It's not clear what's "right", so I think it's better to err
on the safer(?) side and apply the `advertised-calling-convention` to
the place where it's set.

`advertised-calling-convention` is not used very often (and that's
good), so I don't think it's super important to do the very best we can
with it.  We currently cover byte-compiler warnings when too many args
are passed, the arglist in Eldoc, and the top-line arglist in `C-h f`.
I think it's pretty good already.


        Stefan

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Monnier [2022-10-24 15:50 -0400] wrote:

> Exactly.  It's not clear what's "right", so I think it's better to err
> on the safer(?) side and apply the `advertised-calling-convention` to
> the place where it's set.
>
> `advertised-calling-convention` is not used very often (and that's
> good), so I don't think it's super important to do the very best we can
> with it.  We currently cover byte-compiler warnings when too many args
> are passed, the arglist in Eldoc, and the top-line arglist in `C-h f`.
> I think it's pretty good already.

Agreed.  So is there something left to be done here, or can this bug be
closed?

Thanks,

-- 
Basil

bug#58563: 29.0.50; Generic functions and advertised-calling-convention

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

Basil L. Contovounesios [2022-10-26 16:51:22] wrote:
> Agreed.  So is there something left to be done here, or can this bug be
> closed?

Let's see...


        Stefan