Subject: bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler

From: Stefan Monnier <[email-masked]>
Date: Tue, 01 Dec 2020 00:20:04 -0500

> However, the test uses `assq' to check `byte-compile-lexical-variables',
> but that is not an alist.  It seems to me that the correct test would
> use `memq'.
>
> Please see the attached patch that fixes this.  It also adds tests.
>
> WDYT?  Am I missing something?

I have a strong feeling of déjà vu, so I think you're not missing
anything and it's just an old bug which I thought I had fixed.

        Stefan

> From 79fc4be79fd19d4272811b9792680bd5cf08b93b Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <[email-masked]>
> Date: Tue, 1 Dec 2020 04:11:59 +0100
> Subject: [PATCH] Fix test for failed uses of lexical vars in byte-compiler
>
> * lisp/emacs-lisp/bytecomp.el (byte-compile-form): Fix test.
> * test/lisp/emacs-lisp/bytecomp-tests.el
> (bytecomp--define-warning-file-test): Don't prefix tests with
> 'warn'.
> (bytecomp/error-lexical-var-with-add-hook\.el)
> (bytecomp/error-lexical-var-with-remove-hook\.el)
> (bytecomp/error-lexical-var-with-run-hook-with-args-until-failure\.el)
> (bytecomp/error-lexical-var-with-run-hook-with-args-until-success\.el)
> (bytecomp/error-lexical-var-with-run-hook-with-args\.el)
> (bytecomp/error-lexical-var-with-symbol-value\.el): New tests.
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el:
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el:
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el:
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el:
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el:
> * test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el:
> New files.
> ---
>  lisp/emacs-lisp/bytecomp.el                   |  2 +-
>  .../error-lexical-var-with-add-hook.el        |  4 +++
>  .../error-lexical-var-with-remove-hook.el     |  4 +++
>  ...r-with-run-hook-with-args-until-failure.el |  3 +++
>  ...r-with-run-hook-with-args-until-success.el |  3 +++
>  ...ror-lexical-var-with-run-hook-with-args.el |  3 +++
>  .../error-lexical-var-with-symbol-value.el    |  4 +++
>  test/lisp/emacs-lisp/bytecomp-tests.el        | 26 ++++++++++++++++---
>  8 files changed, 45 insertions(+), 4 deletions(-)
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el
>  create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el
>
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index a20f363456..879f08a09f 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -3203,7 +3203,7 @@ byte-compile-form
>                               run-hook-with-args-until-failure))
>            (pcase (cdr form)
>              (`(',var . ,_)
> -             (when (assq var byte-compile-lexical-variables)
> +             (when (memq var byte-compile-lexical-variables)
>                 (byte-compile-report-error
>                  (format-message "%s cannot use lexical var `%s'" fn var))))))
>          ;; Warn about using obsolete hooks.
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el
> new file mode 100644
> index 0000000000..5f390898e6
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-add-hook.el
> @@ -0,0 +1,4 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (add-hook 'foo #'next-line)
> +  foo)
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el
> new file mode 100644
> index 0000000000..eaa625eba1
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-remove-hook.el
> @@ -0,0 +1,4 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (remove-hook 'foo #'next-line)
> +  foo)
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el
> new file mode 100644
> index 0000000000..7a116ad464
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-failure.el
> @@ -0,0 +1,3 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (run-hook-with-args-until-failure 'foo))
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el
> new file mode 100644
> index 0000000000..96d10a343d
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args-until-success.el
> @@ -0,0 +1,3 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (run-hook-with-args-until-success 'foo #'next-line))
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el
> new file mode 100644
> index 0000000000..bb9101bd07
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-run-hook-with-args.el
> @@ -0,0 +1,3 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (run-hook-with-args 'foo))
> diff --git a/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el
> new file mode 100644
> index 0000000000..5f390898e6
> --- /dev/null
> +++ b/test/lisp/emacs-lisp/bytecomp-resources/error-lexical-var-with-symbol-value.el
> @@ -0,0 +1,4 @@
> +;;; -*- lexical-binding: t; -*-
> +(let ((foo nil))
> +  (add-hook 'foo #'next-line)
> +  foo)
> diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
> index c9070c03b3..db4ff06860 100644
> --- a/test/lisp/emacs-lisp/bytecomp-tests.el
> +++ b/test/lisp/emacs-lisp/bytecomp-tests.el
> @@ -548,7 +548,7 @@ test-eager-load-macro-expansion-eval-and-compile
>    (should (equal (funcall 'def) -1)))
>  
>  (defmacro bytecomp--define-warning-file-test (file re-warning &optional reverse)
> -  `(ert-deftest ,(intern (format "bytecomp-warn/%s" file)) ()
> +  `(ert-deftest ,(intern (format "bytecomp/%s" file)) ()
>       :expected-result ,(if reverse :failed :passed)
>       (with-current-buffer (get-buffer-create "*Compile-Log*")
>         (let ((inhibit-read-only t)) (erase-buffer))
> @@ -556,9 +556,29 @@ bytecomp--define-warning-file-test
>         (ert-info ((buffer-string) :prefix "buffer: ")
>           (should (re-search-forward ,re-warning))))))
>  
> -(bytecomp--define-warning-file-test "warn-free-setq.el" "free.*foo")
> +(bytecomp--define-warning-file-test "error-lexical-var-with-add-hook.el"
> +                            "add-hook.*lexical var")
>  
> -(bytecomp--define-warning-file-test "warn-free-variable-reference.el" "free.*bar")
> +(bytecomp--define-warning-file-test "error-lexical-var-with-remove-hook.el"
> +                            "remove-hook.*lexical var")
> +
> +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args-until-failure.el"
> +                            "args-until-failure.*lexical var")
> +
> +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args-until-success.el"
> +                            "args-until-success.*lexical var")
> +
> +(bytecomp--define-warning-file-test "error-lexical-var-with-run-hook-with-args.el"
> +                            "args.*lexical var")
> +
> +(bytecomp--define-warning-file-test "error-lexical-var-with-symbol-value.el"
> +                            "symbol-value.*lexical var")
> +
> +(bytecomp--define-warning-file-test "warn-free-setq.el"
> +                            "free.*foo")
> +
> +(bytecomp--define-warning-file-test "warn-free-variable-reference.el"
> +                            "free.*bar")
>  
>  (ert-deftest test-eager-load-macro-expansion-eval-when-compile ()
>    ;; Make sure we interpret eval-when-compile forms properly.  CLISP