bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[David Fiander]

Create a file called "bind-test.el" with the contents:
---
(use-package dired
     :bind ("C-c d" . dired-at-point))
---

Compile this file.

Run emacs -Q and then

     (load-file "bind-test.elc")

See the error message
The error message

     ⛔ Error (use-package): dired/:catch: Symbol’s value as variable is 
void: personal-keybindings

appears.


In GNU Emacs 29.1 (build 1, x86_64-unknown-freebsd13.2, GTK+ Version
3.24.34, cairo version 1.17.4) of 2023-07-25 built on
victoria.l.fiander.info
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: 13.2-RELEASE-p1

Configured using:
'configure --with-xwidgets'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBOTF LIBXML2 M17N_FLT MODULES NOTIFY KQUEUE PDUMPER PNG RSVG
SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2
XPM XWIDGETS GTK3 ZLIB

Important settings:
value of $LANG: C.UTF-8
locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
paredit-mode: t
server-mode: t
savehist-mode: t
desktop-save-mode: t
fido-vertical-mode: t
icomplete-vertical-mode: t
icomplete-mode: t
fido-mode: t
global-company-mode: t
company-mode: t
global-auto-revert-mode: t
midnight-mode: t
delete-selection-mode: t
auto-insert-mode: t
display-time-mode: t
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
prettify-symbols-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
column-number-mode: t
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t

Load-path shadows:
/home/david/.config/emacs/elpa/transient-20230602.2121/transient hides 
/usr/local/share/emacs/29.1/lisp/transient

Features:
(shadow sort mail-extr emacsbug message yank-media puny dired-x dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util text-property-search mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils time-date paredit edmacro
kmacro server savehist desktop frameset diminish modus-vivendi-theme
modus-themes shell-here icomplete company-oddmuse company-keywords
company-etags etags fileloop generator xref project ring company-gtags
company-dabbrev-code company-dabbrev company-files company-clang
company-capf company-cmake company-semantic company-template
company-bbdb company file-patterns ffap thingatpt autorevert filenotify
midnight delsel autoinsert time avoid diminish-autoloads pcase
slime-autoloads info package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl tooltip
cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow
isearch easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads xwidget-internal dbusbind
kqueue lcms2 dynamic-setting system-font-setting font-render-setting
cairo move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 177418 124626)
(symbols 48 13784 1)
(strings 32 41749 1992)
(string-bytes 1 1306786)
(vectors 16 20978)
(vector-slots 8 279162 6216)
(floats 8 99 141)
(intervals 56 320 0)
(buffers 976 12))

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> Date: Thu, 27 Jul 2023 09:51:35 -0500
> From: David Fiander <david@fiander.info>
> 
> Create a file called "bind-test.el" with the contents:
> ---
> (use-package dired
>      :bind ("C-c d" . dired-at-point))
> ---
> 
> Compile this file.
> 
> Run emacs -Q and then
> 
>      (load-file "bind-test.elc")
> 
> See the error message
> The error message
> 
>      ⛔ Error (use-package): dired/:catch: Symbol’s value as variable is 
> void: personal-keybindings
> 
> appears.

I cannot reproduce this.  I get no errors, and the binding in :bind is
performed.

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[David Fiander]

How very odd. I'm running on a pretty much vanilla 29.1RC build, and 
have confirmed this on both my main FreeBSD 13.2 machine and on Ubuntu 
LTS 22.04. Loading the source file works, but loading the complied file 
doesn't.

- David

On 2023-07-27 11:52, Eli Zaretskii wrote:
>> Date: Thu, 27 Jul 2023 09:51:35 -0500
>> From: David Fiander <david@fiander.info>
>>
>> Create a file called "bind-test.el" with the contents:
>> ---
>> (use-package dired
>>       :bind ("C-c d" . dired-at-point))
>> ---
>>
>> Compile this file.
>>
>> Run emacs -Q and then
>>
>>       (load-file "bind-test.elc")
>>
>> See the error message
>> The error message
>>
>>       ⛔ Error (use-package): dired/:catch: Symbol’s value as variable is
>> void: personal-keybindings
>>
>> appears.
> 
> I cannot reproduce this.  I get no errors, and the binding in :bind is
> performed.

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> Date: Thu, 27 Jul 2023 12:03:10 -0500
> Cc: 64901@debbugs.gnu.org
> From: David Fiander <david@fiander.info>
> 
> How very odd. I'm running on a pretty much vanilla 29.1RC build, and 
> have confirmed this on both my main FreeBSD 13.2 machine and on Ubuntu 
> LTS 22.04. Loading the source file works, but loading the complied file 
> doesn't.

Sorry, my bad.  If I try loading a byte-compiled file, then yes, I see
the error.

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> Cc: 64901@debbugs.gnu.org
> Date: Thu, 27 Jul 2023 20:12:16 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Thu, 27 Jul 2023 12:03:10 -0500
> > Cc: 64901@debbugs.gnu.org
> > From: David Fiander <david@fiander.info>
> > 
> > How very odd. I'm running on a pretty much vanilla 29.1RC build, and 
> > have confirmed this on both my main FreeBSD 13.2 machine and on Ubuntu 
> > LTS 22.04. Loading the source file works, but loading the complied file 
> > doesn't.
> 
> Sorry, my bad.  If I try loading a byte-compiled file, then yes, I see
> the error.

John, can you help me out here: why is the byte-compiled file in the
OP's recipe fail to load?

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2023-07-28 09:56 +0300] wrote:

>> Cc: 64901@debbugs.gnu.org
>> Date: Thu, 27 Jul 2023 20:12:16 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > Date: Thu, 27 Jul 2023 12:03:10 -0500
>> > Cc: 64901@debbugs.gnu.org
>> > From: David Fiander <david@fiander.info>
>> > 
>> > How very odd. I'm running on a pretty much vanilla 29.1RC build, and 
>> > have confirmed this on both my main FreeBSD 13.2 machine and on Ubuntu 
>> > LTS 22.04. Loading the source file works, but loading the complied file 
>> > doesn't.
>> 
>> Sorry, my bad.  If I try loading a byte-compiled file, then yes, I see
>> the error.
>
> John, can you help me out here: why is the byte-compiled file in the
> OP's recipe fail to load?

I'm not John, but macroexpanding the use-package call (specifically the
subexpression that calls the autoloaded bind-keys macro) reveals that it
indeed refers to the bind-key.el variable personal-keybindings.

My guess is that bind-key.el is loaded as expected for the autoloaded
macro call during interpretation/compilation, but once macroexpanded,
there is nothing left to load bind-key.el at runtime.

So (require 'bind-key) is currently needed for byte-compiling such
use-package keywords.

HTH,
-- 
Basil

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Cc: John Wiegley <johnw@gnu.org>,  64901@debbugs.gnu.org,  david@fiander.info
> Date: Fri, 28 Jul 2023 11:28:35 +0200
> 
> Eli Zaretskii [2023-07-28 09:56 +0300] wrote:
> 
> I'm not John, but macroexpanding the use-package call (specifically the
> subexpression that calls the autoloaded bind-keys macro) reveals that it
> indeed refers to the bind-key.el variable personal-keybindings.
> 
> My guess is that bind-key.el is loaded as expected for the autoloaded
> macro call during interpretation/compilation, but once macroexpanded,
> there is nothing left to load bind-key.el at runtime.
> 
> So (require 'bind-key) is currently needed for byte-compiling such
> use-package keywords.

Thanks.

I'd like us to remove that restriction (and the need for the
workaround).

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii [2023-07-28 15:12 +0300] wrote:

>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>> Cc: John Wiegley <johnw@gnu.org>,  64901@debbugs.gnu.org,  david@fiander.info
>> Date: Fri, 28 Jul 2023 11:28:35 +0200
>> 
>> So (require 'bind-key) is currently needed for byte-compiling such
>> use-package keywords.
>
> I'd like us to remove that restriction (and the need for the
> workaround).

It looks like a pretty pervasive restriction in bind-key.el: all of its
autoloads are macros, and all of them expand to reference non-autoloaded
bind-key.el definitions.  E.g. try byte-compiling (unbind-key "💥").

So I think the library was designed to be used with (require 'bind-key).

Of course, I think it would be better to lift that restriction, so that
the library's autoloads are not redundant.  But maybe John or a user of
bind-key.el should chime in.

-- 
Basil

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[John Wiegley]

I have no further notes to add, but I would like to see that restriction lifted also.

John

On Fri, Jul 28, 2023, at 9:55 AM, Basil L. Contovounesios wrote:
> Eli Zaretskii [2023-07-28 15:12 +0300] wrote:
>
>>> From: "Basil L. Contovounesios" <contovob@tcd.ie>
>>> Cc: John Wiegley <johnw@gnu.org>,  64901@debbugs.gnu.org,  david@fiander.info
>>> Date: Fri, 28 Jul 2023 11:28:35 +0200
>>> 
>>> So (require 'bind-key) is currently needed for byte-compiling such
>>> use-package keywords.
>>
>> I'd like us to remove that restriction (and the need for the
>> workaround).
>
> It looks like a pretty pervasive restriction in bind-key.el: all of its
> autoloads are macros, and all of them expand to reference non-autoloaded
> bind-key.el definitions.  E.g. try byte-compiling (unbind-key "💥").
>
> So I think the library was designed to be used with (require 'bind-key).
>
> Of course, I think it would be better to lift that restriction, so that
> the library's autoloads are not redundant.  But maybe John or a user of
> bind-key.el should chime in.
>
> -- 
> Basil

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> Date: Fri, 28 Jul 2023 10:37:48 -0700
> From: "John Wiegley" <johnw@gnu.org>
> Cc: 64901@debbugs.gnu.org, david@fiander.info
> 
> I have no further notes to add, but I would like to see that restriction lifted also.

Any suggestions for how should we go about that?

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> I have no further notes to add, but I would like to see that restriction
>> lifted also.

> Any suggestions for how should we go about that?

I would suggest that in the macro ‘bind-key’, we emit ‘(require ’bind-key)’. I
can see now what an oversight this was, since indeed the macro will omit code
that references a variable that may not be in scope after direct compilation:

(defmacro bind-key (key-name command &optional keymap predicate)
  (let ((namevar (make-symbol "name"))
        (keyvar (make-symbol "key"))
        (kmapvar (make-symbol "kmap"))
        (kdescvar (make-symbol "kdesc"))
        (bindingvar (make-symbol "binding")))
    `(let* ((,namevar ,key-name)
            (,keyvar ,(if (stringp key-name) (read-kbd-macro key-name)
                        `(if (vectorp ,namevar) ,namevar
                           (read-kbd-macro ,namevar))))
            (,kmapvar (or (if (and ,keymap (symbolp ,keymap))
                              (symbol-value ,keymap) ,keymap)
                          global-map))
            (,kdescvar (cons (if (stringp ,namevar) ,namevar
                               (key-description ,namevar))
                             (if (symbolp ,keymap) ,keymap (quote ,keymap))))
            (,bindingvar (lookup-key ,kmapvar ,keyvar)))
       (let ((entry (assoc ,kdescvar personal-keybindings))
             (details (list ,command
                            (unless (numberp ,bindingvar)
                              ,bindingvar))))
         (if entry
             (setcdr entry details)
           (add-to-list 'personal-keybindings (cons ,kdescvar details))))
       ,(if predicate
            `(define-key ,kmapvar ,keyvar
               '(menu-item "" nil :filter (lambda (&optional _)
                                            (when ,predicate
                                              ,command))))
          `(define-key ,kmapvar ,keyvar ,command)))))

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> From: John Wiegley <johnw@gnu.org>
> Cc: contovob@tcd.ie,  64901@debbugs.gnu.org,  david@fiander.info
> Date: Tue, 01 Aug 2023 14:37:51 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I have no further notes to add, but I would like to see that restriction
> >> lifted also.
> 
> > Any suggestions for how should we go about that?
> 
> I would suggest that in the macro ‘bind-key’, we emit ‘(require ’bind-key)’. I
> can see now what an oversight this was, since indeed the macro will omit code
> that references a variable that may not be in scope after direct compilation:

Thanks.  Is the below what you had in mind?

diff --git a/lisp/use-package/bind-key.el b/lisp/use-package/bind-key.el
index 0ab72ea..4affd8f 100644
--- a/lisp/use-package/bind-key.el
+++ b/lisp/use-package/bind-key.el
@@ -180,6 +180,7 @@ bind-key
 Emacs can evaluate this form at any time that it does redisplay
 or operates on menu data structures, so you should write it so it
 can safely be called at any time."
+  (require 'bind-key)
   (let ((namevar (make-symbol "name"))
         (keyvar (make-symbol "key"))
         (kmapvar (make-symbol "kmap"))

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> Thanks.  Is the below what you had in mind?
> 
> diff --git a/lisp/use-package/bind-key.el b/lisp/use-package/bind-key.el
> index 0ab72ea..4affd8f 100644
> --- a/lisp/use-package/bind-key.el
> +++ b/lisp/use-package/bind-key.el
> @@ -180,6 +180,7 @@ bind-key
>  Emacs can evaluate this form at any time that it does redisplay
>  or operates on menu data structures, so you should write it so it
>  can safely be called at any time."
> +  (require 'bind-key)
>    (let ((namevar (make-symbol "name"))
>          (keyvar (make-symbol "key"))
>          (kmapvar (make-symbol "kmap"))

That’s too early, it would be:

  (let ((namevar (make-symbol "name"))
        (keyvar (make-symbol "key"))
        (kmapvar (make-symbol "kmap"))
        (kdescvar (make-symbol "kdesc"))
        (bindingvar (make-symbol "binding")))
    `(let* ((,namevar ,key-name)
            (,keyvar ,(if (stringp key-name) (read-kbd-macro key-name)
                        `(if (vectorp ,namevar) ,namevar
                           (read-kbd-macro ,namevar))))
            (,kmapvar (or (if (and ,keymap (symbolp ,keymap))
                              (symbol-value ,keymap) ,keymap)
                          global-map))
            (,kdescvar (cons (if (stringp ,namevar) ,namevar
                               (key-description ,namevar))
                             (if (symbolp ,keymap) ,keymap (quote ,keymap))))
            (,bindingvar (lookup-key ,kmapvar ,keyvar)))
       (require 'bind-key)      ; ensure `personal-keybindings' is in scope
       (let ((entry (assoc ,kdescvar personal-keybindings))
             (details (list ,command
                            (unless (numberp ,bindingvar)
                              ,bindingvar))))
         (if entry
             (setcdr entry details)
           (add-to-list 'personal-keybindings (cons ,kdescvar details))))
       ,(if predicate
            `(define-key ,kmapvar ,keyvar
               '(menu-item "" nil :filter (lambda (&optional _)
                                            (when ,predicate
                                              ,command))))
          `(define-key ,kmapvar ,keyvar ,command))))

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#64901: 29.1; use-package fails to require bind-key at runtime in compiled files

[Eli Zaretskii]

> From: John Wiegley <johnw@gnu.org>
> Cc: contovob@tcd.ie,  64901@debbugs.gnu.org,  david@fiander.info
> Date: Thu, 03 Aug 2023 15:16:39 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks.  Is the below what you had in mind?
> > 
> > diff --git a/lisp/use-package/bind-key.el b/lisp/use-package/bind-key.el
> > index 0ab72ea..4affd8f 100644
> > --- a/lisp/use-package/bind-key.el
> > +++ b/lisp/use-package/bind-key.el
> > @@ -180,6 +180,7 @@ bind-key
> >  Emacs can evaluate this form at any time that it does redisplay
> >  or operates on menu data structures, so you should write it so it
> >  can safely be called at any time."
> > +  (require 'bind-key)
> >    (let ((namevar (make-symbol "name"))
> >          (keyvar (make-symbol "key"))
> >          (kmapvar (make-symbol "kmap"))
> 
> That’s too early, it would be:

Thanks, installed, and closing the bug.