bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

[Juan José García Ripoll]

(cl-loop
 for database in nil
 for aux = (message "databse: %S" database)
 for entry being the hash-values of database
 do (message "FOO %S" database))

This code should do nothing, because variable DATABASE runs over an empty list. Yet the macroexpansion of the loop produces code that tries to run over the entries of DATABASE, which is NIL.

(cl--block-wrapper
 (catch '--cl-block-nil--
   (let* ((--cl-var-- nil)
	  (database nil)
	  (aux nil))
     (cl-block --cl-finish--
       (maphash (lambda (--cl-var-- entry)
		  (or (consp --cl-var--) (cl-return-from --cl-finish-- nil))
		  (setq database (car --cl-var--))
		  (setq aux (message "databse: %S" database))
		  (message "FOO %S" database)
		  (setq --cl-var-- (cdr --cl-var--)))
		database))
     nil)))

Compare this with

(cl-prettyprint
 (macroexpand '(cl-loop
 for database in nil
 for aux = (message "databse: %S" database)
 do (message "FOO %S" database))))

(cl--block-wrapper
 (catch '--cl-block-nil--
   (let* ((--cl-var-- nil)
	  (database nil)
	  (aux nil))
     (while (consp --cl-var--)
       (setq database (car --cl-var--))
       (setq aux (message "databse: %S" database))
       (message "FOO %S" database)
       (setq --cl-var-- (cdr --cl-var--)))
     nil)))

Here the resulting code properly checks for the --cl-var-- variabel that runs the loop


In GNU Emacs 27.1.90 (build 1, x86_64-w64-mingw32)
Repository revision: 37eba74d609c74bcf9ac3c481a29377913783ac4
Repository branch: HEAD
Windowing system distributor 'Microsoft Corp.', version 10.0.19042
System Description: Microsoft Windows 10 Pro (v10.0.2009.19042.928)

Recent messages:
nnimap read 0k from correo.csic.es [2 times]
No more unseen articles
No more unread articles
nnimap read 0k from correo.csic.es
No more unseen articles
No more unread articles
Auto-saving...done
Mark set
Replaced 122 occurrences
Calendar ~/Nextcloud/Documents/Notes/calendar.ics unchanged
Quit
Configured using:
 'configure
 --prefix=/c/Users/juanj/src/emacs-build/pkg/emacs-27_1_90-x86_64
 --with-zlib --with-gnutls --with-xml2 --with-lcms2 --with-json
 --with-harfbuzz --with-cairo --with-rsvg --with-png --with-gif
 --with-tiff --with-jpeg --with-xpm --disable-build-details
 --disable-silent-rules --without-dbus'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY W32NOTIFY ACL GNUTLS LIBXML2
HARFBUZZ ZLIB TOOLKIT_SCROLL_BARS MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: cp1252

Major mode: Summary

Minor modes in effect:
  shell-dirtrack-mode: t
  display-time-mode: t
  which-key-mode: t
  icomplete-mode: t
  save-place-mode: t
  savehist-mode: t
  gcmh-mode: t
  override-global-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/modus-vivendi-theme-0.13.2/modus-vivendi-theme hides c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/modus-themes-1.2.4/modus-vivendi-theme
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/modus-operandi-theme-0.13.2/modus-operandi-theme hides c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/modus-themes-1.2.4/modus-operandi-theme
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-virtual hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-virtual
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-view hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-view
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-util hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-util
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-tools hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-tools
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-sync hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-sync
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-outline hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-outline
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-occur hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-occur
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-misc hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-misc
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-loader hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-loader
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-links hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-links
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-isearch hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-isearch
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-info hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-info
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-history hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-history
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-dev hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-dev
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-cache hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-cache
c:/Users/juanj/OneDrive/Library/Emacs/elpa-27/pdf-tools-0.90/pdf-annot hides c:/Users/juanj/scoop/apps/emacs/current/share/emacs/site-lisp/pdf-tools/pdf-annot

Features:
(shadow warnings emacsbug rng-xsd xsd-regexp rng-cmpct rng-nxml
rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util
rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode nxml-util
nxml-enc xmltok arc-mode archive-mode doc-view image-mode exif nnfolder
jka-compr misearch multi-isearch gnus-icalendar org-capture org-refile
org-tempo tempo org org-macro org-footnote org-pcomplete org-list
org-faces org-entities noutline outline org-version ob-python python
tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat shell pcomplete ob ob-tangle org-src ob-ref ob-lob ob-table
ob-exp ob-comint comint ring ob-emacs-lisp ob-core ob-eval org-table ol
org-keys org-compat org-macs org-loaddefs icalendar flow-fill smtpmail
flyspell ispell vc-git diff-mode mailalias sendmail bbdb-mua bbdb-com
crm bbdb bbdb-site timezone face-remap nnir mule-util sort smiley
ansi-color gnus-cite mm-archive mail-extr gnus-bcklg gnus-async qp
gnus-ml gnus-topic gnus-demon utf-7 nndraft nnmh nnmaild epa-file gnutls
network-stream nsm nnml nnnil gnus-agent gnus-srvr gnus-score score-mode
nnvirtual gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime dig
nntp gnus-cache gnus-sum url url-proxy url-privacy url-expand
url-methods url-history shr url-cookie url-domsuf url-util svg xml dom
gnus-group gnus-undo gnus-start gnus-cloud nnimap nnmail mail-source
utf7 netrc nnoo parse-time iso8601 gnus-spec gnus-int gnus-range message
rmc puny dired dired-loaddefs format-spec rfc822 mml mml-sec epa derived
epg mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader gnus-win gnus nnheader gnus-util rmail
rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search
mail-utils mm-util mail-prsvr wid-edit time-date time cal-china lunar
solar cal-dst cal-bahai cal-islam cal-hebrew holidays hol-loaddefs appt
diary-lib diary-loaddefs cal-menu calendar cal-loaddefs
benchmark-init-modes debug backtrace find-func mailcap which-key
icomplete epg-config modus-operandi-theme modus-themes saveplace
savehist edmacro kmacro benchmark-init advice gcmh diminish cl-extra
help-mode use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core finder-inf tex-site info package easymenu browse-url
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
disp-table term/w32-win w32-win w32-vars term/common-win tool-bar dnd
fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
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 charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads w32notify w32
lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 1068058 390211)
 (symbols 48 37670 37)
 (strings 32 286486 56033)
 (string-bytes 1 8189807)
 (vectors 16 85853)
 (vector-slots 8 2485239 445144)
 (floats 8 1011 1568)
 (intervals 56 14806 18837)
 (buffers 1000 184))

-- 
Juan José García Ripoll

Quantum Information and Foundations Group
Institute of Fundamental Physics IFF-CSIC
Calle Serrano 113b, Madrid 28006 Spain
http://quinfog.hbar.es - http://juanjose.garciaripoll.com

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

[Lars Ingebrigtsen]

Juan José García Ripoll <juanjose.garcia.ripoll@csic.es> writes:

> (cl-loop
>  for database in nil
>  for aux = (message "databse: %S" database)
>  for entry being the hash-values of database
>  do (message "FOO %S" database))
>
> This code should do nothing, because variable DATABASE runs over an empty list. Yet the macroexpansion of the loop produces code that tries to run over the entries of DATABASE, which is NIL.
>
> (cl--block-wrapper
>  (catch '--cl-block-nil--
>    (let* ((--cl-var-- nil)
> 	  (database nil)
> 	  (aux nil))
>      (cl-block --cl-finish--
>        (maphash (lambda (--cl-var-- entry)
> 		  (or (consp --cl-var--) (cl-return-from --cl-finish-- nil))
> 		  (setq database (car --cl-var--))
> 		  (setq aux (message "databse: %S" database))
> 		  (message "FOO %S" database)
> 		  (setq --cl-var-- (cdr --cl-var--)))
> 		database))
>      nil)))

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

Yes, that does seem wrong.  I had a brief look at cl--parse-loop-clause,
but this is code I haven't looked at before, and I'm not sure I
understand the control flow here.  Adding Stefan to the CCs; I'm sure he
can tell what should be done immediately.  😀

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

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

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

> Yes, that does seem wrong.

Agreed, but I don't know what the code is supposed to do when `database`
is not-nil, so I can't even tell if the code is meaningful.

> I had a brief look at cl--parse-loop-clause, but this is code
> I haven't looked at before, and I'm not sure I understand the control
> flow here.  Adding Stefan to the CCs; I'm sure he can tell what should
> be done immediately.  😀

Well, it's fairly obvious: we should just fix the problem.
Next!


        Stefan

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

[Lars Ingebrigtsen]

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

>> Yes, that does seem wrong.
>
> Agreed, but I don't know what the code is supposed to do when `database`
> is not-nil, so I can't even tell if the code is meaningful.

(cl-loop
 for database in nil
 for aux = (message "databse: %S" database)
 for entry being the hash-values of database
 do (message "FOO %S" database))

Hm...  Oh, yeah -- that's not really a meaningful loop statement, is it?
But by analogue, this equally pointless loop returns nil:

(cl-loop
 for database in '((1) (2))
 for entry in database
 collect entry)

So I guess the original loop should also do that.

But since it's a meaningless loop, perhaps erroring out in the hash
table case is fine, too?

>> I had a brief look at cl--parse-loop-clause, but this is code
>> I haven't looked at before, and I'm not sure I understand the control
>> flow here.  Adding Stefan to the CCs; I'm sure he can tell what should
>> be done immediately.  😀
>
> Well, it's fairly obvious: we should just fix the problem.
> Next!

Whew!  That was simple!

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

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

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

Lars Ingebrigtsen [2022-07-02 13:24:36] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Yes, that does seem wrong.
>>
>> Agreed, but I don't know what the code is supposed to do when `database`
>> is not-nil, so I can't even tell if the code is meaningful.
>
> (cl-loop
>  for database in nil
>  for aux = (message "databse: %S" database)
>  for entry being the hash-values of database
>  do (message "FOO %S" database))
>
> Hm...  Oh, yeah -- that's not really a meaningful loop statement, is it?

I think it's not because `database` is both a loop variables (i.e. one
that iterates over elements of something else) and one of the something
else over which we want to iterate.  So the only meaningful
interpretation I can see would be 2 nested loops, but the `loop` macro
is designed to make a single loop, not nested loops.

FWIW, I just tried it with `clisp` and it happpily expands it into
a single loop which does something weird (it sets up up a hash-table
iterator to operator over "the hash-table `database`" but at a time
where the `database` variable has not yet been initialized (i.e. it's
nil)).

> But since it's a meaningless loop, perhaps erroring out in the hash
> table case is fine, too?

I'm OK with erroring out if we can emit a meaning error message.

> Whew!  That was simple!

Occam's razor, man: it's all in the shave!


        Stefan "who doesn't shave"

bug#48219: 27.1.90; CL-LOOP facility fails with hash tables

[Lars Ingebrigtsen]

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

> I think it's not because `database` is both a loop variables (i.e. one
> that iterates over elements of something else) and one of the something
> else over which we want to iterate.  So the only meaningful
> interpretation I can see would be 2 nested loops, but the `loop` macro
> is designed to make a single loop, not nested loops.

Yes, I think it should be a single loop, and in that case, the
loop is meaningless.

> FWIW, I just tried it with `clisp` and it happpily expands it into
> a single loop which does something weird (it sets up up a hash-table
> iterator to operator over "the hash-table `database`" but at a time
> where the `database` variable has not yet been initialized (i.e. it's
> nil)).

So it signals an error in this case, too?

>> But since it's a meaningless loop, perhaps erroring out in the hash
>> table case is fine, too?
>
> I'm OK with erroring out if we can emit a meaning error message.

I think the current error message is OK, really...

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