bug#8711: 24.0.50; binding _ to unused values with lexical-binding

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Helmut Eller]

There seems be no way to avoid a warning in code like this:

;; -*- lexical-binding: t -*-
(eval-when-compile (require 'cl))
(defun foo (x) (destructuring-bind (_) x))
(defun bar (x) (destructuring-bind (_) x (ignore _)))

In foo the compiler complains because _ is never used and
it bar it complains because it is used.


In GNU Emacs 24.0.50.1 (i686-pc-linux-gnu, GTK+ Version 2.20.1)
 of 2011-05-21 on ix
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
configured using `configure  '--with-gif=no' '--with-gnutls=no''

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Lawrence Mitchell]

Helmut Eller wrote:
> There seems be no way to avoid a warning in code like this:

> ;; -*- lexical-binding: t -*-
> (eval-when-compile (require 'cl))
> (defun foo (x) (destructuring-bind (_) x))
> (defun bar (x) (destructuring-bind (_) x (ignore _)))

> In foo the compiler complains because _ is never used and
> it bar it complains because it is used.

This latter is because cconv-analyse-use is not smart enough when
checking for use of variables starting with an underscore.

;; -*- lexical-binding: t -*-

(let ((_ nil))
  (ignore _))

=>

Warning: variable `_' not left unused.

This despite ignore not touching it.

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

>> There seems be no way to avoid a warning in code like this:

>> ;; -*- lexical-binding: t -*-
>> (eval-when-compile (require 'cl))
>> (defun foo (x) (destructuring-bind (_) x))

Hmm... the byte-compiler complains because the above gets turned into

     (let* ((--cl-rest-- x)
            (_
             (if (= (length --cl-rest--) 1)
                 (car --cl-rest--)
               (signal 'wrong-number-of-arguments (list nil (length --cl-rest--))))))
       nil)

which gets changes by byte-opt.el into
       
     (let* ((--cl-rest-- x))
       (if (= (length --cl-rest--) 1)
           (car --cl-rest--)
         (signal 'wrong-number-of-arguments (list nil (length --cl-rest--))))
       nil)

And note that this is only done for a `nil' body (tho it can also be
something like (progn nil) because that would be optimized to nil as
well), and not for a body like `5' (although the optimization is just
as valid for `5' as for nil).
         
>> (defun bar (x) (destructuring-bind (_) x (ignore _)))

  (defun bar (x) (destructuring-bind (_) x (ignore nil)))

works (because (ignore nil) happens not to be optimized to nil).
 
> This latter is because cconv-analyse-use is not smart enough when
> checking for use of variables starting with an underscore.

> ;; -*- lexical-binding: t -*-

> (let ((_ nil))
>   (ignore _))

> =>

> Warning: variable `_' not left unused.

> This despite ignore not touching it.

The two warnings come from different analyzes:
- The warning for `bar' comes from cconv.el which is intended to check
  whether the variable is syntactically used, rather than semantically.
- The warning for `foo' checks whether pure functions are not called for
  their side-effects and it's applied after optimizations so in the
  above code, the code generated by destructuring-bind ends up optimized
  to something that calls `car' without binding the result to _ because
  it figured that _ is not used and just got rid of it without warning.

So most likely the answers I give here aren't satisfactory to the OP,
since his real problem is probably different than (destructuring-bind
(_) x) and the solution for that problem is probably going to be
yet different.


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Helmut Eller]

* Stefan Monnier [2011-05-23 14:24] writes:

> The two warnings come from different analyzes:
> - The warning for `bar' comes from cconv.el which is intended to check
>   whether the variable is syntactically used, rather than semantically.
> - The warning for `foo' checks whether pure functions are not called for
>   their side-effects and it's applied after optimizations so in the
>   above code, the code generated by destructuring-bind ends up optimized
>   to something that calls `car' without binding the result to _ because
>   it figured that _ is not used and just got rid of it without warning.
>
> So most likely the answers I give here aren't satisfactory to the OP,
> since his real problem is probably different than (destructuring-bind
> (_) x) and the solution for that problem is probably going to be
> yet different.

My problem is basically that I have a macro "destructure-case" that
expands to destructuring-bind, e.g.:

(destructure-case location
  ((:error _) nil)) ; do nothing

expands to

(ecase (car location)
  (:error (destructuring-bind (_) (cdr location)
            (ignore _)
            nil)))

The macro inserts the (ignore _) to suppress the "value returned from
(car --cl-rest--) is unused" warning.  But that trick only works for the
non-lexical-binding case.

I think that rewriting (let ((_ foo))) to (progn foo nil) is not quite
right because that loses the information that the value that flows to
the variable _ is intentionally ignored, but I guess that is somewhat
hard to fix this.  (Maybe byte-optimize-letX could call
byte-optimize-form-code-walker directly but instead of specifying that
the form is evaluated "for-effect" somehow say that the value is
"ignorable".)

It's perhaps easier to teach destructuring-bind the _ convention so that
it can produce more direct code.


There is also a somewhat related problem with loop:

;; -*- lexical-binding: t -*-
(defun foo (alist) (loop for (_key . value) in alist collect value))

produces a "variable `_key' not left unused" warning.

Helmut

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

> My problem is basically that I have a macro "destructure-case" that
> expands to destructuring-bind, e.g.:

> (destructure-case location
>   ((:error _) nil)) ; do nothing

> expands to

> (ecase (car location)
>   (:error (destructuring-bind (_) (cdr location)
>             (ignore _)
>             nil)))

> The macro inserts the (ignore _) to suppress the "value returned from
> (car --cl-rest--) is unused" warning.

It can insert (ignore nil) instead which should have the same effect but
without triggering the other warning with lexical-binding.

> I think that rewriting (let ((_ foo))) to (progn foo nil) is not quite
> right because that loses the information that the value that flows to
> the variable _ is intentionally ignored, but I guess that is somewhat
> hard to fix this.  (Maybe byte-optimize-letX could call
> byte-optimize-form-code-walker directly but instead of specifying that
> the form is evaluated "for-effect" somehow say that the value is
> "ignorable".)

I'm not completely satisfied with the "value returned from <foo> is
unused" indeed, but it's not clear exactly how to fix it (short of
always silencing it, which I'd rather not since it occasionally catches
errors).

> It's perhaps easier to teach destructuring-bind the _ convention so that
> it can produce more direct code.

Could be.  destructuring-bind generates some other minorly-problematic
code for the lexbind case.

> There is also a somewhat related problem with loop:

> ;; -*- lexical-binding: t -*-
> (defun foo (alist) (loop for (_key . value) in alist collect value))

> produces a "variable `_key' not left unused" warning.

I think the same problem happens with dotimes/dolist using the old
definition: the loop vars `key' and `value' are let-bound outside the
loop and then setq'd at each loop iteration (it's this setq that causes
them to be "not left unused").  This was OK for the dynamic scoping case
because let-binding is significantly more costly than setq, but it is
not right for the lexical scoping case where the cost of let is not
higher than `setq' and where the semantic is actually then wrong:
e.g. if you "collect (lambda () value)" the current code ends up
returning a list of functions that all return the last `value', rather
than a list of functions that each return one of the `values' in
the alist.


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Helmut Eller]

* Stefan Monnier [2011-05-23 19:29] writes:

>> My problem is basically that I have a macro "destructure-case" that
>> expands to destructuring-bind, e.g.:
>
>> (destructure-case location
>>   ((:error _) nil)) ; do nothing
>
>> expands to
>
>> (ecase (car location)
>>   (:error (destructuring-bind (_) (cdr location)
>>             (ignore _)
>>             nil)))
>
>> The macro inserts the (ignore _) to suppress the "value returned from
>> (car --cl-rest--) is unused" warning.
>
> It can insert (ignore nil) instead which should have the same effect but
> without triggering the other warning with lexical-binding.

Indeed.  But I quickly run into another case where that doesn't help:

(defun foo (x) (destructuring-bind (y &rest _) x y))

>> There is also a somewhat related problem with loop:
>
>> ;; -*- lexical-binding: t -*-
>> (defun foo (alist) (loop for (_key . value) in alist collect value))
>
>> produces a "variable `_key' not left unused" warning.
>
> I think the same problem happens with dotimes/dolist using the old
> definition: the loop vars `key' and `value' are let-bound outside the
> loop and then setq'd at each loop iteration (it's this setq that causes
> them to be "not left unused").  This was OK for the dynamic scoping case
> because let-binding is significantly more costly than setq, but it is
> not right for the lexical scoping case where the cost of let is not
> higher than `setq' and where the semantic is actually then wrong:
> e.g. if you "collect (lambda () value)" the current code ends up
> returning a list of functions that all return the last `value', rather
> than a list of functions that each return one of the `values' in
> the alist.

The semantic of loop (in Common Lisp) allows both variants. So the
"portable" idiom is "collect (let ((value value)) (lambda () value))".

Helmut

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Helmut Eller]

* Stefan Monnier [2011-05-23 19:29] writes:

> I think the same problem happens with dotimes/dolist using the old
> definition: the loop vars `key' and `value' are let-bound outside the
> loop and then setq'd at each loop iteration (it's this setq that causes
> them to be "not left unused").  This was OK for the dynamic scoping case
> because let-binding is significantly more costly than setq, but it is
> not right for the lexical scoping case where the cost of let is not
> higher than `setq' and where the semantic is actually then wrong:
> e.g. if you "collect (lambda () value)" the current code ends up
> returning a list of functions that all return the last `value', rather
> than a list of functions that each return one of the `values' in
> the alist.

BTW, is there a good reason why we can't disallow setq on closed over
variables in Elisp?  That kind of mutability seems like a bad idea in a
concurrent world.  Closures are a new feature so we could make a
conscious decision not to introduce the problematic mutable variables.

Helmut 

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

>> I think the same problem happens with dotimes/dolist using the old
>> definition: the loop vars `key' and `value' are let-bound outside the
>> loop and then setq'd at each loop iteration (it's this setq that causes
>> them to be "not left unused").  This was OK for the dynamic scoping case
>> because let-binding is significantly more costly than setq, but it is
>> not right for the lexical scoping case where the cost of let is not
>> higher than `setq' and where the semantic is actually then wrong:
>> e.g. if you "collect (lambda () value)" the current code ends up
>> returning a list of functions that all return the last `value', rather
>> than a list of functions that each return one of the `values' in
>> the alist.
> BTW, is there a good reason why we can't disallow setq on closed over
> variables in Elisp?

Yes and no: technically, we could, of course, but I'm not sure that's
worth the trouble.  I'm in favor of pure languages (e.g. I've local
changes that make Emacs strings immutable, often install changes that
reduce the use of setq, ...) but realistically Emacs Lisp won't get rid
of setq any time soon.  Now, why single out setq on closed over variables?
Note that currently closures are used internally a lot more often than
you'd think, since every condition-case, unwind-protect, and catch
makes its body and branches into closures.  Also it's common for
lambda arguments to mapcar and mapc to use setq on some closed over
variable used a sort of accumulator.

> That kind of mutability seems like a bad idea in a
> concurrent world.

Emacs has a crapload of global state, so we won't be able to wiggle
around global mutability problems.  Maybe every bit helps, but I'm very
much unconvinced.

> Closures are a new feature so we could make a conscious decision not
> to introduce the problematic mutable variables.

But I also want it to be as easy as possible to turn valid dynbind Elisp
code into valid lexbind code, so any additional restriction needs a very
good justification.


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

>> It can insert (ignore nil) instead which should have the same effect but
>> without triggering the other warning with lexical-binding.

> Indeed.  But I quickly run into another case where that doesn't help:

> (defun foo (x) (destructuring-bind (y &rest _) x y))

I don't see any warning for this code.  What warning do you get?

> The semantic of loop (in Common Lisp) allows both variants. So the
> "portable" idiom is "collect (let ((value value)) (lambda () value))".

Interesting.  I didn't think they'd accept the alternate semantic, which
is clearly less useful and less efficient at the same time (tho maybe
there are implementation strategies that may not make it that much less
efficient).


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Helmut Eller]

* Stefan Monnier [2011-05-24 00:56] writes:

>>> It can insert (ignore nil) instead which should have the same effect but
>>> without triggering the other warning with lexical-binding.
>
>> Indeed.  But I quickly run into another case where that doesn't help:
>
>> (defun foo (x) (destructuring-bind (y &rest _) x y))
>
> I don't see any warning for this code.  What warning do you get?

;; -*- lexical-binding: t -*-
(defun foo (x) (destructuring-bind (y &rest _) x y))

x.el:2:1:Warning: variable `_' not left unused


Helmut

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

> ;; -*- lexical-binding: t -*-
> (defun foo (x) (destructuring-bind (y &rest _) x y))

> x.el:2:1:Warning: variable `_' not left unused

Ah, yes, this one.  This is because CL's destructuring-bind expands the
above to something like

  (let ((_ x)
        (y (prog1 (car _) (setq _ (cdr _)))))
   y)

where _ is indeed not left unused.  It also means that a closure that
for (destructuring-bind (y &rest z) x (lambda () z)) the closure
captures a mutated var which is something more costly (the var needs to
be put inside a cons cell, leading to code along the lines of:

  (let ((z (list x))
        (y (prog1 (car (car z)) (setcar z (cdr (car z))))))
   (make-closure () [z] (car z)))

Again this is a side-effect of `let' being costly in dynbind code.
For lexbind code it would be better for destructuring-bind to use an
additional internal var:

  (let ((<internal> x)
        (y (prog1 (car <internal>) (setq <internal> (cdr <internal>))))
        (_ <internal>))
    y)

Which would not generate any warning and would lead to more efficient
code when the &rest var is captured by a closure.


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Juanma Barranquero]

Speaking of unexpected warnings in lexical code:

  (let (new-list)
    (dolist (item list (nreverse new-list))
      (when (not (funcall predicate item))
	(setq new-list (cons item new-list)))))

In toplevel form:
doc-view.el:614:1:Warning: Unused lexical variable `item'
Wrote c:/emacs/trunk/lisp/doc-view.elc

but it does not happen if the RESULT of `dolist' is moved outside:

  (let (new-list)
    (dolist (item list)
      (when (not (funcall predicate item))
	(setq new-list (cons item new-list))))
     (nreverse new-list))

Apparently, the macroexpansion of `dolist' invokes RESULT as

(setq VAR nil)
RESULT

or, in the cl-macs `dolist', as

(let ((VAR nil))
  RESULT)

which I suppose has been doing for decades, but is still a bit
strange. The docstring for `dolist' does not say that VAR is set to
nil before computing RESULT.

If computing RESULT needed the last VAR, the current code precludes it
(unless it requires VAR to be nil, of course ;-)

And, if computing RESULT requieres an outside VAR, the programmer is
going to be forced to use this anyway:

(let ((VAR 'myval))
  (dolist (VAR mylist)
    ...)
  (compute-my-result VAR))  ;; with the let-bound VAR, not the dolist-bound one

so setting it to nil in the (dolist (VAR LIST RESULT) ...) case does
not bring any clear benefit, even in the non-lexical case.

Or am I missing something obvious?

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

> Apparently, the macroexpansion of `dolist' invokes RESULT as

> (setq VAR nil)
> RESULT

> or, in the cl-macs `dolist', as

> (let ((VAR nil))
>   RESULT)

> which I suppose has been doing for decades, but is still a bit
> strange.

That's how dolist/dotimes are defined in Common-Lisp, hence that's
how it works in cl-macs.  The subr versions also do it because when we
introduced them, some people brought up the issue and it seemed there
was no point in being incompatible.

> The docstring for `dolist' does not say that VAR is set to
> nil before computing RESULT.

Indeed.

> If computing RESULT needed the last VAR, the current code precludes it
> (unless it requires VAR to be nil, of course ;-)

Yes, I do find it very odd as well, but CLHS says clearly "At the time
result-form is processed, var is bound to nil".

> And, if computing RESULT requieres an outside VAR, the programmer is
> going to be forced to use this anyway:

> (let ((VAR 'myval))
>   (dolist (VAR mylist)
>     ...)
>   (compute-my-result VAR))  ;; with the let-bound VAR, not the dolist-bound one

Or she can just use different names rather than reusing the same
variable name.

> so setting it to nil in the (dolist (VAR LIST RESULT) ...) case does
> not bring any clear benefit, even in the non-lexical case.

Agreed.


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Juanma Barranquero]

On Thu, Jun 2, 2011 at 14:45, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> That's how dolist/dotimes are defined in Common-Lisp, hence that's
> how it works in cl-macs.  The subr versions also do it because when we
> introduced them, some people brought up the issue and it seemed there
> was no point in being incompatible.

I wonder what kind of code (other than a specific test) makes this
incompatibility noticeable in the first place... but I digress ;-)

> Yes, I do find it very odd as well, but CLHS says clearly "At the time
> result-form is processed, var is bound to nil".

Silly me, I should've checked the CLHS. I would still be puzzled, but
at least not by Emacs.

> Or she can just use different names rather than reusing the same
> variable name.

Yeah, that too.

>> so setting it to nil in the (dolist (VAR LIST RESULT) ...) case does
>> not bring any clear benefit, even in the non-lexical case.
>
> Agreed.

Well, at the very least that's something that could/should be
optimized away (at some point in the future, I mean) in the
lexical-binding case (I think we already have some macros that expand
to different code for lexical and non-lexical compilations?).

Meanwhile, will you complain if I change that particular case (in
doc-view.el) to be like

(let (...)
  (dolist (var list)
    ...)
  result)

to avoid the warning?

    Juanma

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Stefan Monnier]

> Meanwhile, will you complain if I change that particular case (in
> doc-view.el) to be like

> (let (...)
>   (dolist (var list)
>     ...)
>   result)

> to avoid the warning?

I don't know which particular case you're referring to, but I've already
installed such changes under the banner "silence compiler warnings".
So yes, by all means,


        Stefan

bug#8711: 24.0.50; binding _ to unused values with lexical-binding

[Juanma Barranquero]

On Thu, Jun 2, 2011 at 16:00, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I don't know which particular case you're referring to

The one I described earlier:

C:\emacs\trunk> bin\emacs.exe -batch -Q -f batch-byte-compile lisp\doc-view.el

In toplevel form:
doc-view.el:614:1:Warning: Unused lexical variable `item'
Wrote c:/emacs/trunk/lisp/doc-view.elc

where line 614 points to this function:

(defun doc-view-remove-if (predicate list)
  "Return LIST with all items removed that satisfy PREDICATE."
  (let (new-list)
    (dolist (item list (nreverse new-list))
      (when (not (funcall predicate item))
        (setq new-list (cons item new-list))))))


    Juanma

bug#8711: bug#26960: 26.0.50; Complaints about unused variable in cl-destructuring-bind

[Lars Ingebrigtsen]

Helmut Eller <eller.helmut@gmail.com> writes:

> There seems be no way to avoid a warning in code like this:
>
> ;; -*- lexical-binding: t -*-
> (eval-when-compile (require 'cl))
> (defun foo (x) (destructuring-bind (_) x))
> (defun bar (x) (destructuring-bind (_) x (ignore _)))
>
> In foo the compiler complains because _ is never used and
> it bar it complains because it is used.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I can reproduce this in Emacs 25.1, but not in Emacs 29.  That is,
there's no warning for `foo'.  There's still a warning for `bar', but
that's correct, isn't it?  `ignore' is just a normal function, so _ is
used there.

However, this example from a merged bug report is giving a warning:

> ;;; -*- lexical-binding: t; -*-
> (require 'cl-lib)
> (cl-destructuring-bind (&whole a b &rest _) '(1 2)
>   (print (list a b)))

And that seems like a bug?

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

bug#26960: 26.0.50; Complaints about unused variable in cl-destructuring-bind

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

> I can reproduce this in Emacs 25.1, but not in Emacs 29.  That is,
> there's no warning for `foo'.  There's still a warning for `bar', but
> that's correct, isn't it?

Yes.

> `ignore' is just a normal function, so _ is used there.

It's not 100% normal, but yes.

> However, this example from a merged bug report is giving a warning:
>
>> ;;; -*- lexical-binding: t; -*-
>> (require 'cl-lib)
>> (cl-destructuring-bind (&whole a b &rest _) '(1 2)
>>   (print (list a b)))
>
> And that seems like a bug?

Indeed.  If you look at the macroexpanded code, you see that
`cl-destructuring-bind` uses:

    (let* ((_ '(1 2))
           (a _)
           (b (if _ (pop _)
                (signal 'wrong-number-of-arguments
                        (list '(&whole a b &rest _) (length _))))))
      (print (list a b)))

so, you can see that the problem is that `cl-destructuring-bind` tries
to avoid using gensym and "abuses" the &rest var as the "iterator"
variable while parsing the list.

It might have been useful back in the dynbinding days because `setq`
might have been a bit cheaper than `let`, but with lexical scoping
`let` byte-compiles to virtually nothing.


        Stefan

bug#8711: bug#26960: 26.0.50; Complaints about unused variable in cl-destructuring-bind

[Lars Ingebrigtsen]

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

> Indeed.  If you look at the macroexpanded code, you see that
> `cl-destructuring-bind` uses:
>
>     (let* ((_ '(1 2))
>            (a _)
>            (b (if _ (pop _)
>                 (signal 'wrong-number-of-arguments
>                         (list '(&whole a b &rest _) (length _))))))
>       (print (list a b)))
>
> so, you can see that the problem is that `cl-destructuring-bind` tries
> to avoid using gensym and "abuses" the &rest var as the "iterator"
> variable while parsing the list.

Hm, right.

I had a quick peek at `cl--do-arglist' (which I guess is what's creating
this form), and...  I think I'll leave this for somebody else to fix.  🫠

In any case, I'm unmerging these two bug reports, and closing the one
that's fixed.

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

bug#26960: 26.0.50; Complaints about unused variable in cl-destructuring-bind

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

> I had a quick peek at `cl--do-arglist' (which I guess is what's creating
> this form), and...  I think I'll leave this for somebody else to fix.  🫠

I did the same some years ago, FWIW :-)


        Stefan