* bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler
@ 2020-12-01 3:17 Stefan Kangas
2020-12-01 5:20 ` Stefan Monnier
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Kangas @ 2020-12-01 3:17 UTC (permalink / raw)
To: 44980; +Cc: monnier
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
Severity: minor
Hi Stefan,
In 2015, you added this:
commit ad5a7c86d017ce8e9ff1312331ef09181be823bf
Author: Stefan Monnier <monnier@iro.umontreal.ca>
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?
[-- Attachment #2: 0001-Fix-test-for-failed-uses-of-lexical-vars-in-byte-com.patch --]
[-- Type: text/x-diff, Size: 8044 bytes --]
From 79fc4be79fd19d4272811b9792680bd5cf08b93b Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler
2020-12-01 3:17 bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler Stefan Kangas
@ 2020-12-01 5:20 ` Stefan Monnier
2020-12-01 12:38 ` Stefan Kangas
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Monnier @ 2020-12-01 5:20 UTC (permalink / raw)
To: Stefan Kangas; +Cc: 44980
> 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 <stefan@marxist.se>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler
2020-12-01 5:20 ` Stefan Monnier
@ 2020-12-01 12:38 ` Stefan Kangas
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Kangas @ 2020-12-01 12:38 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 44980
tags 44980 fixed
close 44980
thanks
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 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.
OK, thanks. Pushed to master as commit ace6eba036, and closing.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-01 12:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 3:17 bug#44980: [PATCH] Fix test for failed uses of lexical vars in byte-compiler Stefan Kangas
2020-12-01 5:20 ` Stefan Monnier
2020-12-01 12:38 ` Stefan Kangas
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).