Subject: Re: bug#65797: `buffer-match-p` should not use `func-arity`

From: Joseph Turner <[email-masked]>
Date: Wed, 11 Oct 2023 21:53:31 -0700

Stefan Monnier <[email-masked]> writes:

>> I am in favor of this solution as well.
>
> Then how 'bout something like the patch below which changes the
> `&optional` into an `&rest` but tries to preserve compatibility with the
> old calling convention.
>
>
>         Stefan
>
>
> diff --git a/lisp/subr.el b/lisp/subr.el
> index e88815fa58c..06c9923b525 100644
> --- a/lisp/subr.el
> +++ b/lisp/subr.el
> @@ -7295,13 +7292,13 @@ string-lines
>              (setq start (length string)))))
>        (nreverse lines))))
>
> -(defun buffer-match-p (condition buffer-or-name &optional arg)
> +(defun buffer-match-p (condition buffer-or-name &rest args)
>    "Return non-nil if BUFFER-OR-NAME matches CONDITION.
>  CONDITION is either:
>  - the symbol t, to always match,
>  - the symbol nil, which never matches,
>  - a regular expression, to match a buffer name,
> -- a predicate function that takes BUFFER-OR-NAME and ARG as
> +- a predicate function that takes BUFFER-OR-NAME plus ARGS as
>    arguments, and returns non-nil if the buffer matches,
>  - a cons-cell, where the car describes how to interpret the cdr.
>    The car can be one of the following:
> @@ -7326,9 +7323,18 @@ buffer-match-p
>                        ((pred stringp)
>                         (string-match-p condition (buffer-name buffer)))
>                        ((pred functionp)
> -                       (if (eq 1 (cdr (func-arity condition)))
> -                           (funcall condition buffer-or-name)
> -                         (funcall condition buffer-or-name arg)))
> +                       (if (cdr args)
> +                           ;; More than 1 argument: no need for
> +                           ;; Emacs-29 backward compatibility!
> +                           (apply condition buffer-or-name args)
> +                         (condition-case err
> +                             (apply condition buffer-or-name args)
> +                           (wrong-number-of-arguments
> +                            ;; Backward compatibility with Emacs-29 semantics.
> +                            (message "Trying obsolete calling convention for: %S"
> +                                     Condition)
> +                            (apply condition buffer-or-name
> +                                   (if args '(nil) nil))))))
>                        (`(major-mode . ,mode)
>                         (eq
>                          (buffer-local-value 'major-mode buffer)
> @@ -7350,17 +7356,17 @@ buffer-match-p
>                  (throw 'match t)))))))
>      (funcall match (list condition))))

At first, I was concerned that the condition-case would mess up error
handling in user code, but all 9 permutations give the expected results:

(let ((condition
       (lambda (buf) t)
       ;; (lambda (buf arg1) t)
       ;; (lambda (buf arg1 arg2) t)
       ))
  (condition-case err
      (buffer-match-p condition (current-buffer)
                      ;; 'arg1
                      ;; 'arg2
                      )
    (wrong-number-of-arguments
     "Caught")))

> -(defun match-buffers (condition &optional buffers arg)
> +(defun match-buffers (condition &optional buffers &rest args)
>    "Return a list of buffers that match CONDITION, or nil if none match.
>  See `buffer-match-p' for various supported CONDITIONs.
>  By default all buffers are checked, but the optional
>  argument BUFFERS can restrict that: its value should be
>  an explicit list of buffers to check.
> -Optional argument ARG is passed to `buffer-match-p', for
> +Optional arguments ARGS are passed to `buffer-match-p', for
>  predicate conditions in CONDITION."
>    (let (bufs)
>      (dolist (buf (or buffers (buffer-list)))
> -      (when (buffer-match-p condition (get-buffer buf) arg)
> +      (when (apply #'buffer-match-p condition (get-buffer buf) args)
>          (push buf bufs)))
>      bufs))
>