bug#6376: 23.2; byte compile add-to-list report free variable

[Stefan Kangas]

Kevin Ryde <user42@zip.com.au> writes:

> I got a bit further to these few lines.  Not quite ready yet, but
> getting closer.  The byte-compile-check-var is more or less a break-out
> of the checks in byte-compile-variable-ref.
>
> add-hook and remove-hook end up with checks against non-variables, but
> they don't demand a bound variable.  I suppose they could helpfully also
> notice undefined functions in their second arg too.  You'd be tempted to
> have some sort of general arg-type description for the builtin funcs
> rather than doing checks plus "normal-call" for each.

(That was 10 years ago.)

This change looks useful to me.  Any chance you could fix it up and send
it as a patch?

> (require 'bytecomp)
>
> (defun byte-compile-check-var (symbol base-op)
>   ;; SYMBOL is the name of a variable.  BASE-OP is one of the following
>   ;; according to what's being done to the variable.
>   ;;    'byte-varref
>   ;;    'byte-varset
>   ;;    'byte-varbind
>   ;;    'varset-def    -- set, defining if not already (eg. add-hook)
>   ;; Emit a warning if SYMBOL is unbound, or if it's a constant being set or
>   ;; bound.
>   ;;
>   (cond ((and (not (symbolp symbol))
>               ;; `constants' warning includes all non-variables
>               (byte-compile-warning-enabled-p 'constants))
>          (byte-compile-warn "%s non-variable `%S'"
>                             (assoc-default base-op
>                                            '((byte-varref . "reference to")
>                                              (byte-varset . "assignment to")
>                                              (varset-def  . "assignment to")
>                                              (byte-varbind . "let-bind of"))
>                                            nil base-op)
>                             symbol))
>
>         ((and (not (eq base-op 'byte-varref))
>               (byte-compile-warning-enabled-p 'constants)
>               (byte-compile-const-symbol-p symbol t))
>          (byte-compile-warn "%s constant `%S'"
>                             (assoc-default
>                              base-op
>                              '((byte-varset . "variable assignment to")
>                                (varset-def  . "variable assignment to")
>                                (byte-varbind . "let-bind of"))
>                              nil base-op)
>                             symbol))
>
>         ((and (not (memq base-op '(varset-def byte-varbind)))
>               (byte-compile-warning-enabled-p 'free-vars)
>               (not (memq symbol byte-compile-bound-variables))
>               (not (memq symbol byte-compile-free-assignments)))
>          (byte-compile-warn "%s to free variable `%S'"
>                             (assoc-default base-op
>                                            '((byte-varref . "reference to")
>                                              (byte-varset . "assignment to"))
>                                            nil base-op)
>                             symbol)
>          (push symbol byte-compile-free-assignments))))
>
> (defun byte-compile-check-argvar (arg base-op)
>   ;; ARG is a function argument form which is supposed to evaluate to a
>   ;; symbol naming a variable.  If ARG is (quote FOO) or :foo then check
>   ;; that it's a bound variable per bytecomp-check-var.  If ARG is
>   ;; self-evaluating like nil, t, strings, etc then pass to the check too,
>   ;; to possibly report assignment to a constant.  Code like (quote nil) or
>   ;; (quote "foo") reaches this point as plain nil or t.
>   ;;
>   (cond ((eq 'quote (car-safe arg))
>          (byte-compile-check-var (car-safe (cdr arg)) base-op))
>         ((or (memq arg '(nil t))
>              ;; anything except a symbol or list is self-evaluating
>              (not (or (symbolp arg)
>                       (consp arg))))
>          (byte-compile-check-var arg base-op))))
>
> (defun byte-compile-addtolist (form)
>   ;; first arg is the name of a variable being changed, eg. (foo 'var ...)
>   (if (>= (safe-length form) 2)
>       (byte-compile-check-argvar (cadr form) 'byte-varset))
>   (byte-compile-normal-call form))
> (byte-defop-compiler-1 add-to-list         byte-compile-addtolist)
> (byte-defop-compiler-1 add-to-ordered-list byte-compile-addtolist)
>
> (defun byte-compile-addremhook (form)
>   ;; first arg is the name of a variable being changed, eg. (foo 'var ...)
>   ;; only
>   (if (>= (safe-length form) 2)
>       (byte-compile-check-argvar (cadr form) 'varset-def))
>   (byte-compile-normal-call form))
> (byte-defop-compiler-1 add-hook    byte-compile-addremhook)
> (byte-defop-compiler-1 remove-hook byte-compile-addremhook)