bug#71120: 29.3; buglet in cl-loop

bug#71120: 29.3; buglet in cl-loop

[Philippe Schnoebelen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

When I need a list of 100 random dice throws I write

	(cl-loop for i from 1 to 100 collect (random 6))

It compiles just fine.

If instead I use

	(cl-loop for _i from 1 to 100 collect (random 6))

then I get a compilation warning:

	foo.el:1:18: Warning: variable ‘_i’ not left unused

It should be the other way around.

The variable 'i' is unused in the first form and that deserves a warning 
at compile time. Otherwise the compiler will not help me catch the typo in

	(cl-loop for i from 0 to 10 do
	   (cl-loop for j from 0 to 10 do
	      (foo j j))) ;; <<<=== typo, I meant (foo i j)


--ph.schnoebelen, happy and thankful GNU Emacs user since 1983 (Thanks 
to all involved!!)

bug#71120: 29.3; buglet in cl-loop

[Philip Kaludercic]

Philippe Schnoebelen <phs@lmf.cnrs.fr> writes:

> When I need a list of 100 random dice throws I write
>
> 	(cl-loop for i from 1 to 100 collect (random 6))
>
> It compiles just fine.
>
> If instead I use
>
> 	(cl-loop for _i from 1 to 100 collect (random 6))
>
> then I get a compilation warning:
>
> 	foo.el:1:18: Warning: variable ‘_i’ not left unused
>
> It should be the other way around.

The issue here is that the warning describes an issue in the output, in
the latter case

  (let* ((_i 1) (--cl-var-- nil))
    (while (<= _i 100)
      (setq --cl-var-- (cons (random 6) --cl-var--))
      (setq _i (+ _i 1)))
    (nreverse --cl-var--))

As you see, _i is both evaluated in (+ _i 1) and updated.

Here you have a minimal working example of the warning:

  (byte-compile (lambda () (let ((_x 3)) _x)))

  ;; Warning: variable ‘_x’ not left unused

My guess is that fixing this would require cl-loop to notice that the
counter is prefixed with a "_" and then use some other variable, but
that might lead to issues with existing code.  Perhaps this
transformation might be safe in that case:

  (let* ((<fresh-variable> 1) (--cl-var-- nil))
    (while (<= <fresh-variable> 100)
      (let ((_i <fresh-variable>))
	(setq --cl-var-- (cons (random 6) --cl-var--))
	(setq <fresh-variable> (+ <fresh-variable> 1))))
    (nreverse --cl-var--))

I have added Mattias and Stefan to the CCs, as they'll probably have
more qualified comments to add.

>
> The variable 'i' is unused in the first form and that deserves a
> warning at compile time. Otherwise the compiler will not help me catch
> the typo in
>
> 	(cl-loop for i from 0 to 10 do
> 	   (cl-loop for j from 0 to 10 do
> 	      (foo j j))) ;; <<<=== typo, I meant (foo i j)
>
>
> --ph.schnoebelen, happy and thankful GNU Emacs user since 1983 (Thanks
>   to all involved!!)
>
>
>
>
>

-- 
	Philip Kaludercic on peregrine

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Philip Kaludercic <philipk@posteo.net> writes:

> Philippe Schnoebelen <phs@lmf.cnrs.fr> writes:
>
>> When I need a list of 100 random dice throws I write
>>
>> 	(cl-loop for i from 1 to 100 collect (random 6))
>>
>> It compiles just fine.
>>
>> If instead I use
>>
>> 	(cl-loop for _i from 1 to 100 collect (random 6))
>>
>> then I get a compilation warning:
>>
>> 	foo.el:1:18: Warning: variable ‘_i’ not left unused
>>
>> It should be the other way around.
>
> The issue here is that the warning describes an issue in the output, in
> the latter case
>
>   (let* ((_i 1) (--cl-var-- nil))
>     (while (<= _i 100)
>       (setq --cl-var-- (cons (random 6) --cl-var--))
>       (setq _i (+ _i 1)))
>     (nreverse --cl-var--))
>
> As you see, _i is both evaluated in (+ _i 1) and updated.
>
> Here you have a minimal working example of the warning:
>
>   (byte-compile (lambda () (let ((_x 3)) _x)))
>
>   ;; Warning: variable ‘_x’ not left unused
>
> My guess is that fixing this would require cl-loop to notice that the
> counter is prefixed with a "_" and then use some other variable, but
> that might lead to issues with existing code.  Perhaps this
> transformation might be safe in that case:
>
>   (let* ((<fresh-variable> 1) (--cl-var-- nil))
>     (while (<= <fresh-variable> 100)
>       (let ((_i <fresh-variable>))
> 	(setq --cl-var-- (cons (random 6) --cl-var--))
> 	(setq <fresh-variable> (+ <fresh-variable> 1))))
>     (nreverse --cl-var--))
>
> I have added Mattias and Stefan to the CCs, as they'll probably have
> more qualified comments to add.

This is the same transformation that came to my mind reading the orginal
report, I think it should be safe.

BTW Philippe, you can workaround the bug with:

(cl-loop repeat 100 collect (random 6))

Bests

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Andrea Corallo <acorallo@gnu.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Philippe Schnoebelen <phs@lmf.cnrs.fr> writes:
>>
>>> When I need a list of 100 random dice throws I write
>>>
>>> 	(cl-loop for i from 1 to 100 collect (random 6))
>>>
>>> It compiles just fine.
>>>
>>> If instead I use
>>>
>>> 	(cl-loop for _i from 1 to 100 collect (random 6))
>>>
>>> then I get a compilation warning:
>>>
>>> 	foo.el:1:18: Warning: variable ‘_i’ not left unused
>>>
>>> It should be the other way around.
>>
>> The issue here is that the warning describes an issue in the output, in
>> the latter case
>>
>>   (let* ((_i 1) (--cl-var-- nil))
>>     (while (<= _i 100)
>>       (setq --cl-var-- (cons (random 6) --cl-var--))
>>       (setq _i (+ _i 1)))
>>     (nreverse --cl-var--))
>>
>> As you see, _i is both evaluated in (+ _i 1) and updated.
>>
>> Here you have a minimal working example of the warning:
>>
>>   (byte-compile (lambda () (let ((_x 3)) _x)))
>>
>>   ;; Warning: variable ‘_x’ not left unused
>>
>> My guess is that fixing this would require cl-loop to notice that the
>> counter is prefixed with a "_" and then use some other variable, but
>> that might lead to issues with existing code.  Perhaps this
>> transformation might be safe in that case:
>>
>>   (let* ((<fresh-variable> 1) (--cl-var-- nil))
>>     (while (<= <fresh-variable> 100)
>>       (let ((_i <fresh-variable>))
>> 	(setq --cl-var-- (cons (random 6) --cl-var--))
>> 	(setq <fresh-variable> (+ <fresh-variable> 1))))
>>     (nreverse --cl-var--))
>>
>> I have added Mattias and Stefan to the CCs, as they'll probably have
>> more qualified comments to add.
>
> This is the same transformation that came to my mind reading the orginal
> report, I think it should be safe.

As a datapoint, SBCL does expands this with inside a

(declare (ignore i))

Unfortuantelly we don't support it :/ and our 'ignore' doesn't work here
as goes in the opposite direction :(

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>   (let* ((<fresh-variable> 1) (--cl-var-- nil))
>>     (while (<= <fresh-variable> 100)
>>       (let ((_i <fresh-variable>))
>> 	(setq --cl-var-- (cons (random 6) --cl-var--))
>> 	(setq <fresh-variable> (+ <fresh-variable> 1))))
>>     (nreverse --cl-var--))
>>
>> I have added Mattias and Stefan to the CCs, as they'll probably have
>> more qualified comments to add.

Not really: There's a similar problem with `cl-destructuring-bind` and
similarly, when I started to look at the corresponding code I quickly
gave up.

At least for `cl-destructuring-bind` I think I understand the intended
behavior well enough that I might be able to implement code which
behaves as we want (tho I don't know how to *modify* the existing code
to reach that stage).

In the case of `cl-loop` the intended behavior is much too complex for
my poor head, so I don't think I'd be able to decide whether it does the
right thing for all cases.


        Stefan

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

Philippe Schnoebelen <phs@lmf.cnrs.fr> writes:

> When I need a list of 100 random dice throws I write
> 
> 	(cl-loop for i from 1 to 100 collect (random 6))
> 
> It compiles just fine.
> 
> If instead I use
> 
> 	(cl-loop for _i from 1 to 100 collect (random 6))
> 
> then I get a compilation warning:
> 
> 	foo.el:1:18: Warning: variable ‘_i’ not left unused

Quite unfair that you have no a priori way of knowing whether your variable name is actually the one that `cl-loop` uses for iteration or just one bound for each iteration (as in `dotimes`).

A sloppy reading of Common Lisp's `loop` spec, which we don't need to follow since this isn't CL but we'd be fools to deviate too far from without a good reason, didn't tell me anything.

In particular nothing about whether the user is allowed to alter the variable in order to change the iteration. For example, what should

  (cl-loop for i from 1 to 100
           when (= i 3) do (setq i 98)
           collect i)

return? Perhaps better not touch that.

Thus I don't think there's anything we really need to do here, do you?

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> Philippe Schnoebelen <phs@lmf.cnrs.fr> writes:
>
>> When I need a list of 100 random dice throws I write
>> 
>> 	(cl-loop for i from 1 to 100 collect (random 6))
>> 
>> It compiles just fine.
>> 
>> If instead I use
>> 
>> 	(cl-loop for _i from 1 to 100 collect (random 6))
>> 
>> then I get a compilation warning:
>> 
>> 	foo.el:1:18: Warning: variable ‘_i’ not left unused
>
> Quite unfair that you have no a priori way of knowing whether your variable name is actually the one that `cl-loop` uses for iteration or just one bound for each iteration (as in `dotimes`).
>
> A sloppy reading of Common Lisp's `loop` spec, which we don't need to follow since this isn't CL but we'd be fools to deviate too far from without a good reason, didn't tell me anything.
>
> In particular nothing about whether the user is allowed to alter the variable in order to change the iteration. For example, what should
>
>   (cl-loop for i from 1 to 100
>            when (= i 3) do (setq i 98)
>            collect i)
>
> return? Perhaps better not touch that.

If the spec doesn't say anything usually means it's left to the
implementors (IOW it's UB).

> Thus I don't think there's anything we really need to do here, do you?

I, for one, think the nicest option is the one SBCL (and I guess other
CL implementations) are using, that is to have 'i' 'ignorable', the
problem is that we don't support this at language level.

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>>> 	(cl-loop for i from 1 to 100 collect (random 6))
[...]
>>> 	(cl-loop for _i from 1 to 100 collect (random 6))
>> Thus I don't think there's anything we really need to do here, do you?
> I, for one, think the nicest option is the one SBCL (and I guess other
> CL implementations) are using, that is to have 'i' 'ignorable', the
> problem is that we don't support this at language level.

I don't quite see how "ignorable" comes into play here.

In the first case above, the var is "not used", has a "normal"
name, and we get no warning.  Does SBCL do the same or does it emit
a warning?

In the second case, the var is "not used", has an "I'm not used" name,
and we do get a warning.  Does SBCL even support "I'm not used" names?


        Stefan

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

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

>>>> 	(cl-loop for i from 1 to 100 collect (random 6))
> [...]
>>>> 	(cl-loop for _i from 1 to 100 collect (random 6))
>>> Thus I don't think there's anything we really need to do here, do you?
>> I, for one, think the nicest option is the one SBCL (and I guess other
>> CL implementations) are using, that is to have 'i' 'ignorable', the
>> problem is that we don't support this at language level.
>
> I don't quite see how "ignorable" comes into play here.
>
> In the first case above, the var is "not used", has a "normal"
> name, and we get no warning.  Does SBCL do the same or does it emit
> a warning?

It does not emit a warning

> In the second case, the var is "not used", has an "I'm not used" name,
> and we do get a warning.  Does SBCL even support "I'm not used" names?

It does not.

In Elisp a variable can be either normal or _* (not used).

In CL a variable other than normal can be 'ignore' which correspond to
our _* or 'ignorable'.  This last AFAIU stands for might or might not be
used but don't emit warnings.

It's no big deal but I think 'ignorable' is useful for macros expanding
code with variables that might or not be used by the user like in this.

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

30 maj 2024 kl. 17.41 skrev Andrea Corallo <acorallo@gnu.org>:

> In CL a variable other than normal can be 'ignore' which correspond to
> our _* or 'ignorable'.  This last AFAIU stands for might or might not be
> used but don't emit warnings.

Like (ignore x) in elisp?

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> 30 maj 2024 kl. 17.41 skrev Andrea Corallo <acorallo@gnu.org>:
>
>> In CL a variable other than normal can be 'ignore' which correspond to
>> our _* or 'ignorable'.  This last AFAIU stands for might or might not be
>> used but don't emit warnings.
>
> Like (ignore x) in elisp?

Yes, but with the caveat that (ignore _i) still generates a warning.

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

30 maj 2024 kl. 19.15 skrev Andrea Corallo <acorallo@gnu.org>:

>> Like (ignore x) in elisp?
> 
> Yes, but with the caveat that (ignore _i) still generates a warning.

That could be fixed; it likely involves some other reforms that we want to do anyway (and being prepared behind the scenes). Will take a while, though.

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> 30 maj 2024 kl. 19.15 skrev Andrea Corallo <acorallo@gnu.org>:
>
>>> Like (ignore x) in elisp?
>> 
>> Yes, but with the caveat that (ignore _i) still generates a warning.
>
> That could be fixed;

Cool

> it likely involves some other reforms that we want to do anyway (and
> being prepared behind the scenes). Will take a while, though.

Would you like to share with us what's cookin'? 😀

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Gerd Möllmann]

Andrea Corallo <acorallo@gnu.org> writes:

> Mattias Engdegård <mattias.engdegard@gmail.com> writes:
>
>> 30 maj 2024 kl. 19.15 skrev Andrea Corallo <acorallo@gnu.org>:
>>
>>>> Like (ignore x) in elisp?
>>> 
>>> Yes, but with the caveat that (ignore _i) still generates a warning.
>>
>> That could be fixed;
>
> Cool
>
>> it likely involves some other reforms that we want to do anyway (and
>> being prepared behind the scenes). Will take a while, though.
>
> Would you like to share with us what's cookin'? 😀

Good question. Who is "we" and why is it not on emacs-devel?

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

30 maj 2024 kl. 19.42 skrev Andrea Corallo <acorallo@gnu.org>:

>> That could be fixed;
> 
> Cool

We could probably do it in cconv now but it would be a hack and I would prefer not to.

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> 30 maj 2024 kl. 19.42 skrev Andrea Corallo <acorallo@gnu.org>:
>
>>> That could be fixed;
>> 
>> Cool
>
> We could probably do it in cconv now but it would be a hack and I would prefer not to.

Okay so I guess we can keep this bug open as a reminder for this?

Thanks

  Andrea

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

3 juni 2024 kl. 17.24 skrev Andrea Corallo <acorallo@gnu.org>:

> Okay so I guess we can keep this bug open as a reminder for this?

Thank you, but there's no need for that as far as I'm concerned.

bug#71120: 29.3; buglet in cl-loop

[Andrea Corallo]

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> 3 juni 2024 kl. 17.24 skrev Andrea Corallo <acorallo@gnu.org>:
>
>> Okay so I guess we can keep this bug open as a reminder for this?
>
> Thank you, but there's no need for that as far as I'm concerned.

Do you prefer a dedicated bug for 'ignore'?

bug#71120: 29.3; buglet in cl-loop

[Mattias Engdegård]

3 juni 2024 kl. 17.42 skrev Andrea Corallo <acorallo@gnu.org>:

> Do you prefer a dedicated bug for 'ignore'?

I can keep track of it for myself but feel free to add a bug if you want to make sure it isn't forgotten.