GNU bug report logs - #44980
[PATCH] Fix test for failed uses of lexical vars in byte-compiler

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 44980 in the body.
You can then email your comments to 44980 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Fix test for failed uses of lexical vars in byte-compiler
Date: Mon, 30 Nov 2020 21:17:41 -0600

[Message part 1 (text/plain, inline)]

Severity: minor

Hi Stefan,

In 2015, you added this:

commit ad5a7c86d017ce8e9ff1312331ef09181be823bf
Author: Stefan Monnier <monnier <at> 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?

[0001-Fix-test-for-failed-uses-of-lexical-vars-in-byte-com.patch (text/x-diff, attachment)]

Message #8 received at 44980 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 44980 <at> debbugs.gnu.org
Subject: Re: bug#44980: [PATCH] Fix test for failed uses of lexical vars in
 byte-compiler
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 <stefan <at> 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

Message #11 received at 44980 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 44980 <at> debbugs.gnu.org
Subject: Re: bug#44980: [PATCH] Fix test for failed uses of lexical vars in
 byte-compiler
Date: Tue, 1 Dec 2020 06:38:01 -0600

tags 44980 fixed
close 44980
thanks

Stefan Monnier <monnier <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.