bug#15715: 24.3; Sanity check of ARGLIST when calling `defun'

bug#15715: 24.3; Sanity check of ARGLIST when calling `defun'

[Nicolas Richard]

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

Hello,

As suggested here http://permalink.gmane.org/gmane.emacs.help/94196 I
attach a patch to make (defun ...) check that the arglist is an actual
list of symbols when defining a function, and otherwise signal an error.

Without this, code like (defun test-foobar ...) succeeds although the
function is doomed to fail at runtime.

This won't catch every missing "arglist" but that looks impossible
anyway.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Sanity check of ARGLIST when calling `defun' --]
[-- Type: text/x-diff, Size: 959 bytes --]

diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index 0bb0495..d671892 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -180,6 +180,10 @@ The return value is undefined.
   ;; from
   ;;    (defun foo (arg) (toto)).
   (declare (doc-string 3))
+  (if (null
+       (and (listp arglist)
+            (null (delq t (mapcar #'symbolp arglist)))))
+      (error "Malformed arglist: %s" arglist))
   (let ((decls (cond
                 ((eq (car-safe docstring) 'declare)
                  (prog1 (cdr docstring) (setq docstring nil)))
@@ -209,7 +213,7 @@ The return value is undefined.
                     nil)
                    (t (message "Warning: Unknown defun property `%S' in %S"
                                (car x) name)))))
-                   decls))
+            decls))
           (def (list 'defalias
                      (list 'quote name)
                      (list 'function
-- 
1.8.1.5


[-- Attachment #3: Type: text/plain, Size: 563 bytes --]


It was also suggested in that thread by P.J. Bourguignon to use
alternate methods, but both required parts of cl (for "every" and
"dolist"), hence I'm submitting this version (and checked that "make
bootstrap" succeeds).

Another version would be something along the lines of:
  (mapc (function (lambda (elt)
                    (if (null (symbolp elt))
                        (error "Malformed arglist: %s" arglist))))
        (if (listp arglist)
            arglist
          '("")))
but it doesn't look more efficient from my (very limited) test.

-- 
Nico.

bug#15715: 24.3; Sanity check of ARGLIST when calling `defun'

[Lars Ingebrigtsen]

Nicolas Richard <theonewiththeevillook@yahoo.fr> writes:

> As suggested here http://permalink.gmane.org/gmane.emacs.help/94196 I
> attach a patch to make (defun ...) check that the arglist is an actual
> list of symbols when defining a function, and otherwise signal an error.
>
> Without this, code like (defun test-foobar ...) succeeds although the
> function is doomed to fail at runtime.

I think this is a good idea (catching errors early is always nice), but
it might break a lot of code out there in the wild.  People defining
functions in their .emacs that they never actually call, for instance.
So I think this change sounds kinda dangerous.

What do all y'all think?

> This won't catch every missing "arglist" but that looks impossible
> anyway.
>
> diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
> index 0bb0495..d671892 100644
> --- a/lisp/emacs-lisp/byte-run.el
> +++ b/lisp/emacs-lisp/byte-run.el
> @@ -180,6 +180,10 @@ The return value is undefined.
>    ;; from
>    ;;    (defun foo (arg) (toto)).
>    (declare (doc-string 3))
> +  (if (null
> +       (and (listp arglist)
> +            (null (delq t (mapcar #'symbolp arglist)))))
> +      (error "Malformed arglist: %s" arglist))
>    (let ((decls (cond
>                  ((eq (car-safe docstring) 'declare)
>                   (prog1 (cdr docstring) (setq docstring nil)))
> @@ -209,7 +213,7 @@ The return value is undefined.
>                      nil)
>                     (t (message "Warning: Unknown defun property `%S' in %S"
>                                 (car x) name)))))
> -                   decls))
> +            decls))
>            (def (list 'defalias
>                       (list 'quote name)
>                       (list 'function

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

bug#15715: 24.3; Sanity check of ARGLIST when calling `defun'

[Glenn Morris]

Version: 26.1

Thanks; applied as 0107336.