GNU bug report logs - #46819
28.0.50; Undesireable warning: Empty let body from the `benchmark-run' implementation

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46819 in the body.
You can then email your comments to 46819 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: bug-gnu-emacs <at> gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: 28.0.50; Undesireable warning: Empty let body from the
 `benchmark-run' implementation
Date: Sat, 27 Feb 2021 12:18:56 -0800

This behavior began with Stefan's recent commit d41a4ad4ae (*
lisp/emacs-lisp/macroexp.el ;; (macroexp--expand-all): Warn on empty let
bodies, 2021-02-15)

`benchmark-run' has a timing `doloop' with an empty body, which `doloop`
expands to a `let` with an empty body (in the lexical-binding case).

Save the following into my-benchmark-warning.el

----------------------------------------------------------------------
;;; Code:

(require 'benchmark)

(defun my-list-benchmark ()
  "Lorem ipsum dolor."
  (benchmark-run 100
    (list 1 2 3)))

(provide 'my-benchmark-warning)
;;; my-benchmark-warning.el ends here
----------------------------------------------------------------------

Then run this and you'll see the interaction below:

----------------------------------------------------------------------
% ./src/emacs -Q --batch -f batch-byte-compile ./my-benchmark-warning.el

In my-list-benchmark:
./my-benchmark-warning.el:32:8: Warning: Empty let body
----------------------------------------------------------------------





In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.3.0, NS appkit-2022.30 Version 11.2.1 (Build 20D74))
 of 2021-02-27 built on matts-mbp-2016.lan
Repository revision: de33de1a660283f45a10acfaedef20c460553dbd
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2022
System Description:  macOS 11.2.1

Configured using:
 'configure --without-info'

Configured features:
ACL GLIB GMP GNUTLS JPEG JSON LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER
PNG RSVG THREADS TIFF TOOLKIT_SCROLL_BARS ZLIB

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

Major mode: Special

Minor modes in effect:
  winner-mode: t
  electric-pair-mode: t
  auto-insert-mode: t
  display-time-mode: t
  show-paren-mode: t
  keyfreq-autosave-mode: t
  keyfreq-mode: t
  icomplete-mode: t
  magit-auto-revert-mode: t
  global-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  shell-dirtrack-mode: t
  global-flycheck-mode: t
  which-key-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/Users/matt/git/notmuch/emacs/notmuch-mua hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-mua
/Users/matt/git/notmuch/emacs/notmuch-tree hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-tree
/Users/matt/git/notmuch/emacs/notmuch-show hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-show
/Users/matt/git/notmuch/emacs/notmuch-tag hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-tag
/Users/matt/git/notmuch/emacs/coolj hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/coolj
/Users/matt/git/notmuch/emacs/notmuch-wash hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-wash
/Users/matt/git/notmuch/emacs/notmuch-maildir-fcc hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-maildir-fcc
/Users/matt/git/notmuch/emacs/notmuch-compat hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-compat
/Users/matt/git/notmuch/emacs/notmuch hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch
/Users/matt/git/notmuch/emacs/notmuch-address hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-address
/Users/matt/git/notmuch/emacs/rstdoc hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/rstdoc
/Users/matt/git/notmuch/emacs/notmuch-lib hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-lib
/Users/matt/git/notmuch/emacs/notmuch-print hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-print
/Users/matt/git/notmuch/emacs/notmuch-draft hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-draft
/Users/matt/git/notmuch/emacs/notmuch-jump hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-jump
/Users/matt/git/notmuch/emacs/notmuch-parser hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-parser
/Users/matt/git/notmuch/emacs/notmuch-query hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-query
/Users/matt/git/notmuch/emacs/notmuch-message hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-message
/Users/matt/git/notmuch/emacs/notmuch-hello hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-hello
/Users/matt/git/notmuch/emacs/notmuch-crypto hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-crypto
/Users/matt/git/notmuch/emacs/make-deps hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/make-deps
/Users/matt/git/notmuch/emacs/notmuch-company hides /Users/matt/.emacs.d/elpa/notmuch-0.31.3/notmuch-company

Features:
(shadow sort company-oddmuse company-keywords company-etags
company-gtags company-dabbrev-code company-dabbrev company-files
company-clang company-capf company-cmake company-semantic
company-template company-bbdb mail-extr emacsbug sendmail org-element
avl-tree ol-w3m ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-search eieio-opt speedbar ezimage dframe gnus-art mm-uu mml2015
gnus-sum gnus-group gnus-undo gnus-start gnus-dbus dbus gnus-cloud
nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range
gnus-win ol-eww eww xdg url-queue mm-url gnus nnheader ol-docview
doc-view jka-compr image-mode exif ol-bibtex bibtex ol-bbdb winner
cus-start cus-load elec-pair slime-fancy slime-indentation
slime-cl-indent cl-indent slime-trace-dialog slime-fontifying-fu
slime-package-fu slime-references slime-compiler-notes-tree
slime-scratch slime-presentations bridge slime-macrostep macrostep
slime-mdot-fu slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl elp slime-parse slime gud apropos arc-mode archive-mode
hyperspec slime-autoloads warnings org-protocol notmuch notmuch-tree
notmuch-jump notmuch-hello notmuch-show notmuch-print notmuch-crypto
notmuch-mua notmuch-message notmuch-draft notmuch-maildir-fcc
notmuch-address notmuch-company notmuch-parser notmuch-wash coolj
notmuch-query goto-addr icalendar diary-lib diary-loaddefs notmuch-tag
notmuch-lib notmuch-compat hl-line mm-view mml-smime smime dig
elisp-format protbuf org-drill persist org-id org-agenda org-refile org
ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote
org-src ob-comint org-pcomplete org-list org-faces org-entities
org-version ob-emacs-lisp ob-core ob-eval org-table ol org-keys
org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs diminish
ctypes skeleton autoinsert time paren ert debug backtrace keyfreq
generic icomplete highlight-symbol cl hi-lock racket-mode
racket-bug-report racket-collection tq racket-repl-buffer-name
racket-stepper racket-logger racket-profile racket-smart-open racket-xp
racket-xp-complete racket-show pos-tip racket-imenu racket-edit hideshow
racket-repl semantic/symref/grep semantic/symref semantic/util-modes
semantic/util semantic pp semantic/tag semantic/lex semantic/fw
mode-local cedet racket-eldoc racket-describe shr kinsoku svg xml dom
racket-visit racket-complete racket-common racket-parens racket-indent
racket-font-lock racket-ppss racket-keywords-and-builtins racket-doc
racket-cmd racket-util racket-browse-url racket-custom sh-script smie
executable company-lsp company lsp-mode lsp-protocol tree-widget
wid-edit pcase network-stream nsm markdown-mode noutline outline lv
inline ht f s ewoc dash-functional bindat projectile grep ibuf-ext
ibuffer ibuffer-loaddefs cider tramp-sh cider-debug cider-browse-ns
cider-mode cider-inspector cider-completion cider-profile cider-eval
cider-repl-history pulse cider-repl cider-resolve cider-test
cider-overlays cider-stacktrace cider-doc cider-browse-spec
cider-clojuredocs cider-eldoc cider-client cider-common cider-connection
cider-util color cider-popup sesman-browser nrepl-client tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat
parse-time iso8601 ls-lisp queue nrepl-dict cider-compat spinner
parseedn parseclj-parser parseclj-lex a sesman vc vc-dispatcher edmacro
kmacro clojure-mode lisp-mnt align magit-submodule magit-obsolete
magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull
magit-fetch magit-clone magit-remote magit-commit magit-sequence
magit-notes magit-worktree magit-tag magit-merge magit-branch
magit-reset magit-files magit-refs magit-status magit magit-repos
magit-apply magit-wip magit-log which-func imenu magit-diff smerge-mode
diff diff-mode magit-core magit-autorevert autorevert filenotify
magit-margin magit-transient magit-process magit-mode git-commit
transient format-spec magit-git magit-section magit-utils crm log-edit
message rmc puny dired dired-loaddefs rfc822 mml mml-sec epa derived epg
epg-config gnus-util rmail rmail-loaddefs time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
with-editor async-bytecomp async shell pcomplete server ido flycheck
find-func rx dash go-mode find-file ffap thingatpt etags fileloop
generator xref project compile text-property-search comint ansi-color
ring which-key advice exec-path-from-shell 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
info package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap 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 iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util 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 easymenu 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 button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads kqueue
cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 766437 32631)
 (symbols 48 56488 4)
 (strings 32 248491 4562)
 (string-bytes 1 7334590)
 (vectors 16 89116)
 (vector-slots 8 956750 71031)
 (floats 8 732 300)
 (intervals 56 545 265)
 (buffers 992 12))

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: 28.0.50; Undesireable warning: Empty let body from the
 `benchmark-run' implementation
Date: Sat, 27 Feb 2021 16:00:21 -0500

> This behavior began with Stefan's recent commit d41a4ad4ae (*
> lisp/emacs-lisp/macroexp.el ;; (macroexp--expand-all): Warn on empty let
> bodies, 2021-02-15)
>
> `benchmark-run' has a timing `doloop' with an empty body, which `doloop`
> expands to a `let` with an empty body (in the lexical-binding case).

It'd be easy to put an explicit nil in the loop's body, but....

The idea was that in actual (non-macroexpanded) source code, empty let
bodies are usually the result of a misplaced paren.  But it's probably
more often annoying than useful since we can't easily distinguish a "let
from source code" from a "let from a macro expansion".

So I'm also voting to revert that change.
Any objection?


        Stefan

Message #11 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>
Cc: 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Sun, 28 Feb 2021 16:04:42 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>> It'd be easy to put an explicit nil in the loop's body, but....
>>
>> The idea was that in actual (non-macroexpanded) source code, empty
>> let bodies are usually the result of a misplaced paren.  But it's
>> probably more often annoying than useful since we can't easily
>> distinguish a "let from source code" from a "let from a macro
>> expansion".
>>
>> So I'm also voting to revert that change.  Any objection?
>
> It is a nice and helpful warning, though, so if it could be fixed in
> another way, that'd be great.  (But I have no idea how, of course.)

It might be hard to fix in the general case.  Think about this example:

(let (...)
  (some-macro blah))

What if that macro expands to nothing, based on some criteria not
entirely under programmer control?  Is the code "wrong"?

Message #14 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Mon, 01 Mar 2021 01:09:03 +0100

Matt Armstrong <matt <at> rfc20.org> writes:

> It might be hard to fix in the general case.  Think about this example:
>
> (let (...)
>   (some-macro blah))
>
> What if that macro expands to nothing, based on some criteria not
> entirely under programmer control?  Is the code "wrong"?

No.  But if the code is

(let (...)
  )

then it probably is.  So the warning (if retained) has to be done on the
form read before doing any macro expansions.

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

Message #17 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Mon, 01 Mar 2021 01:19:33 +0100

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

>> It might be hard to fix in the general case.  Think about this example:
>>
>> (let (...)
>>   (some-macro blah))
>>
>> What if that macro expands to nothing, based on some criteria not
>> entirely under programmer control?  Is the code "wrong"?

(And besides -- the macro can't really expand to nothing here, can it?
(nil is something.)  Or am I misunderstanding what it's warning about?)

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

Message #20 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Mon, 01 Mar 2021 00:04:34 -0500

> It might be hard to fix in the general case.  Think about this example:
>
> (let (...)
>   (some-macro blah))
>
> What if that macro expands to nothing,

No can do.  A macro call has to expand to exactly one expression (that
expression can be nil, but it's still one expression).


        Stefan

Message #23 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Matt Armstrong <matt <at> rfc20.org>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Sun, 28 Feb 2021 15:13:24 +0100

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

> It'd be easy to put an explicit nil in the loop's body, but....
>
> The idea was that in actual (non-macroexpanded) source code, empty let
> bodies are usually the result of a misplaced paren.  But it's probably
> more often annoying than useful since we can't easily distinguish a "let
> from source code" from a "let from a macro expansion".
>
> So I'm also voting to revert that change.
> Any objection?

It is a nice and helpful warning, though, so if it could be fixed in
another way, that'd be great.  (But I have no idea how, of course.)

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

Message #26 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Mon, 01 Mar 2021 21:07:21 -0800

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

>> It might be hard to fix in the general case.  Think about this example:
>>
>> (let (...)
>>   (some-macro blah))
>>
>> What if that macro expands to nothing,
>
> No can do.  A macro call has to expand to exactly one expression (that
> expression can be nil, but it's still one expression).
>
>         Stefan

Doh, of course.  That'd be a C macro I was thinking of...

Message #29 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Matt Armstrong <matt <at> rfc20.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Tue, 09 Mar 2021 20:53:19 -0800

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>> It'd be easy to put an explicit nil in the loop's body, but....
>>
>> The idea was that in actual (non-macroexpanded) source code, empty let
>> bodies are usually the result of a misplaced paren.  But it's probably
>> more often annoying than useful since we can't easily distinguish a "let
>> from source code" from a "let from a macro expansion".
>>
>> So I'm also voting to revert that change.
>> Any objection?
>
> It is a nice and helpful warning, though, so if it could be fixed in
> another way, that'd be great.  (But I have no idea how, of course.)

So, after a little flurry of a conversation, this went quiet.

How about we fix benchmark's use of `let', and leave the warning in
place.  It does not seem to be generating many other complaints, so it
may not be particularly annoying after all.  I'm currently working with
`benchmark-run' a bunch and would like the warning gone from that one
use.

I can make a patch, but a one liner might be quicker for one of you to
just commit directly.  ;-)

Message #32 received at 46819 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Matt Armstrong <matt <at> rfc20.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 46819 <at> debbugs.gnu.org
Subject: Re: bug#46819: 28.0.50; Undesireable warning: Empty let body from
 the `benchmark-run' implementation
Date: Wed, 10 Mar 2021 15:54:30 +0100

Matt Armstrong <matt <at> rfc20.org> writes:

> How about we fix benchmark's use of `let', and leave the warning in
> place.  It does not seem to be generating many other complaints, so it
> may not be particularly annoying after all.  I'm currently working with
> `benchmark-run' a bunch and would like the warning gone from that one
> use.
>
> I can make a patch, but a one liner might be quicker for one of you to
> just commit directly.  ;-)

Yup; now done.

I think we should keep the byte-compilation warning -- at least for
now.  If it leads to false positives all over the place, then perhaps
not, but it's a useful warning, in my opinion.

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.