bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

Write the following to a file foo.el:

-------------------- foo.el --------------------
;;; -*- lexical-binding: t -*-

(defun delete-process@emacs-fix (&rest patch-args)
  "Allows interactive calls."
  (interactive
   (list
    (completing-read "Delete process: "
                     (mapcar
                      (lambda (process)
                        (list (process-name process)))
                      (process-list))))))
-------------------- foo.el --------------------

(This is used to advise function `delete-process' in my .emacs.)

Then start Emacs as "emacs -Q" and

  M-x byte-compile-file ~/foo.el RET

For me this results in two warnings:

-------------------- *Compile-Log* --------------------
Compiling file /home/jschmidt/foo.el at Thu Sep 21 12:34:05 2023
Entering directory ‘/home/jschmidt/’

In delete-process@emacs-fix:
foo.el:5:4: Warning: misplaced interactive spec: ‘(interactive (list
    (completing-read Delete process: (mapcar #'(lambda (process) (list
    (process-name process))) (process-list)))))’
foo.el:3:40: Warning: Unused lexical argument `patch-args'
-------------------- *Compile-Log* --------------------

I'm OK with the latter, but I think the former is incorrect.

The former warning goes away if any of the following is done:

- the "lexical-binding: t" is removed from the file
- `patch-args' is renamed to `_patch-args'
- an explicit nil body is added to the function


In GNU Emacs 29.1 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-08-30, modified by Debian
 built on x86-csail-01
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/libexec
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-libsystemd --with-pop=yes
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils
 --with-native-compilation --with-x=yes --with-x-toolkit=lucid
 --with-toolkit-scroll-bars --without-gsettings 'CFLAGS=-g -O2
 -ffile-prefix-map=/build/reproducible-path/emacs-29.1+1=. -fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XAW3D XDBE XIM XINPUT2 XPM
LUCID ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-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:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
comp comp-cstr warnings icons subr-x rx cl-seq cl-macs gv cl-extra
help-mode bytecomp byte-compile cl-lib sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils 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 dbusbind inotify lcms2
dynamic-setting font-render-setting cairo x-toolkit xinput2 x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 76983 14037)
 (symbols 48 7141 0)
 (strings 32 20219 2411)
 (string-bytes 1 594205)
 (vectors 16 15313)
 (vector-slots 8 322958 19812)
 (floats 8 40 46)
 (intervals 56 304 0)
 (buffers 984 11))

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

Smaller test case is

-------------------- foo.el --------------------
;;; -*- lexical-binding: t -*-

(defun delete-process@emacs-fix (arg)
  "Allows interactive calls."
  (interactive "^P"))
-------------------- foo.el --------------------

And even smaller might be (but I cannot interpret the results for sure)
that

-------------------- bad case --------------------
(byte-compile-preprocess
 '(defun delete-process@emacs-fix
    (arg)
    "Allows interactive calls."
    (interactive "^P")))

=>

(defalias 'delete-process@emacs-fix
  #'(lambda (arg) "Allows interactive calls."
      (macroexp--funcall-if-compiled
       '#[0 "\300:\203\f\0\303\304\300\"\202\0\304\300!\205\0\305\302\306\301#\207"
	  [lexical "Unused lexical argument ‘arg’" arg apply
		   byte-compile-warning-enabled-p byte-compile-warn-x
		   "%s"]
	  4...])
      ;; this dangling interactive form ultimately seems to
      ;; trigger the warning
      (interactive "^P")))
-------------------- bad case --------------------

while

-------------------- good case --------------------
(byte-compile-preprocess
 '(defun delete-process@emacs-fix
    (arg)
    "Allows interactive calls."
    (interactive "^P")
    nil))

=>

(defalias 'delete-process@emacs-fix
  #'(lambda (arg) "Allows interactive calls."
      (interactive "^P")
      (macroexp--funcall-if-compiled
       '#[0 "\300:\203\f\0\303\304\300\"\202\0\304\300!\205\0\305\302\306\301#\207"
	  [lexical "Unused lexical argument ‘arg’" arg apply
		   byte-compile-warning-enabled-p byte-compile-warn-x
		   "%s"]
	  4])
      nil))
-------------------- good case --------------------

so `byte-compile-preprocess' could be the culprit here, inserting the
warning wrapped into the `macroexp--funcall-if-compiled' in the wrong
place.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

Found the issue I think:

-------------------- bad case --------------------
(macroexp-parse-body '("Allows interactive calls." (interactive "^P")))

=>

(("Allows interactive calls.")
 (interactive "^P"))
-------------------- bad case --------------------

-------------------- good case --------------------
(macroexp-parse-body '("Allows interactive calls." (interactive "^P") nil))

=>

(("Allows interactive calls." (interactive "^P"))
 nil)
-------------------- good case --------------------

That is, macroexp-parse-body does not consider the case that a body can
consist of declarations only and, if this is the case, puts the last
declaration into the body forms.

Could provide a patch if somebody confirms that this is really the root
cause of this issue.  Yet on the other hand this is pretty deep elisp,
so if somebody else steps forward, I'll be glad as well.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

[Eli Zaretskii]

> Date: Thu, 21 Sep 2023 23:39:47 +0200
> From:  Jens Schmidt via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Found the issue I think:
> 
> -------------------- bad case --------------------
> (macroexp-parse-body '("Allows interactive calls." (interactive "^P")))
> 
> =>
> 
> (("Allows interactive calls.")
>  (interactive "^P"))
> -------------------- bad case --------------------
> 
> -------------------- good case --------------------
> (macroexp-parse-body '("Allows interactive calls." (interactive "^P") nil))
> 
> =>
> 
> (("Allows interactive calls." (interactive "^P"))
>  nil)
> -------------------- good case --------------------
> 
> That is, macroexp-parse-body does not consider the case that a body can
> consist of declarations only and, if this is the case, puts the last
> declaration into the body forms.
> 
> Could provide a patch if somebody confirms that this is really the root
> cause of this issue.  Yet on the other hand this is pretty deep elisp,
> so if somebody else steps forward, I'll be glad as well.

Adding Stefan, in case he has comments/suggestions.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

>> Date: Thu, 21 Sep 2023 23:39:47 +0200
>> From:  Jens Schmidt via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Found the issue I think:
>> 
>> -------------------- bad case --------------------
>> (macroexp-parse-body '("Allows interactive calls." (interactive "^P")))
>> 
>> =>
>> 
>> (("Allows interactive calls.")
>>  (interactive "^P"))
>> -------------------- bad case --------------------
>> 
>> -------------------- good case --------------------
>> (macroexp-parse-body '("Allows interactive calls." (interactive "^P") nil))
>> 
>> =>
>> 
>> (("Allows interactive calls." (interactive "^P"))
>>  nil)
>> -------------------- good case --------------------
>> 
>> That is, macroexp-parse-body does not consider the case that a body can
>> consist of declarations only and, if this is the case, puts the last
>> declaration into the body forms.
>> 
>> Could provide a patch if somebody confirms that this is really the root
>> cause of this issue.  Yet on the other hand this is pretty deep elisp,
>> so if somebody else steps forward, I'll be glad as well.
>
> Adding Stefan, in case he has comments/suggestions.

I'm in favor of requiring *something* after the declarations.
So yes, the first case above is a bug and should be fixed, but rather
than return

    (("Allows interactive calls." (interactive "^P"))
     nil)

I think it should return something like

    (("Allows interactive calls." (interactive "^P"))
     ,(macroexp-warn-and-return "Missing body" ...))
    

-- Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

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

> I'm in favor of requiring *something* after the declarations.
> So yes, the first case above is a bug and should be fixed, but rather
> than return
>
>     (("Allows interactive calls." (interactive "^P"))
>      nil)
>
> I think it should return something like
>
>     (("Allows interactive calls." (interactive "^P"))
>      ,(macroexp-warn-and-return "Missing body" ...))
>
>
> -- Stefan

I managed to cobble up something like that, but are you really sure you
want to warn about an empty/missing body?  I have a number of arguments
against that, the main being that `cl-defgeneric' is processed through
`macroexp-parse-body' exactly like `defun' - and for `cl-defgeneric' an
empty body seems to be the rule and not the exception.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

> I managed to cobble up something like that, but are you really sure you
> want to warn about an empty/missing body?  I have a number of arguments
> against that, the main being that `cl-defgeneric' is processed through
> `macroexp-parse-body' exactly like `defun' - and for `cl-defgeneric' an
> empty body seems to be the rule and not the exception.

Hmm... for `cl-defgeneric`, indeed, an empty body is normal (it doesn't
mean "return nil" but it means the absence of a default method).
Very good point.

I know Mattias played with this part of the code (mostly to try and
figure what to do about the ordering of the various possible kinds of
declarations, which is a related yet different issue).  Maybe he has
a more informed opinion.


        Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

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

> I know Mattias played with this part of the code (mostly to try and
> figure what to do about the ordering of the various possible kinds of
> declarations, which is a related yet different issue).  Maybe he has
> a more informed opinion.

One more data point for opinion building: When byte-compiling a
completely empty function (no declarations, no body forms):

------------------------- snip -------------------------
(let ((lexical-binding t))
  (byte-compile
   '(defun foo (arg))))
------------------------- snip -------------------------

some upper layer already seems to replace the empty body by a sole nil,
which is then kept by function `macroexp-parse-body':

-------------------- *trace-output --------------------
1 -> (macroexp-parse-body (nil))
1 <- macroexp-parse-body: (nil nil)
-------------------- *trace-output --------------------

So probably we should mimic that in `macroexp-parse-body' if there are
declarations, but no body forms?  Or change that upper layer to also add
a sole nil in the declarations-only case?

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

[Mattias Engdegård]

[-- Attachment #1: Type: text/plain, Size: 1362 bytes --]

22 sep. 2023 kl. 23.39 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> I know Mattias played with this part of the code (mostly to try and
> figure what to do about the ordering of the various possible kinds of
> declarations, which is a related yet different issue).  Maybe he has
> a more informed opinion.

Can't lay much claim to that I'm afraid. In this case I'd just have macroexp-parse-body return an empty body and be done with it. Suggested patch attached.

Our triptych of meta-forms in function bodies (documentation, declare, interactive) is still not handled in a very principled or robust way. We keep parsing and re-parsing them in several places. I'm tempted to replace lambda with an intermediate form where these things are already parsed, early in the front-end (maybe even macroexpand-all).

There are all sorts of little annoyances, such as:

- if nothing comes after a (literal) doc string, the doc string also becomes the function body
- `declare` is only allowed in named definitions because it is macro-expanded very early, so we have no way of annotating lambda expressions
- not sure :documentation is handled correctly everywhere since it's a late addition
- that 'misplaced interactive spec' warning shouldn't be emitted from the Lisp optimiser at all but fully handled in the front-end like all syntax errors


[-- Attachment #2: macroexp-parse-body.diff --]
[-- Type: application/octet-stream, Size: 837 bytes --]

diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index f96e0d74026..3bf0222c40d 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -525,12 +525,15 @@ macroexpand--all-toplevel
 (defun macroexp-parse-body (body)
   "Parse a function BODY into (DECLARATIONS . EXPS)."
   (let ((decls ()))
-    (while (and (cdr body)
+    (while (and body
                 (let ((e (car body)))
                   (or (stringp e)
                       (memq (car-safe e)
                             '(:documentation declare interactive cl-declare)))))
       (push (pop body) decls))
+    (when (and (null body) (stringp (car decls)))
+      ;; No body but a literal doc string: move it back to the body.
+      (push (pop decls) body))
     (cons (nreverse decls) body)))
 
 (defun macroexp-progn (exps)

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

> - `declare` is only allowed in named definitions because it is
> macro-expanded very early, so we have no way of annotating lambda
> expressions

It's not because of timing, it's because these `declare` all store their
info on the symbol (and some of them even fundamentally really apply to
the name and wouldn't make sense when applied to an anonymous function,
e.g. obsolescence).


        Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

[Mattias Engdegård]

23 sep. 2023 kl. 18.08 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
> 
>> - `declare` is only allowed in named definitions because it is
>> macro-expanded very early, so we have no way of annotating lambda
>> expressions
> 
> It's not because of timing, it's because these `declare` all store their
> info on the symbol (and some of them even fundamentally really apply to
> the name and wouldn't make sense when applied to an anonymous function,
> e.g. obsolescence).

That is true, but some of the declarations might be useful for lambda expressions as well, and more to the point, we don't have any other way to annotate functions in general, only function symbols. Arguably we should invent new syntax for whatever we need, but it would be one more thing to parse.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

> That is true, but some of the declarations might be useful for lambda
> expressions as well, and more to the point, we don't have any other way to
> annotate functions in general, only function symbols.

FWIW, `set-advertised-calling-convention` stores the info with the
actual function (in an `eq` has-table) rather than as symbol property.

> Arguably we should invent new syntax for whatever we need, but it
> would be one more thing to parse.

I seem to remember trying to share `declare` for both (i.e. leave the
entries not handled by `defmacro/defun` in the `lambda`), but I think
I didn't spend very much time on this for lack of a good use-case.


        Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

Mattias Engdegård <mattias.engdegard@gmail.com> writes:

> Our triptych of meta-forms in function bodies (documentation,
> declare, interactive) is still not handled in a very principled
> or robust way.

Just curious since `macroexp-parse-body' also takes care about it: Where
is `cl-declare' in this picture?  Is it relevant at all?

I'm asking since

------------------------- snip -------------------------
(byte-compile
 '(defun foo ()
    (cl-declare)
    (interactive)))
------------------------- snip -------------------------

gives the same "misplaced interactive spec" warning, but for a different
reason: The `cl-declare' gets replaced by nil somewhere before `m-p-b'
gets called, so `m-p-b' sees and returns:

-------------------- *trace-output* --------------------
1 -> (macroexp-parse-body (nil (interactive)))
1 <- macroexp-parse-body: (nil nil (interactive))
-------------------- *trace-output* --------------------

> [...] Suggested patch attached.

Thanks, will give it a try.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

>> Our triptych of meta-forms in function bodies (documentation,
>> declare, interactive) is still not handled in a very principled
>> or robust way.
> Just curious since `macroexp-parse-body' also takes care about it: Where
> is `cl-declare' in this picture?

I don't think anybody treats it with much respect and it comes with
a fair share of caveats.  Personally I keep a safe distance from it :-)

> ------------------------- snip -------------------------
> (byte-compile
>  '(defun foo ()
>     (cl-declare)
>     (interactive)))
> ------------------------- snip -------------------------

I think you *have* to place it after the `interactive` if you want it to work.


        Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

[Mattias Engdegård]

23 sep. 2023 kl. 21.01 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

> `set-advertised-calling-convention` stores the info with the
> actual function (in an `eq` has-table) rather than as symbol property.

Ah yes. You wouldn't remember the reason for it, would you?

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

>> `set-advertised-calling-convention` stores the info with the
>> actual function (in an `eq` has-table) rather than as symbol property.
> Ah yes. You wouldn't remember the reason for it, would you?

I do: it was a kind of experiment with the intention to move further in
this direction for all those kinds of properties which fundamentally
apply to the lambda rather than to the name to which it happens to
be bound.

What I don't know is why I didn't follow that same idea for
other properties.


        Stefan

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

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

Jens Schmidt <jschmidt4gnu@vodafonemail.de> writes:

> Mattias Engdegård <mattias.engdegard@gmail.com> writes:

>> [...] Suggested patch attached.
>
> Thanks, will give it a try.

As expected, your patch fixes this issue.

Thanks.

bug#66136: 29.1; byte-compiler reports "misplaced interactive spec" with empty fct in lexical environment

[Mattias Engdegård]

25 sep. 2023 kl. 12.24 skrev Jens Schmidt <jschmidt4gnu@vodafonemail.de>:

> As expected, your patch fixes this issue.

Thank you, a slightly improved version is now on master. Closing.