bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Michael Heerdegen]

Hello,

the latest changes in byte-opt.el seem to have broken some of my code.
It worked since yesterday, and it still works when interpreted, but the
compiled code raises this error today:

#+begin_src emacs-lisp
Debugger entered--Lisp error: (void-variable number)
  bbdb-anniv-diary-entries()
  alarm-clock-diary-list-entries-hook-fun-to-alarm-clock(bbdb-anniv-diary-entries #f(compiled-function (dt) #<bytecode 0x10302b42caa2d23>) #f(compiled-function (&rest args2) #<bytecode -0x159ba228e7b687fb>))
  #f(compiled-function () #<bytecode -0xe7267f5f0b223bb>)()
  [...]
#+end_src

`alarm-clock-diary-list-entries-hook-fun-to-alarm-clock' has this
definition:

#+begin_src emacs-lisp
(defun alarm-clock-diary-list-entries-hook-fun-to-alarm-clock (fun &optional date-fun filter-return)
  ;; For functions to put in `diary-list-entries-hook',
  ;; e.g. `bbdb-anniv-diary-entries'
  (require 'calendar)
  (require 'diary-lib)
  (with-suppressed-warnings ((lexical date original-date number))
    (defvar date)
    (defvar original-date)
    (defvar number))
  (defvar diary-entries-list)
  (let* ((date (funcall (or date-fun #'identity) (calendar-current-date)))
         (original-date date)
         (diary-entries-list '())
         (number 1))
    (funcall fun)
    (dolist (to-add (funcall (or filter-return #'identity)
                             (mapcar #'substring-no-properties (mapcar #'cadr diary-entries-list))))
      (alarm-clock-add-automatically-maybe
       to-add (list (alarm-clock-last-midnight))
       nil '((origin . diary-list-entries-hook-fun))))))
#+end_src

FUN is bound to `bbdb-anniv-diary-entries'.  `number' should be bound
dynamically when that function is `funcall'ed.

TIA,

Michael.


In GNU Emacs 28.0.50 (build 34, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-02-08 built on drachen
Repository revision: f0afbebe7c423ec8b1003f88af11122dfec28dd1
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Mattias Engdegård]

8 feb. 2021 kl. 18.29 skrev Michael Heerdegen <michael_heerdegen@web.de>:

> the latest changes in byte-opt.el seem to have broken some of my code.
> It worked since yesterday, and it still works when interpreted, but the
> compiled code raises this error today:

Intereresting, thank you! And sorry about that.

>  (with-suppressed-warnings ((lexical date original-date number))
>    (defvar date)
>    (defvar original-date)
>    (defvar number))

There's a lack of clarity regarding the exact semantics of local `defvar`. The manual says that its effect is confined to the current lexical scope, but what exactly does that mean?

As it turns out, parts of the implementation have different opinions about that. As you observed, the recently added optimisation on master takes a strict syntactic view: even a `progn` is a lexical scope, and `with-suppressed-warnings` wraps its body in a `progn`; thus your `defvar` declarations have no effect outside that construct.

However, the interpreter is at the other extreme end and takes a very dynamic view. For example:

(defun f (x)
  (if x (defvar my-var))
  (let ((my-var 17))
    (do-something)))

Here, whether my-var is a lexical or dynamic variable depends on the argument x! The compiler unsurprisingly is of more static persuasion but sadly hazy on the details. For example, the above function is compiled (in Emacs 27) with my-var lexically bound, but if we say

(defun g ()
  (if (= 1 2) (defvar my-var))
  (let ((my-var 17))
    (do-something)))

then my-var becomes dynamic. Ouch.

While we ponder over the problem, you may try separate with-suppressed-warnings for each variable. Ie,

  (with-suppressed-warnings ((lexical date))
    (defvar date))
  (with-suppressed-warnings ((lexical original-date))
    (defvar original-date))
  (with-suppressed-warnings ((lexical number))
    (defvar number))

hoping that each single-expression `progn` will rapidly decay into its confined expression (a defvar) and thus will be syntactically in the right lexical scope.

(Stefan, it looks like your latest Gnus patch may fall in the same trap. Or?)

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Stefan Monnier]

> There's a lack of clarity regarding the exact semantics of local
> `defvar`. The manual says that its effect is confined to the current
> lexical scope, but what exactly does that mean?

Adding a variable to the context creates a new scope.
A `progn` should not introduce a new scope.

> As it turns out, parts of the implementation have different opinions
> about that.

Indeed, there's a fair bit a space between my above two statements.

> As you observed, the recently added optimisation on master takes
> a strict syntactic view: even a `progn` is a lexical scope,

I think that's clearly an error.  Adding/removing `progn` shouldn't make
any difference in this respect.

> (Stefan, it looks like your latest Gnus patch may fall in the same trap. Or?)

The (with-suppressed-warnings (...) (defvar)) form is used at
several places.  It's the preferred way to declare a variable
dynamically scoped without incurring the "not prefixed" warning and
without making the `with-suppressed-warnings` silencer cover more code
than intended.


        Stefan

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Mattias Engdegård]

9 feb. 2021 kl. 16.31 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> Adding a variable to the context creates a new scope.
> A `progn` should not introduce a new scope.

All right, `defvar` modifies the current scope and only let, let* and lambda create new scopes. Fine, but it leaves questions unanswered.

* Does `defvar` affect new bindings only, or variable references in the current scope as well? In

 (let ((my-var EXPR))
   (defvar my-var)
   (use my-var))

does the last line refer to the lexical my-var bound in the first line, or to the dynamic my-var?

* Does the defvar have to be 'executed' to be effective? That's how the interpreter works, but it clearly can't work in the compiler. The defvar form probably has to 'precede' the binding form which it tries to affect, in some way.

> The (with-suppressed-warnings (...) (defvar)) form is used at
> several places.  It's the preferred way to declare a variable
> dynamically scoped without incurring the "not prefixed" warning and
> without making the `with-suppressed-warnings` silencer cover more code
> than intended.

Yes, but it does (currently) work if used on a single variable at a time, which is the suggested workaround for the time being.

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Stefan Monnier]

>> Adding a variable to the context creates a new scope.
>> A `progn` should not introduce a new scope.
>
> All right, `defvar` modifies the current scope and only let, let* and
>  lambda create new scopes. Fine, but it leaves questions unanswered.

Oh, yes.

> * Does `defvar` affect new bindings only, or variable references in the current scope as well?

Subsequent bindings.

>  (let ((my-var EXPR))
>    (defvar my-var)
>    (use my-var))

`defvar` here is "too late" to affect the earlier `let`.

> does the last line refer to the lexical my-var bound in the first
> line, or to the dynamic my-var?

If there's a lexical var in scope with the right name, that's what is
used, regardless if the variable is (currently) declared as dynamically
scoped or not.  E.g.

    (funcall (lambda (default-directory)
               (cd "/")
               default-directory)
             3)

returns 3 (in `lexical-binding` mode).

> * Does the defvar have to be 'executed' to be effective? That's how
> the interpreter works, but it clearly can't work in the compiler.

Your (if foo (defvar bar)) example indeed points to some of the other
murkiness, where the behavior differs between the interpreter and the
compiler, yes.

If it hurts, don't do that ;-)

> The defvar form probably has to 'precede' the binding form which it
> tries to affect, in some way.

A `defvar` should be effective on all subsequent equally or more
deeply-nested code.  Whether it also affects other subsequent code
is the less clear part.  `progn` (and macros to expand to `progn`, of
course) is the only "sure" exception (i.e. a `defvar` also affects the
code after its surrounding `progn`, if any).

>> The (with-suppressed-warnings (...) (defvar)) form is used at
>> several places.  It's the preferred way to declare a variable
>> dynamically scoped without incurring the "not prefixed" warning and
>> without making the `with-suppressed-warnings` silencer cover more code
>> than intended.
> Yes, but it does (currently) work if used on a single variable at a time,
> which is the suggested workaround for the time being.

It's better than nothing, but I think we really should fix the `progn`
case, because it's important.


        Stefan

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Michael Heerdegen]

Mattias Engdegård <mattiase@acm.org> writes:

> As it turns out, parts of the implementation have different opinions
> about that. As you observed, the recently added optimisation on master
> takes a strict syntactic view: even a `progn` is a lexical scope, and
> `with-suppressed-warnings` wraps its body in a `progn`; thus your
> `defvar` declarations have no effect outside that construct.

Seems this is the related code change (took quite a while until I found
that):

83983b6b7a115474572973b62eb5e42251713e63
Author:     Mattias Engdegård <mattiase@acm.org>
AuthorDate: Sat Feb 6 18:34:45 2021 +0100

 (defun byte-optimize-body (forms all-for-effect)
   ;; Optimize the cdr of a progn or implicit progn; all forms is a list of
@@ -590,6 +763,7 @@ byte-optimize-body
   ;; all-for-effect is true.  returns a new list of forms.
   (let ((rest forms)
 	(result nil)
+        (byte-optimize--dynamic-vars byte-optimize--dynamic-vars)
 	fe new)
     (while rest
       (setq fe (or all-for-effect (cdr rest)))

Should that line just be removed?

TIA,

Michael.

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Mattias Engdegård]

10 feb. 2021 kl. 04.20 skrev Michael Heerdegen <michael_heerdegen@web.de>:

> Should that line just be removed?

Almost, but not quite that simple. I'll fix it later today.

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Mattias Engdegård]

> Almost, but not quite that simple. I'll fix it later today.

A correction has now been pushed to master (f3ae26cb2ae). Does it help?

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Michael Heerdegen]

Mattias Engdegård <mattiase@acm.org> writes:

> A correction has now been pushed to master (f3ae26cb2ae). Does it
> help?

Yes, works as expected now, thanks.  And also thanks for working on that
optimization stuff.

Unless someone thinks otherwise, you might close.

Regards,

Michael.

bug#46387: 28.0.50; Compiled code making a variable dynamic stopped working

[Mattias Engdegård]

10 feb. 2021 kl. 23.44 skrev Michael Heerdegen <michael_heerdegen@web.de>:

> Yes, works as expected now, thanks.

Excellent, closing then.

>  And also thanks for working on that
> optimization stuff.

Low and uncertain payoff, high risk of becoming immensely unpopular by breaking everything in mysterious ways -- the dream job!