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" <bug-gnu-emacs@gnu.org>]

[-- 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)