Package: emacs;
Reported by: JD Smith <jdtsmith <at> gmail.com>
Date: Fri, 11 Jul 2025 19:29:02 UTC
Severity: normal
Tags: patch
Fixed in version 31
Done: "J.D. Smith" <jdtsmith <at> gmail.com>
View this message in rfc822 format
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: "J.D. Smith" <jdtsmith <at> gmail.com> Cc: 78995 <at> debbugs.gnu.org Subject: bug#78995: [PATCH] ;;;autoload-expand for special macros Date: Sat, 26 Jul 2025 13:13:40 -0400
[ Sorry for the long delay. ]
>> Here is an update on progress. The attached is working reasonably well,
>> reproducing lisp/loaddefs.el with a few changes (see attached). I went
>> with (declare ((autoload . macro-expand))) on the notion that there
>> might be other autoload-related declarations in the future, but open to
>> suggestion.
[ I don't have an opinion on the naming here but I don't understand your
argument: wouldn't a declaration name of `autoload` be *more* likely
to conflict with another future autoload-related declaration? ]
> Warning (emacs): loaddefs-gen: load error
> (file-missing Cannot open load file No such file or directory tramp-loaddefs)
> ...
>
> In this case these are harmless warnings, since in fact
> `tramp--with-startup' is /not/ requesting macro-expand, so failing to
> define it doesn't matter. Subsequent builds do not show this error.
> But this could be alarming to people, so we probably need a solution.
Yeah, it's one of those nasty issues. One way to look at it is that the
crux of the matter is that we use `;;;###autoload` for 2 different
purpose:
A. To copy the code into the loaddefs file.
B. To arrange for the file to be autoloaded when the corresponding thingy
is used.
(B) is the one that deserves the name "autoload" while (A) might be
better named "preload".
Your `;;;###autoload-expand` was a fix for that core problem, in a sense.
The distinction between the two is not that clear when you consider that
a `;;;###autoload` cookie placed on a `defun` may arrange to autoload
that file when calling that function but it may *also* preload some
`put`s extracted from the `declare`.
Maybe a "general" solution would be to introduce a `;;;###preload`.
Not sure it's worth the trouble, tho.
Also, I'm not sure how "general" this would be, since the same problem
will tend to happen with any package which `;;;###autoload` a call to
a macro that's not predefined if that package uses its own
`FOO-loaddefs.el`, even if that macro is defined in some other library.
So, the general problem is: your patch will sometimes `eval-buffer` for
a buffer which doesn't expect to be evaluated before its package's
`FOO-loaddefs.el` has been created. I suspect that in most cases this
can be fixed by replacing
(require 'foo-loaddefs)
by
(require 'foo-loaddefs nil t)
It's not ideal, but I think it's acceptable. Maybe it won't always be
sufficient either. E.g. in `cl-lib` we solved this problem (for
a different reason) with:
(unless (load "cl-loaddefs" 'noerror 'quiet)
;; When bootstrapping, cl-loaddefs hasn't been built yet!
(require 'cl-macs)
(require 'cl-seq)
;; FIXME: Arguably we should also load `cl-extra', except that this
;; currently causes more bootstrap troubles, and `cl-extra' is
;; rarely used, so instead we explicitly (require 'cl-extra) at
;; those rare places where we do need it.
)
BTW, in the specific case of Tramp, I see:
(defmacro tramp--with-startup (&rest body)
"Schedule BODY to be executed at the end of tramp.el."
`(add-hook 'tramp--startup-hook (lambda nil ,@body)))
so I think `tramp--with-startup` *is* one of the macros that would
benefit from your new `(autoload macro-expand)` declaration.
> [1] Note that the order of declare entries already matters, because some
> of them generate forms which include `:autoload-end', and this
> token, once encountered _anywhere_, puts an absolute stop to
> emitting additional declare-generated forms into loaddefs.
> E.g. `(declare (debug ...))' does this. This, by the way, is why
> /some/ macros have their (e.g.) `indent' declares passed through to
> the loaddefs files, and others do not.
Really? Sounds like a bug. Are you sure?
(macroexpand '(defmacro my-foo () (declare (debug t) (indent 1)) (help)))
gives me:
(prog1 (defalias 'my-foo (cons 'macro #'(lambda nil (help))))
(progn :autoload-end (put 'my-foo 'edebug-form-spec 't))
(function-put 'my-foo 'lisp-indent-function 1))
and AFAIK the `:autoload-end` truncation is done with
(let ((end (memq :autoload-end form)))
(when end ;Cut-off anything after the :autoload-end marker.
(setq form (copy-sequence form))
(setcdr (memq :autoload-end form) nil))
so it should only truncate to:
(prog1 (defalias 'my-foo (cons 'macro #'(lambda nil (help))))
(progn :autoload-end)
(function-put 'my-foo 'lisp-indent-function 1))
and thus keep the subsequent `indent` declaration intact.
> [2] Some of these macros aren't yet suited for full expansion anyway.
> For example `define-derived-mode' includes a bunch of unwanted forms
> if it gets the expansion treatment. So for now, leaving the
> shortcut list 2 intact seems to be the lesser evil.
Interesting.
> +(defalias 'byte-run--set-autoload
> + #'(lambda (name _args spec)
> + (list 'function-put (list 'quote name)
> + ''autoload (list 'quote spec))))
I think the choice of `autoload` as name of symbol property is too
generic and runs too high a risk of clashing with other uses.
> (defconst loaddefs--function-like-operators
"operators" doesn't sound quite right.
Maybe "loaddefs--defining-macros"?
> If EXPANSION is non-nil, we're processing the macro expansion of an
> -expression, in which case we want to handle forms differently."
> +expression, in which case we want to handle forms differently. Note
> +that macros can request expansion by including `(autoload macro-expand)'
> +among their `declare' forms."
Nitpick: I'd start "Note..." on its own line. We're not trying to
justify-fill the text. 🙂
> @@ -196,30 +212,35 @@ loaddefs-generate--make-autoload
> (cdr form)))))
> (when exps (cons 'progn exps)))))
>
> - ;; For complex cases, try again on the macro-expansion.
> - ((and (memq car '(easy-mmode-define-global-mode define-global-minor-mode
> - define-globalized-minor-mode defun defmacro
> - easy-mmode-define-minor-mode define-minor-mode
> - define-inline cl-defun cl-defmacro cl-defgeneric
> - cl-defstruct pcase-defmacro iter-defun cl-iter-defun))
> - (macrop car)
> - (setq expand (let ((load-true-file-name file)
> - (load-file-name file))
> - (macroexpand form)))
> - (memq (car expand) '(progn prog1 defalias)))
> + ;; For macros which request it, try again on their expansion.
> + ((progn
> + ;; If the car is entirely unknown, we load the file first to
> + ;; give packages a chance to define their macros.
> + (unless (or (null car) (macrop car) (functionp car) (special-form-p car)
I'd test (and (symbolp car) (not (fboundp car)))
> + (memq car '(defclass defcustom deftheme defgroup))
This deserves a comment in the code.
> + (alist-get file load-history)
`alist-get` defaults to testing with `eq` so it won't work well with
a string key. 🙁
I'd go with `assoc`.
> + (and (macrop car)
> + (eq 'macro-expand ; a macro can declare (autoload macro-expand)
> + (or (get car 'autoload)
> + (get (car-safe (last (function-alias-p car))) 'autoload)))
I'd have expected `function-get` here.
> - ;; For special function-like operators, use the `autoload' function.
> - ((memq car '(define-skeleton define-derived-mode
> - define-compilation-mode define-generic-mode
> - easy-mmode-define-global-mode define-global-minor-mode
> - define-globalized-minor-mode
> - easy-mmode-define-minor-mode define-minor-mode
> - cl-defun defun* cl-defmacro defmacro*
> - define-overloadable-function
> - transient-define-prefix transient-define-suffix
> - transient-define-infix transient-define-argument))
> + ;; For special function-like operators, use the `autoload'
> + ;; function.
> + ((memq car loaddefs--function-like-operators)
> (let* ((macrop (memq car '(defmacro cl-defmacro defmacro*)))
> (name (nth 1 form))
> (args (pcase car
If we swap this case with the preceding one we can remove the
(memq car loaddefs--function-like-operators)
from the `unless` of that case.
[ And it probably results in a diff that's harder to read, so thanks for
not doing it this time. 🙂 ]
> @@ -257,8 +278,8 @@ loaddefs-generate--make-autoload
> t)
> (and (eq (car-safe (car body)) 'interactive)
> ;; List of modes or just t.
> - (or (if (nthcdr 1 (car body))
> - (list 'quote (nthcdr 1 (car body)))
> + (or (if (nthcdr 2 (car body))
> + (list 'quote (nthcdr 2 (car body)))
> t))))
> ,(if macrop ''macro nil)))))
I think you can just push this to `master` as separate patch, thank you.
> @@ -19835,7 +19836,7 @@
> loaddefs.el output file, and the rest are the directories to
> use.")
> (load "theme-loaddefs.el" t)
> -(register-definition-prefixes "loaddefs-gen" '("autoload-" "generated-autoload-" "loaddefs-generate--" "no-update-autoloads"))
> +(register-definition-prefixes "loaddefs-gen" '("autoload-" "generated-autoload-" "loaddefs-" "my/tmp-lddg-list2" "no-update-autoloads"))
Maybe better use `autoload-` rather than `loaddefs-` as prefix for the
new definitions.
[ I don't see `my/tmp-lddg-list2` in the patch you sent, so I assume it
was something temporary. ]
Stefan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.