GNU bug report logs - #31641
26.1; iter-do variable not left unused warning

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 31641 in the body.
You can then email your comments to 31641 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: Christopher Wellons <wellons <at> nullprogram.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; iter-do variable not left unused warning
Date: Tue, 29 May 2018 09:12:30 -0400

When byte-compiling an iter-do form with a variable intended to be left 
unused, the compiler emits a false warning:

;;; -*- lexical-binding: t; -*-
(require 'generator)
(iter-do (_ i))
;; -> "Warning: variable ‘_’ not left unused"

Giving the variable a name has the opposite effect, though it's not 
a false warning in this case:

(iter-do (v i))
;; -> "Unused lexical variable ‘v’"

This issue does not occur with similar, long-established forms:

;;; -*- lexical-binding: t; -*-
(dolist (_ '(a b c)))
(dotimes (_ 10))
;; -> no warnings

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

From: Noam Postavsky <npostavs <at> gmail.com>
To: Christopher Wellons <wellons <at> nullprogram.com>
Cc: 31641 <at> debbugs.gnu.org
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Tue, 29 May 2018 18:13:50 -0400

tags 31641 + confirmed
severity 31641 minor
quit

Christopher Wellons <wellons <at> nullprogram.com> writes:

> When byte-compiling an iter-do form with a variable intended to be
> left unused, the compiler emits a false warning:
>
> ;;; -*- lexical-binding: t; -*-
> (require 'generator)
> (iter-do (_ i))
> ;; -> "Warning: variable ‘_’ not left unused"

Looking at the expansion, I guess the setf should be dropped if the
variable name starts with _.

(let (_
      #3=#:iter-do-result11
      (#1=#:iter-do-iterator-done8 nil)
      (#2=#:iter-do-iterator10 i))
  (while (not #1#)
    (condition-case #4=#:iter-do-condition9
        (setf _ (iter-next #2#))
      (iter-end-of-sequence
       (setf #3# (cdr #4#))
       (setf #1# t)))
    (unless #1#))
  #3#)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Christopher Wellons <wellons <at> nullprogram.com>,
 Daniel Colascione <dancol <at> dancol.org>, 31641 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Thu, 04 Feb 2021 11:05:24 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

>> When byte-compiling an iter-do form with a variable intended to be
>> left unused, the compiler emits a false warning:
>>
>> ;;; -*- lexical-binding: t; -*-
>> (require 'generator)
>> (iter-do (_ i))
>> ;; -> "Warning: variable ‘_’ not left unused"
>
> Looking at the expansion, I guess the setf should be dropped if the
> variable name starts with _.
>
> (let (_
>       #3=#:iter-do-result11
>       (#1=#:iter-do-iterator-done8 nil)
>       (#2=#:iter-do-iterator10 i))
>   (while (not #1#)
>     (condition-case #4=#:iter-do-condition9
>         (setf _ (iter-next #2#))
>       (iter-end-of-sequence
>        (setf #3# (cdr #4#))
>        (setf #1# t)))
>     (unless #1#))
>   #3#)

The following patch does this, but I'm not sure whether this is correct
or not -- in other cases, the _ convention just removes the warning, but
doesn't change the semantics.

I wondered whether we could just suppress this warning like this:

             ,(if (string-match-p "\\`_" (symbol-name var))
                  `(with-suppressed-warnings ((not-unused ,var))
                     (setf ,var (iter-next ,it-symbol)))
                `(setf ,var (iter-next ,it-symbol)))

But no, cconv--analyze-use is called too early, and would have to be
taught about `with-suppressed-warnings'...  which, looking at the code,
isn't immediately obvious how to do.

So does anybody have any ideas here?

diff --git a/lisp/emacs-lisp/generator.el b/lisp/emacs-lisp/generator.el
index 9eb6d95964..0b644cc72c 100644
--- a/lisp/emacs-lisp/generator.el
+++ b/lisp/emacs-lisp/generator.el
@@ -731,7 +731,10 @@ iter-do
            (,it-symbol ,iterator))
        (while (not ,done-symbol)
          (condition-case ,condition-symbol
-             (setf ,var (iter-next ,it-symbol))
+             ;; Variables that start with an underscore shouldn't be set.
+             ,(if (string-match-p "\\`_" (symbol-name var))
+                  nil
+                `(setf ,var (iter-next ,it-symbol)))
            (iter-end-of-sequence
             (setf ,result-symbol (cdr ,condition-symbol))
             (setf ,done-symbol t)))


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Christopher Wellons <wellons <at> nullprogram.com>,
 Daniel Colascione <dancol <at> dancol.org>, 31641 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Thu, 04 Feb 2021 11:36:27 -0500

>>> When byte-compiling an iter-do form with a variable intended to be
>>> left unused, the compiler emits a false warning:
>>>
>>> ;;; -*- lexical-binding: t; -*-
>>> (require 'generator)
>>> (iter-do (_ i))
>>> ;; -> "Warning: variable ‘_’ not left unused"
>>
>> Looking at the expansion, I guess the setf should be dropped if the
>> variable name starts with _.
>>
>> (let (_
>>       #3=#:iter-do-result11
>>       (#1=#:iter-do-iterator-done8 nil)
>>       (#2=#:iter-do-iterator10 i))
>>   (while (not #1#)
>>     (condition-case #4=#:iter-do-condition9
>>         (setf _ (iter-next #2#))
>>       (iter-end-of-sequence
>>        (setf #3# (cdr #4#))
>>        (setf #1# t)))
>>     (unless #1#))
>>   #3#)

FWIW, I find the above expansion to provide somewhat "dirty" semantics
in the sense that

    (let ((funs '()))
      (iter-do (n i) (push (lambda () n) funs))
      funs)

will return a list of functions which all return the same value (the
last `n`).

You can clean up this semantics and the warning at the same time by
using an expansion like:

    (let (#3=#:iter-do-result11
          (#1=#:iter-do-iterator-done8 nil)
          (#2=#:iter-do-iterator10 i))
      (while (not #1#)
        (let ((_ (condition-case #4=#:iter-do-condition9
                     (iter-next #2#)
                   (iter-end-of-sequence
                    (setf #3# (cdr #4#))
                    (setf #1# t)))
        (unless #1# [BODY])
      #3#)

BTW, I think we can remove the duplicate #1 test by moving the body of
the `while` into its test, e.g.:

    (let (#3=#:iter-do-result11
          (#1=#:iter-do-iterator-done8 nil)
          (#2=#:iter-do-iterator10 i))
      (while
        (let ((_ (condition-case #4=#:iter-do-condition9
                     (iter-next #2#)
                   (iter-end-of-sequence
                    (setf #3# (cdr #4#))
                    (setf #1# t)))))
          (unless #1#
            [BODY]
            t)))
      #3#)

It's too bad that [BODY] can throw `iter-end-of-sequence`, since
otherwise we could move the `condition-case` outside of the loop and get
something more efficient.


        Stefan


> The following patch does this, but I'm not sure whether this is correct
> or not -- in other cases, the _ convention just removes the warning, but
> doesn't change the semantics.
>
> I wondered whether we could just suppress this warning like this:
>
>              ,(if (string-match-p "\\`_" (symbol-name var))
>                   `(with-suppressed-warnings ((not-unused ,var))
>                      (setf ,var (iter-next ,it-symbol)))
>                 `(setf ,var (iter-next ,it-symbol)))
>
> But no, cconv--analyze-use is called too early, and would have to be
> taught about `with-suppressed-warnings'...  which, looking at the code,
> isn't immediately obvious how to do.
>
> So does anybody have any ideas here?
>
> diff --git a/lisp/emacs-lisp/generator.el b/lisp/emacs-lisp/generator.el
> index 9eb6d95964..0b644cc72c 100644
> --- a/lisp/emacs-lisp/generator.el
> +++ b/lisp/emacs-lisp/generator.el
> @@ -731,7 +731,10 @@ iter-do
>             (,it-symbol ,iterator))
>         (while (not ,done-symbol)
>           (condition-case ,condition-symbol
> -             (setf ,var (iter-next ,it-symbol))
> +             ;; Variables that start with an underscore shouldn't be set.
> +             ,(if (string-match-p "\\`_" (symbol-name var))
> +                  nil
> +                `(setf ,var (iter-next ,it-symbol)))
>             (iter-end-of-sequence
>              (setf ,result-symbol (cdr ,condition-symbol))
>              (setf ,done-symbol t)))

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Christopher Wellons <wellons <at> nullprogram.com>,
 Daniel Colascione <dancol <at> dancol.org>, 31641 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Fri, 05 Feb 2021 09:53:10 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> FWIW, I find the above expansion to provide somewhat "dirty" semantics
> in the sense that
>
>     (let ((funs '()))
>       (iter-do (n i) (push (lambda () n) funs))
>       funs)
>
> will return a list of functions which all return the same value (the
> last `n`).
>
> You can clean up this semantics and the warning at the same time by
> using an expansion like:

If I'm reading that correctly, that does seem like more obvious
semantics, but is it too late to change this now?  I'm not sure how much
generator.el is used in the wild yet...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Christopher Wellons <wellons <at> nullprogram.com>,
 Daniel Colascione <dancol <at> dancol.org>, 31641 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Fri, 05 Feb 2021 10:03:53 -0500

> If I'm reading that correctly, that does seem like more obvious
> semantics, but is it too late to change this now?  I'm not sure how much
> generator.el is used in the wild yet...

The change in semantics is fairly subtle (e.g. Common-Lisp allows both
choices for the similar choice of behavior in `dotimes` and `dolist`),
so it's rather unlikely to break existing code, IMO (and I think the
likelihood would be higher if the change were in the other direction).


        Stefan

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Christopher Wellons <wellons <at> nullprogram.com>, 31641 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Fri, 05 Feb 2021 17:12:15 +0000

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> diff --git a/lisp/emacs-lisp/generator.el b/lisp/emacs-lisp/generator.el
> index 9eb6d95964..0b644cc72c 100644
> --- a/lisp/emacs-lisp/generator.el
> +++ b/lisp/emacs-lisp/generator.el
> @@ -731,7 +731,10 @@ iter-do
>             (,it-symbol ,iterator))
>         (while (not ,done-symbol)
>           (condition-case ,condition-symbol
> -             (setf ,var (iter-next ,it-symbol))
> +             ;; Variables that start with an underscore shouldn't be set.
> +             ,(if (string-match-p "\\`_" (symbol-name var))
> +                  nil
> +                `(setf ,var (iter-next ,it-symbol)))

Nit - AKA the following?

  ,(unless (string-prefix-p "_" (symbol-name var))
     `(setf ,var (iter-next ,it-symbol)))

-- 
Basil

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Christopher Wellons <wellons <at> nullprogram.com>,
 Daniel Colascione <dancol <at> dancol.org>, 31641 <at> debbugs.gnu.org,
 Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#31641: 26.1; iter-do variable not left unused warning
Date: Sat, 06 Feb 2021 11:31:49 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> You can clean up this semantics and the warning at the same time by
> using an expansion like:
>
>     (let (#3=#:iter-do-result11
>           (#1=#:iter-do-iterator-done8 nil)
>           (#2=#:iter-do-iterator10 i))
>       (while (not #1#)
>         (let ((_ (condition-case #4=#:iter-do-condition9
>                      (iter-next #2#)
>                    (iter-end-of-sequence
>                     (setf #3# (cdr #4#))
>                     (setf #1# t)))
>         (unless #1# [BODY])
>       #3#)

So I tried this...

diff --git a/lisp/emacs-lisp/generator.el b/lisp/emacs-lisp/generator.el
index 9eb6d95964..dfd2513350 100644
--- a/lisp/emacs-lisp/generator.el
+++ b/lisp/emacs-lisp/generator.el
@@ -725,17 +725,18 @@ iter-do
         (condition-symbol (cps--gensym "iter-do-condition"))
         (it-symbol (cps--gensym "iter-do-iterator"))
         (result-symbol (cps--gensym "iter-do-result")))
-    `(let (,var
-           ,result-symbol
+    `(let (,result-symbol
            (,done-symbol nil)
            (,it-symbol ,iterator))
        (while (not ,done-symbol)
-         (condition-case ,condition-symbol
-             (setf ,var (iter-next ,it-symbol))
-           (iter-end-of-sequence
-            (setf ,result-symbol (cdr ,condition-symbol))
-            (setf ,done-symbol t)))
-         (unless ,done-symbol ,@body))
+         (let ((,var
+                (condition-case ,condition-symbol
+                    (iter-next ,it-symbol)
+                  (iter-end-of-sequence
+                   (setf ,result-symbol (cdr ,condition-symbol))
+                   (setf ,done-symbol t)))))
+           (unless ,done-symbol
+             ,@body)))
        ,result-symbol)))
 
 (defvar cl--loop-args)

But then this fails:

1 unexpected results:
   FAILED  iter-lambda-variable-shadowing

Uhm...  oh, it fails even without that change?  ...  If I just say "make
check", then it doesn't fail, but if I say "make generator-tests", then
it fails?  Very odd.

> BTW, I think we can remove the duplicate #1 test by moving the body of
> the `while` into its test, e.g.:
>
>     (let (#3=#:iter-do-result11
>           (#1=#:iter-do-iterator-done8 nil)
>           (#2=#:iter-do-iterator10 i))
>       (while
>         (let ((_ (condition-case #4=#:iter-do-condition9
>                      (iter-next #2#)
>                    (iter-end-of-sequence
>                     (setf #3# (cdr #4#))
>                     (setf #1# t)))))
>           (unless #1#
>             [BODY]
>             t)))
>       #3#)

Indeed.  I've now pushed something like this to Emacs 28.

> It's too bad that [BODY] can throw `iter-end-of-sequence`, since
> otherwise we could move the `condition-case` outside of the loop and get
> something more efficient.

Yup.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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