bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Vitalie Spinu]

Hello,

Compiling the following file:

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

    (let ((foo t))
      (defvar tmp-bar (if foo 0 1 -1)))

gives:

    tmp.el:4:23: Warning: reference to free variable ‘foo’


When loading the .elc file the error

    Debugger entered--Lisp error: (void-variable foo)
      byte-code("\301\10\203\11\0\302\202\n\0\303!\207" [foo (lambda
(def-tmp-var) (defvar tmp-bar def-tmp-var)) 0 -1] 2)
      load(".../tmp.elc" nil nil t)


No problems with emacs-27 branch.


  Vitalie



In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.22.30, cairo version 1.15.10)
 of 2021-03-03 built on NB277
Repository revision: a350ae058caedcb7be7d332564817954e3624e60
Repository branch: HEAD
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Ubuntu 18.04.5 LTS

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Joseph Mingrone]

On Thu, 2021-03-04 at 01:14, Vitalie Spinu <spinuvit@gmail.com> wrote:

> Hello,

> Compiling the following file:

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

>     (let ((foo t))
>       (defvar tmp-bar (if foo 0 1 -1)))

> gives:

>     tmp.el:4:23: Warning: reference to free variable ‘foo’


> When loading the .elc file the error

>     Debugger entered--Lisp error: (void-variable foo)
>       byte-code("\301\10\203\11\0\302\202\n\0\303!\207" [foo (lambda
> (def-tmp-var) (defvar tmp-bar def-tmp-var)) 0 -1] 2)
>       load(".../tmp.elc" nil nil t)


> No problems with emacs-27 branch.


>   Vitalie



> In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
> 3.22.30, cairo version 1.15.10)
>  of 2021-03-03 built on NB277
> Repository revision: a350ae058caedcb7be7d332564817954e3624e60
> Repository branch: HEAD
> Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
> System Description: Ubuntu 18.04.5 LTS

Hi,

With

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

     (let ((foo t))
       (defvar tmp-bar foo))

the same warning is reported when compiling, but when loading the .elc
file, the debugger isn't started.  Just

     load: Symbol’s value as variable is void: foo

is reported.

With

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

     (defvar foo
       (let ((bar t))
        (if bar 1 0)))

The .elc loads without errors.

Joe

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

> With
>
>      ;;; -*- lexical-binding: t -*-
>
>      (let ((foo t))
>        (defvar tmp-bar foo))
>
> the same warning is reported when compiling, but when loading the .elc
> file, the debugger isn't started.  Just
>
>      load: Symbol’s value as variable is void: foo
>
> is reported.

I can reproduce this problem here, too (and Emacs 27 doesn't have this
problem).

I have not tried to bisect this, but since Pip pushed a change related
to this a couple days ago, I've added Pip to the CCs.

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

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Joseph Mingrone]

On Thu, 2021-03-04 at 21:09, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Joseph Mingrone <jrm@ftfl.ca> writes:

>> With

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

>>      (let ((foo t))
>>        (defvar tmp-bar foo))

>> the same warning is reported when compiling, but when loading the .elc
>> file, the debugger isn't started.  Just

>>      load: Symbol’s value as variable is void: foo

>> is reported.

> I can reproduce this problem here, too (and Emacs 27 doesn't have this
> problem).

> I have not tried to bisect this, but since Pip pushed a change related
> to this a couple days ago, I've added Pip to the CCs.

I think the problem was introduced before 9b4a2dd (2021-02-14).  That
was the last snapshot that I was running and I'm pretty sure I saw the
problem then.  If I can find time tonight, I'll try to confirm that.

Joe

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Lars Ingebrigtsen]

Joseph Mingrone <jrm@ftfl.ca> writes:

> I think the problem was introduced before 9b4a2dd (2021-02-14).  That
> was the last snapshot that I was running and I'm pretty sure I saw the
> problem then.  If I can find time tonight, I'll try to confirm that.

Great; bisecting this would be very helpful.

Pip Cet <pipcet@gmail.com> writes:

> I can reproduce it, three, but I can also reproduce it with the change
> I think you're referring to reverted.

Sorry about that; it was a stab in the dark using the "the one who
touched it last" principle.  :-/

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

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

On Thu, Mar 4, 2021 at 8:48 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Joseph Mingrone <jrm@ftfl.ca> writes:
> > I think the problem was introduced before 9b4a2dd (2021-02-14).  That
> > was the last snapshot that I was running and I'm pretty sure I saw the
> > problem then.  If I can find time tonight, I'll try to confirm that.
>
> Great; bisecting this would be very helpful.

Absolutely.

> Pip Cet <pipcet@gmail.com> writes:
>
> > I can reproduce it, three, but I can also reproduce it with the change
> > I think you're referring to reverted.
>
> Sorry about that; it was a stab in the dark using the "the one who
> touched it last" principle.  :-/

Not a problem at all. In fact, I've taken a look and it seems
byte-optimize-form-code-walker simply ignores the value form in a
defvar now..

So I suspect it's commit 83983b6b7a115474572973b62eb5e42251713e63, I'm
cc'ing Mattias, and I suspect this patch might help (but I've only
done one quick test).

Pip

[-- Attachment #2: 0001-Don-t-ignore-lexically-bound-variables-in-a-defvar-b.patch --]
[-- Type: text/x-patch, Size: 974 bytes --]

From c39ecf1c053f551a4b384d2ad2994fe0e58d67b0 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Thu, 4 Mar 2021 21:06:13 +0000
Subject: [PATCH] Don't ignore lexically-bound variables in a defvar
 (bug#46912)

* lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker): Walk
the value form of a defvar.
---
 lisp/emacs-lisp/byte-opt.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index b51ba801552d6..ba2165a5d59ce 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -607,7 +607,9 @@ byte-optimize-form-code-walker
            (setq args (cddr args)))
          (cons fn (nreverse var-expr-list))))
 
-      (`(defvar ,(and (pred symbolp) name) . ,_)
+      (`(defvar ,(and (pred symbolp) name) . ,rest)
+       (when rest
+         (byte-optimize-form (car rest) nil))
        (push name byte-optimize--dynamic-vars)
        form)
 
-- 
2.30.1

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Joseph Mingrone]

On Thu, 2021-03-04 at 21:08, Pip Cet <pipcet@gmail.com> wrote:

> On Thu, Mar 4, 2021 at 8:48 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>> Joseph Mingrone <jrm@ftfl.ca> writes:
>> > I think the problem was introduced before 9b4a2dd (2021-02-14).  That
>> > was the last snapshot that I was running and I'm pretty sure I saw the
>> > problem then.  If I can find time tonight, I'll try to confirm that.

>> Great; bisecting this would be very helpful.

> Absolutely.

>> Pip Cet <pipcet@gmail.com> writes:

>> > I can reproduce it, three, but I can also reproduce it with the change
>> > I think you're referring to reverted.

>> Sorry about that; it was a stab in the dark using the "the one who
>> touched it last" principle.  :-/

> Not a problem at all. In fact, I've taken a look and it seems
> byte-optimize-form-code-walker simply ignores the value form in a
> defvar now..

> So I suspect it's commit 83983b6b7a115474572973b62eb5e42251713e63, I'm
> cc'ing Mattias, and I suspect this patch might help (but I've only
> done one quick test).

> Pip

With this patch applied to 20596a5, I still see the problems Vitalie
reported.

Joe

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Pip Cet]

[-- Attachment #1: Type: text/plain, Size: 253 bytes --]

On Thu, Mar 4, 2021 at 11:15 PM Joseph Mingrone <jrm@ftfl.ca> wrote:
> With this patch applied to 20596a5, I still see the problems Vitalie
> reported.

Strange, I don't see how I managed to convince myself it works here.
Anyway, this should work.

Pip

[-- Attachment #2: 0001-Don-t-ignore-lexically-bound-variables-in-a-defvar-b.patch --]
[-- Type: text/x-patch, Size: 1286 bytes --]

From 09a195ed2ed7004c54d1701d0017a4fb10f6cbce Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Thu, 4 Mar 2021 21:06:13 +0000
Subject: [PATCH] Don't ignore lexically-bound variables in a defvar
 (bug#46912)

* lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker): Walk
the value form of a defvar.
---
 lisp/emacs-lisp/byte-opt.el | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index b51ba801552d6..b3325816c5c97 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -607,9 +607,12 @@ byte-optimize-form-code-walker
            (setq args (cddr args)))
          (cons fn (nreverse var-expr-list))))
 
-      (`(defvar ,(and (pred symbolp) name) . ,_)
-       (push name byte-optimize--dynamic-vars)
-       form)
+      (`(defvar ,(and (pred symbolp) name) . ,rest)
+       (let ((optimized-rest (and rest
+                                  (cons (byte-optimize-form (car rest) nil)
+                                        (cdr rest)))))
+         (push name byte-optimize--dynamic-vars)
+         `(defvar ,name . ,optimized-rest)))
 
       (`(,(pred byte-code-function-p) . ,exps)
        (cons fn (mapcar #'byte-optimize-form exps)))
-- 
2.30.1

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Joseph Mingrone]

On Fri, 2021-03-05 at 02:52, Pip Cet <pipcet@gmail.com> wrote:

> On Thu, Mar 4, 2021 at 11:15 PM Joseph Mingrone <jrm@ftfl.ca> wrote:
>> With this patch applied to 20596a5, I still see the problems Vitalie
>> reported.

> Strange, I don't see how I managed to convince myself it works here.
> Anyway, this should work.

> Pip

Hi Pip,

I can confirm that your second patch fixes the problem.

Thank you!

Joe

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Vitalie Spinu]

Hi Pip,

The second patch fixed it for me as well. Thanks!

  Vitalie

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Mattias Engdegård]

4 mars 2021 kl. 22.08 skrev Pip Cet <pipcet@gmail.com>:

> Not a problem at all. In fact, I've taken a look and it seems
> byte-optimize-form-code-walker simply ignores the value form in a
> defvar now..

Oh dear, what a silly mistake. Thank you for finding it so quickly, and that (last, corrected) patch of yours looks sound -- please push it right away.

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Pip Cet]

On Fri, Mar 5, 2021 at 9:11 AM Mattias Engdegård <mattiase@acm.org> wrote:
> 4 mars 2021 kl. 22.08 skrev Pip Cet <pipcet@gmail.com>:
>
> > Not a problem at all. In fact, I've taken a look and it seems
> > byte-optimize-form-code-walker simply ignores the value form in a
> > defvar now..
>
> Oh dear, what a silly mistake. Thank you for finding it so quickly, and that (last, corrected) patch of yours looks sound -- please push it right away.

Done, and closing. Sorry about the first patch, I must have "tested"
it after setting byte-optimize to nil...

Pip

bug#46912: 28.0.50; Byte compiled defvar with lexical binding throws (void-variable foo)

[Pip Cet]

On Thu, Mar 4, 2021 at 8:10 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Joseph Mingrone <jrm@ftfl.ca> writes:
> > With
> >
> >      ;;; -*- lexical-binding: t -*-
> >
> >      (let ((foo t))
> >        (defvar tmp-bar foo))
> >
> > the same warning is reported when compiling, but when loading the .elc
> > file, the debugger isn't started.  Just
> >
> >      load: Symbol’s value as variable is void: foo
> >
> > is reported.
>
> I can reproduce this problem here, too (and Emacs 27 doesn't have this
> problem).

I can reproduce it, three, but I can also reproduce it with the change
I think you're referring to reverted.

> I have not tried to bisect this, but since Pip pushed a change related
> to this a couple days ago, I've added Pip to the CCs.

Thanks! I must confess I don't know whether I need to do anything to
make the savannah.gnu.org email address work, but then I need to get a
better email address anyway...

Pip (at mumble, mumble, yes, I do realize that's a non-free email service)