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

From: Stefan Kangas <[email-masked]>
Date: Mon, 30 Nov 2020 21:17:41 -0600

Severity: minor

Hi Stefan,

In 2015, you added this:

commit ad5a7c86d017ce8e9ff1312331ef09181be823bf
Author: Stefan Monnier <[email-masked]>
Date:   Thu Feb 5 14:28:16 2015 -0500

[...]
    (byte-compile-form): Add warnings for failed uses of lexical vars via
    quoted symbols.

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?
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
-- 
2.29.2