GNU bug report logs - #78995
[PATCH] ;;;autoload-expand for special macros

Previous Next

Full log

View this message in rfc822 format

From: "J.D. Smith" <jdtsmith <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 78995 <at> debbugs.gnu.org
Subject: bug#78995: [PATCH] ;;;autoload-expand for special macros
Date: Sat, 26 Jul 2025 21:46:46 -0400

[Message part 1 (text/plain, inline)]

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

Thanks for your feedback.

>>> 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?  ]

I suppose I had imagined multiple flags, à la:

  (autoload (macro-expand something-else))

but maybe that's frowned on.  In which case we should just go with
`(autoload-macro . expand)'.

> 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".

That's a sensible way of looking at it.

> 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.

Could be considered radical.  I guess the contract would be that all
`;;;###preload's would happen before all `;;;autoload's?  I can imagine
people confusing these two (similar to use-package's :init vs. :config).

> 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.

It would be general if it were universally adopted, and autoload
scanning proceeded across all files, first generating "stub" loaddefs
for preloads, then, in a subsequent pass, doing the autoloads.

The main issue as I see it is we are now (for the first time) possibly
/loading/ files, vs. just scanning them passively.  I think the
situation where this distinction matters is quite rare.  Note that the
only file(s) where a load are attempted now are for TRAMP.

> 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.

What about big packages (like tramp) that maintain and load their own
autoloads, and would like to request macro expansion for some of their
package-defined macros, and then also /use/ that macro in autoloads?
E.g. imagine if `tramp--with-startup' wanted to be expanded instead of
included as-is ("preloaded").  That's a pretty specific case, obviously.

> 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.

Ah right, my imagining above was not so misguided then.  Maybe the tramp
maintainer will want to expand that simple macro away now.  I guess
package authors have to decide:

- Do you want to include ("preload") your full macro and any autoloaded
  uses of it (as is) in *-loaddefs.el?  You should be sure to require
  your package's custom autoload file with suppressed errors, in case it
  doesn't yet exist during build.

- Would you prefer your package's macro is expanded at autoload
  generation time instead?  You should require it "for real", and
  declare it (autoload-macro expand).

- You can't get both behaviors.

>> [1] Note that the order of declare entries already matters, 
>
> Really?  Sounds like a bug.  Are you sure?

You're right, it was a transitory bug.  I've moved them all back to the
end (since they are most recently added that seems to make sense).

>> (defconst loaddefs--function-like-operators
>
> "operators" doesn't sound quite right.
> Maybe "loaddefs--defining-macros"?

Yeah I agree, I was picking up the comment below that:

     ;; For special function-like operators, use the `autoload'
     ;; function.

But I like your version better (and have changed the comment).

>> +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.  🙂

Done (even an extra blank to call it out). 

>> +        ;; 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)))

For whatever reason, I get lots of nil cars.  So how about:

          (unless (or (not (symbolp car)) (fboundp car) (null car)
                      ... ;; try to load the file

Is

(eq (fboundp x) (or (macrop x) (functionp x) (special-form-p x))) ∀x∈ (symbolp x)?

>> +                    (memq car '(defclass defcustom deftheme defgroup))
> This deserves a comment in the code.

Done.

> I'd go with `assoc`.

Thanks.

>> +        (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.

I was honestly vague on the difference.  Seems like `function-get'
should maybe chase aliases for you...

> 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.  🙂 ]

Good idea. I only recently decided this was necessary.  Though it isn't
really needed anymore with this change, I think keeping a defconst is a
good idea.

>> @@ -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.

I've attached a separate patch here (I don't have commit privileges).

>> @@ -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.  ]

This last file was an inadvertent inclusion.

Please take a look at the attached patches, and give a try with your own
user macro declaring autoload-expand if you can.  If this all looks good
I can add to the docs and NEWS.

[no-arg-descriptor-in-autoload.patch (text/x-patch, attachment)]

[autoload-expand_4.patch (text/x-patch, attachment)]

Previous Next

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