Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Eli Zaretskii]

Michael,

The changes in the above commit cause the following warning when
compiling simple.el:

  In read-passwd-toggle-visibility:
  simple.el:10880:18: Warning: reference to free variable `icon-preference'

The usual knee-jerk reaction of adding a defvar for the variable
didn't help (in fact, it made the situation worse), so please find a
way of shutting up this warning somehow.  I'm guessing this is due to
a somewhat unusual method of adding icons dependency to simple.el,
which (rightfully) attempts to avoid preloading icons.el, but I'm not
sure how to fix the warning in this situation.  Perhaps Stefan could
have some ideas?

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> Michael,

Hi Eli,

> The changes in the above commit cause the following warning when
> compiling simple.el:
>
>   In read-passwd-toggle-visibility:
>   simple.el:10880:18: Warning: reference to free variable `icon-preference'

I don't see this error, when running 'touch lisp/simple.el; make'. What
do I miss?

Best regards, Michael.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Fri, 15 Mar 2024 09:23:53 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Michael,
> 
> Hi Eli,
> 
> > The changes in the above commit cause the following warning when
> > compiling simple.el:
> >
> >   In read-passwd-toggle-visibility:
> >   simple.el:10880:18: Warning: reference to free variable `icon-preference'
> 
> I don't see this error, when running 'touch lisp/simple.el; make'. What
> do I miss?

Nothing, I guess, not in the recipe: using the same steps I see the
warning each time.

Maybe the way Emacs is configured affects that?  I see this on
MS-Windows, FWIW.

Or maybe I have some outdated .elc file?  I regenerated loaddefs.el,
but that didn't help.  I've byte-compiled icons.el as well, also
didn't help.  What other file(s) can be relevant?

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Andreas Schwab]

On Mär 15 2024, Michael Albinus wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Michael,
>
> Hi Eli,
>
>> The changes in the above commit cause the following warning when
>> compiling simple.el:
>>
>>   In read-passwd-toggle-visibility:
>>   simple.el:10880:18: Warning: reference to free variable `icon-preference'
>
> I don't see this error, when running 'touch lisp/simple.el; make'. What
> do I miss?

Please try rm lisp/simple.elc.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

Andreas Schwab <schwab@linux-m68k.org> writes:

>>> The changes in the above commit cause the following warning when
>>> compiling simple.el:
>>>
>>>   In read-passwd-toggle-visibility:
>>>   simple.el:10880:18: Warning: reference to free variable `icon-preference'
>>
>> I don't see this error, when running 'touch lisp/simple.el; make'. What
>> do I miss?
>
> Please try rm lisp/simple.elc.

The same, no warning. And also on my Windows 10 VM:

--8<---------------cut here---------------start------------->8---
albinus@win10 MINGW64 ~/src/emacs
$ rm lisp/simple.elc; make
make actual-all || make advice-on-failure make-target=all exit-status=$?
make[1]: Entering directory '/home/albinus/src/emacs'
make -C lib all
make[2]: Entering directory '/home/albinus/src/emacs/lib'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/albinus/src/emacs/lib'
make -C nt all
make[2]: Entering directory '/home/albinus/src/emacs/nt'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/albinus/src/emacs/nt'
make -C lib-src all
make[2]: Entering directory '/home/albinus/src/emacs/lib-src'
make[2]: Nothing to be done for 'all'.
make[2]: Leaving directory '/home/albinus/src/emacs/lib-src'
make -C src BIN_DESTDIR=''/mingw64/bin/'' \
         ELN_DESTDIR='/mingw64/lib/emacs/30.0.50/' all
make[2]: Entering directory '/home/albinus/src/emacs/src'
make -C ../nt ../src/emacs.res
make[3]: Entering directory '/home/albinus/src/emacs/nt'
make[3]: '../src/emacs.res' is up to date.
make[3]: Leaving directory '/home/albinus/src/emacs/nt'
make -C ../admin/charsets all
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
make -C ../admin/unidata charscript.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'charscript.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/unidata emoji-zwj.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'emoji-zwj.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/unidata all EMACS="../../src/bootstrap-emacs.exe"
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/charsets cp51932.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'cp51932.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
make -C ../admin/charsets eucjp-ms.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'eucjp-ms.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
  ELC+ELN  ../lisp/simple.elc
rm -f emacs.exe && cp -f temacs.exe emacs.exe
LC_ALL=C ./temacs -batch  -l loadup --temacs=pdump \
        --bin-dest /mingw64/bin/ --eln-dest /mingw64/lib/emacs/30.0.50/
Loading loadup.el (source)...
Dump mode: pdump
Using load-path (c:/msys64/home/albinus/src/emacs/lisp)
Loading emacs-lisp/debug-early (native compiled elisp)...
Loading emacs-lisp/byte-run (native compiled elisp)...
Loading emacs-lisp/backquote (native compiled elisp)...
Loading subr (native compiled elisp)...
Loading keymap (native compiled elisp)...
Loading version (native compiled elisp)...
Loading widget (native compiled elisp)...
Loading custom (native compiled elisp)...
Loading emacs-lisp/map-ynp (native compiled elisp)...
Loading international/mule (native compiled elisp)...
Loading international/mule-conf (native compiled elisp)...
Loading env (native compiled elisp)...
Loading format (native compiled elisp)...
Loading bindings (native compiled elisp)...
Loading window (native compiled elisp)...
Loading files (native compiled elisp)...
Loading emacs-lisp/macroexp (native compiled elisp)...
Loading cus-face (native compiled elisp)...
Loading faces (native compiled elisp)...
Loading loaddefs...
Loading theme-loaddefs.el (source)...
Loading button (native compiled elisp)...
Loading emacs-lisp/cl-preloaded (native compiled elisp)...
Loading emacs-lisp/oclosure (native compiled elisp)...
Loading obarray (native compiled elisp)...
Loading abbrev (native compiled elisp)...
Loading help (native compiled elisp)...
Loading jka-cmpr-hook (native compiled elisp)...
Loading epa-hook (native compiled elisp)...
Loading international/mule-cmds (native compiled elisp)...
Loading case-table (native compiled elisp)...
Loading international/charprop.el (source)...
Loading international/characters (native compiled elisp)...
Loading international/charscript (native compiled elisp)...
Loading international/emoji-zwj (native compiled elisp)...
Loading composite (native compiled elisp)...
Loading language/chinese (native compiled elisp)...
Loading language/cyrillic (native compiled elisp)...
Loading language/indian (native compiled elisp)...
Loading language/sinhala (native compiled elisp)...
Loading language/english (native compiled elisp)...
Loading language/ethiopic (native compiled elisp)...
Loading language/european (native compiled elisp)...
Loading language/czech (native compiled elisp)...
Loading language/slovak (native compiled elisp)...
Loading language/romanian (native compiled elisp)...
Loading language/greek (native compiled elisp)...
Loading language/hebrew (native compiled elisp)...
Loading international/cp51932 (native compiled elisp)...
Loading international/eucjp-ms (native compiled elisp)...
Loading language/japanese (native compiled elisp)...
Loading language/korean (native compiled elisp)...
Loading language/lao (native compiled elisp)...
Loading language/tai-viet (native compiled elisp)...
Loading language/thai (native compiled elisp)...
Loading language/tibetan (native compiled elisp)...
Loading language/vietnamese (native compiled elisp)...
Loading language/misc-lang (native compiled elisp)...
Loading language/utf-8-lang (native compiled elisp)...
Loading language/georgian (native compiled elisp)...
Loading language/khmer (native compiled elisp)...
Loading language/burmese (native compiled elisp)...
Loading language/cham (native compiled elisp)...
Loading language/philippine (native compiled elisp)...
Loading language/indonesian (native compiled elisp)...
Loading indent (native compiled elisp)...
Loading emacs-lisp/cl-generic (native compiled elisp)...
Loading simple (native compiled elisp)...
Loading emacs-lisp/seq (native compiled elisp)...
Loading emacs-lisp/nadvice (native compiled elisp)...
Loading minibuffer (native compiled elisp)...
Loading frame (native compiled elisp)...
Loading startup (native compiled elisp)...
Loading term/tty-colors (native compiled elisp)...
Loading font-core (native compiled elisp)...
Loading emacs-lisp/syntax (native compiled elisp)...
Loading font-lock (native compiled elisp)...
Loading jit-lock (native compiled elisp)...
Loading mouse (native compiled elisp)...
Loading scroll-bar (native compiled elisp)...
Loading select (native compiled elisp)...
Loading emacs-lisp/timer (native compiled elisp)...
Loading emacs-lisp/easymenu (native compiled elisp)...
Loading isearch (native compiled elisp)...
Loading rfn-eshadow (native compiled elisp)...
Loading menu-bar (native compiled elisp)...
Loading tab-bar (native compiled elisp)...
Loading emacs-lisp/lisp (native compiled elisp)...
Loading textmodes/page (native compiled elisp)...
Loading register (native compiled elisp)...
Loading textmodes/paragraphs (native compiled elisp)...
Loading progmodes/prog-mode (native compiled elisp)...
Loading emacs-lisp/lisp-mode (native compiled elisp)...
Loading textmodes/text-mode (native compiled elisp)...
Loading textmodes/fill (native compiled elisp)...
Loading newcomment (native compiled elisp)...
Loading replace (native compiled elisp)...
Loading emacs-lisp/tabulated-list (native compiled elisp)...
Loading buff-menu (native compiled elisp)...
Loading fringe (native compiled elisp)...
Loading emacs-lisp/regexp-opt (native compiled elisp)...
Loading image (native compiled elisp)...
Loading international/fontset (native compiled elisp)...
Loading dnd (native compiled elisp)...
Loading tool-bar (native compiled elisp)...
Loading term/common-win (native compiled elisp)...
Loading w32-vars (native compiled elisp)...
Loading term/w32-win (native compiled elisp)...
Loading disp-table (native compiled elisp)...
Loading w32-fns (native compiled elisp)...
Loading ls-lisp (native compiled elisp)...
Loading dos-w32 (native compiled elisp)...
Loading mwheel (native compiled elisp)...
Loading progmodes/elisp-mode (native compiled elisp)...
Loading emacs-lisp/float-sup (native compiled elisp)...
Loading vc/vc-hooks (native compiled elisp)...
Loading vc/ediff-hook (native compiled elisp)...
Loading uniquify (native compiled elisp)...
Loading electric (native compiled elisp)...
Loading paren (native compiled elisp)...
Loading emacs-lisp/shorthands (native compiled elisp)...
Loading emacs-lisp/eldoc (native compiled elisp)...
Loading emacs-lisp/cconv (native compiled elisp)...
Loading cus-start (native compiled elisp)...
Loading tooltip (native compiled elisp)...
Loading international/iso-transl (native compiled elisp)...
Loading emacs-lisp/rmc (native compiled elisp)...
Loading leim/leim-list.el (source)...
Waiting for git...
Waiting for git...
Finding pointers to doc strings...
Finding pointers to doc strings...done
Pure-hashed: 19100 strings, 1339 vectors, 47780 conses, 1189 bytecodes, 372 others
Dumping under the name emacs.pdmp
Dumping fingerprint: 398945edea1c0819df9b68b3e1ac4688711c9e6a18c2332d5c5248bcc4f1cb10
Dump complete
Byte counts: header=100 hot=10031196 discardable=200136 cold=7178720
Reloc counts: hot=580709 discardable=5422
Adding name emacs-30.0.50.2.exe
Adding name emacs-30.0.50.2.pdmp
cp -f emacs.pdmp bootstrap-emacs.pdmp
make[2]: Leaving directory '/home/albinus/src/emacs/src'
make -C lisp all
make[2]: Entering directory '/home/albinus/src/emacs/lisp'
make -C ../leim all EMACS="../src/emacs.exe"
make[3]: Entering directory '/home/albinus/src/emacs/leim'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/leim'
make -C ../admin/grammars all EMACS="../../src/emacs.exe"
make[3]: Entering directory '/home/albinus/src/emacs/admin/grammars'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/grammars'
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
  GEN      autoloads
  INFO     Scraping files for loaddefs...
  INFO     Scraping files for loaddefs...done
  INFO     Scraping files for loaddefs...
  INFO     Scraping files for loaddefs...done
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
make[3]: Entering directory '/home/albinus/src/emacs/leim'
  GEN      small-ja-dic-option
make[3]: Leaving directory '/home/albinus/src/emacs/leim'
make[3]: Entering directory '/home/albinus/src/emacs/lisp'
make[3]: Nothing to be done for 'compile-targets'.
make[3]: Leaving directory '/home/albinus/src/emacs/lisp'
make[3]: Entering directory '/home/albinus/src/emacs/doc/misc'
make[3]: 'org.texi' is up to date.
make[3]: 'modus-themes.texi' is up to date.
make[3]: Leaving directory '/home/albinus/src/emacs/doc/misc'
make[2]: Leaving directory '/home/albinus/src/emacs/lisp'
make -C doc/lispref info
make[2]: Entering directory '/home/albinus/src/emacs/doc/lispref'
make[2]: Nothing to be done for 'info'.
make[2]: Leaving directory '/home/albinus/src/emacs/doc/lispref'
make -C doc/lispintro info
make[2]: Entering directory '/home/albinus/src/emacs/doc/lispintro'
make[2]: Nothing to be done for 'info'.
make[2]: Leaving directory '/home/albinus/src/emacs/doc/lispintro'
make -C doc/emacs info
make[2]: Entering directory '/home/albinus/src/emacs/doc/emacs'
make[2]: Nothing to be done for 'info'.
make[2]: Leaving directory '/home/albinus/src/emacs/doc/emacs'
make -C doc/misc info
make[2]: Entering directory '/home/albinus/src/emacs/doc/misc'
make[2]: Nothing to be done for 'info'.
make[2]: Leaving directory '/home/albinus/src/emacs/doc/misc'
make -C src BIN_DESTDIR=''/mingw64/bin/'' ELN_DESTDIR='/mingw64/lib/emacs/30.0.50/'
make[2]: Entering directory '/home/albinus/src/emacs/src'
make -C ../nt ../src/emacs.res
make[3]: Entering directory '/home/albinus/src/emacs/nt'
make[3]: '../src/emacs.res' is up to date.
make[3]: Leaving directory '/home/albinus/src/emacs/nt'
make -C ../admin/charsets all
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
make -C ../admin/unidata charscript.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'charscript.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/unidata emoji-zwj.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'emoji-zwj.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/unidata all EMACS="../../src/bootstrap-emacs.exe"
make[3]: Entering directory '/home/albinus/src/emacs/admin/unidata'
make[3]: Nothing to be done for 'all'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/unidata'
make -C ../admin/charsets cp51932.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'cp51932.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
make -C ../admin/charsets eucjp-ms.el
make[3]: Entering directory '/home/albinus/src/emacs/admin/charsets'
make[3]: Nothing to be done for 'eucjp-ms.el'.
make[3]: Leaving directory '/home/albinus/src/emacs/admin/charsets'
make[2]: Leaving directory '/home/albinus/src/emacs/src'
make[1]: Leaving directory '/home/albinus/src/emacs'
make sanity-check make-target=all
make[1]: Entering directory '/home/albinus/src/emacs'
make[1]: Leaving directory '/home/albinus/src/emacs'

albinus@win10 MINGW64 ~/src/emacs
--8<---------------cut here---------------end--------------->8---

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Andreas Schwab]

Please disable native compile.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Stefan Monnier
>  <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Fri, 15 Mar 2024 13:47:28 +0100
> 
> Please disable native compile.

Yes, my build where this happens is without native compilation.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>> Please disable native compile.
>
> Yes, my build where this happens is without native compilation.

Same here, when I disable native compilation: the warning appears. Strange.

Best regards, Michael.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Stefan Monnier]

> Same here, when I disable native compilation: the warning appears. Strange.

That's because `comp` requires `warnings` which requires `icons`, so
`icons` is already loaded before we native compile any file `simple.el`.


        Stefan

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Stefan Monnier]

> The changes in the above commit cause the following warning when
> compiling simple.el:
>
>   In read-passwd-toggle-visibility:
>   simple.el:10880:18: Warning: reference to free variable `icon-preference'

Does the patch below fix the problem?
[ I remember seeing the warning a few days ago, but can't reproduce it
  right now.  ]

Maybe we should do something so `icons.el` is easier to use
from preloaded files.


        Stefan


diff --git a/lisp/simple.el b/lisp/simple.el
index f127290231b..e0988f9dbd1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10870,6 +10932,8 @@ read-passwd-toggle-visibility
   "Toggle minibuffer contents visibility.
 Adapt also mode line."
   (interactive)
+  (require 'icons) ;; Will presumably already be loaded.
+  (defvar icon-preference)
   (setq read-passwd--hide-password (not read-passwd--hide-password))
   (with-current-buffer read-passwd--mode-line-buffer
     (setq read-passwd--mode-line-icon

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Michael Albinus <michael.albinus@gmx.de>,  emacs-devel@gnu.org
> Date: Fri, 15 Mar 2024 09:29:18 -0400
> 
> > The changes in the above commit cause the following warning when
> > compiling simple.el:
> >
> >   In read-passwd-toggle-visibility:
> >   simple.el:10880:18: Warning: reference to free variable `icon-preference'
> 
> Does the patch below fix the problem?
> [ I remember seeing the warning a few days ago, but can't reproduce it
>   right now.  ]

No, it makes things worse:

  In read-passwd-mode:
  simple.el:10907:16: Warning: reference to free variable `read-passwd--show-password-icon'
  simple.el:10912:12: Warning: reference to free variable `mode-line-faces'
  simple.el:10915:4: Error: `define-icon' defined after use in (define-icon read-passwd--hide-password-icon nil '((image "conceal.svg" "conceal.pbm" :height (0.8 . em)) (symbol " ") (text "<\\>")) "Mode line icon to hide a visible password." :group mode-line-faces :version "30.1" :help-echo "mouse-1: Toggle password visibility") (missing `require' of a library file?)
  simple.el:10915:16: Warning: reference to free variable `read-passwd--hide-password-icon'

  In end of data:
  simple.el:10907:4: Warning: the function `define-icon' might not be defined at runtime.
  simple.el:10883:19: Warning: the function `icon-string' might not be defined at runtime.
  Makefile:289: recipe for target `../lisp/simple.elc' failed

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Stefan Monnier]

> No, it makes things worse:
>
>   In read-passwd-mode:
>   simple.el:10907:16: Warning: reference to free variable `read-passwd--show-password-icon'
>   simple.el:10912:12: Warning: reference to free variable `mode-line-faces'
>   simple.el:10915:4: Error: `define-icon' defined after use in (define-icon
> read-passwd--hide-password-icon nil '((image "conceal.svg"
> "conceal.pbm" :height (0.8 . em)) (symbol " ") (text "<\\>")) "Mode line
> icon to hide a visible password." :group mode-line-faces :version
> "30.1" :help-echo "mouse-1: Toggle password visibility") (missing `require'
> of a library file?)
>   simple.el:10915:16: Warning: reference to free variable `read-passwd--hide-password-icon'
>
>   In end of data:
>   simple.el:10907:4: Warning: the function `define-icon' might not be defined at runtime.
>   simple.el:10883:19: Warning: the function `icon-string' might not be defined at runtime.
>   Makefile:289: recipe for target `../lisp/simple.elc' failed

Ah, right: the code as written only works if there's a warning earlier,
because it uses `define-icon` inside a function and that macro is
defined in `icons` and is not autoloaded, so the only reason it
currently works is that `display-warning` loads `icons`: as soon as we
fix the warning, the core problem shows up.

The hideous patch below should result in working code without warnings,
but I hope we can find a better solution (e.g. one that lets us move
the two `define-icon`s to the toplevel of some file rather than being
inside a function).


        Stefan


diff --git a/lisp/simple.el b/lisp/simple.el
index 0645f18cc78..5ce3e27e99b 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10873,6 +10873,9 @@ read-passwd-toggle-visibility
   "Toggle minibuffer contents visibility.
 Adapt also mode line."
   (interactive)
+  (require 'icons) ;; Should usually be loaded already.
+  (defvar icon-preference)
+  (declare-function icon-string "icons" (name))
   (setq read-passwd--hide-password (not read-passwd--hide-password))
   (with-current-buffer read-passwd--mode-line-buffer
     (setq read-passwd--mode-line-icon
@@ -10902,22 +10905,25 @@ read-passwd-mode
   ;; no corresponding Unicode char with a slash.  So we use symbols as
   ;; fallback only, with "⦵" ("\N{CIRCLE WITH HORIZONTAL BAR}") for
   ;; hiding the password.
-  (define-icon read-passwd--show-password-icon nil
-    '((image "reveal.svg" "reveal.pbm" :height (0.8 . em))
-      (symbol "👁")
-      (text "<o>"))
-    "Mode line icon to show a hidden password."
-    :group mode-line-faces
-    :version "30.1"
-    :help-echo "mouse-1: Toggle password visibility")
-  (define-icon read-passwd--hide-password-icon nil
-    '((image "conceal.svg" "conceal.pbm" :height (0.8 . em))
-      (symbol "⦵")
-      (text "<\\>"))
-    "Mode line icon to hide a visible password."
-    :group mode-line-faces
-    :version "30.1"
-    :help-echo "mouse-1: Toggle password visibility")
+  (eval
+   '(progn
+      (define-icon read-passwd--show-password-icon nil
+        '((image "reveal.svg" "reveal.pbm" :height (0.8 . em))
+          (symbol "👁")
+          (text "<o>"))
+        "Mode line icon to show a hidden password."
+        :group mode-line-faces
+        :version "30.1"
+        :help-echo "mouse-1: Toggle password visibility")
+      (define-icon read-passwd--hide-password-icon nil
+        '((image "conceal.svg" "conceal.pbm" :height (0.8 . em))
+          (symbol "⦵")
+          (text "<\\>"))
+        "Mode line icon to hide a visible password."
+        :group mode-line-faces
+        :version "30.1"
+        :help-echo "mouse-1: Toggle password visibility"))
+   t)
 
   (setq read-passwd--hide-password nil
         ;; Stolen from `eldoc-minibuffer-message'.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

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

Hi Stefan,

> The hideous patch below should result in working code without warnings,
> but I hope we can find a better solution (e.g. one that lets us move
> the two `define-icon`s to the toplevel of some file rather than being
> inside a function).

I wonder, whether we should simply move read-passwd and its helper
functions somewhere else, and give it an autoload cookie. Candidates
could be auth-source.el or reveal.el.

>         Stefan

Best regards, Michael.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

>> The hideous patch below should result in working code without warnings,
>> but I hope we can find a better solution (e.g. one that lets us move
>> the two `define-icon`s to the toplevel of some file rather than being
>> inside a function).
>
> I wonder, whether we should simply move read-passwd and its helper
> functions somewhere else, and give it an autoload cookie. Candidates
> could be auth-source.el or reveal.el.

Is there already a decision whether we want to apply Stefan's proposal,
or mine?

Best regards, Michael.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi,

>>> The hideous patch below should result in working code without warnings,
>>> but I hope we can find a better solution (e.g. one that lets us move
>>> the two `define-icon`s to the toplevel of some file rather than being
>>> inside a function).
>>
>> I wonder, whether we should simply move read-passwd and its helper
>> functions somewhere else, and give it an autoload cookie. Candidates
>> could be auth-source.el or reveal.el.
>
> Is there already a decision whether we want to apply Stefan's proposal,
> or mine?

No News is Good News. I've moved read-passwd and its helper objects to
auth-source.el.

Best regards, Michael.

Re: master 39e3fce0d5e0: 'read-passwd' can toggle the visibility of passwords

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: michael.albinus@gmx.de,  emacs-devel@gnu.org
> Date: Fri, 15 Mar 2024 13:55:34 -0400
> 
> >   In end of data:
> >   simple.el:10907:4: Warning: the function `define-icon' might not be defined at runtime.
> >   simple.el:10883:19: Warning: the function `icon-string' might not be defined at runtime.
> >   Makefile:289: recipe for target `../lisp/simple.elc' failed
> 
> Ah, right: the code as written only works if there's a warning earlier,
> because it uses `define-icon` inside a function and that macro is
> defined in `icons` and is not autoloaded, so the only reason it
> currently works is that `display-warning` loads `icons`: as soon as we
> fix the warning, the core problem shows up.
> 
> The hideous patch below should result in working code without warnings,
> but I hope we can find a better solution (e.g. one that lets us move
> the two `define-icon`s to the toplevel of some file rather than being
> inside a function).

Yes, the warnings are gone with this patch.

I do hope we can find a cleaner solution.

How about loading icons when compiling?  Or adding an autoload form
which states that the variable is in icons.el?