autoload cl-defstruct constructor?

autoload cl-defstruct constructor?

[Stephen Leake]

I have code like:

In ada-core.el:

(defun ada-prj-make-compiler (label)
  (funcall (intern (format "make-%s-compiler" (symbol-name label)))))

where 'label' is a user variable indicating choice of Ada compiler.

In gnat-core.el:

(cl-defstruct (gnat-compiler :include wisi-compiler)
   ...)

In foo-core.el, for another Ada compiler:

(cl-defstruct (foo-compiler :include wisi-compiler)
   ...)

The question is how to arrange autoloads so the correct constructor is
available, without loading the file(s) containing the unneeded one(s).

I tried adding an autoload cookie to the cl-defstruct. That almost
works, but update-directory-autoloads doesn't respect the order implied
by :include, so wisi-compiler is declared in autoloads.el after it is
needed.

Is there a way to influence the order of files included in autoloads
(short of renaming the files so alphabetical is correct)?

I could rename the constructor:

    (:constructor create-gnat-compiler)

and provide an ordinary autoloaded defun:

(defun make-gnat-compiler (...)
   (create-gnat-compiler ...))

But that seems inelegant, and maintaining the arg lists is tedious and
error-prone.

Is there a way to autoload just the created constructor?

-- 
-- Stephe

Re: autoload cl-defstruct constructor?

[Stefan Monnier]

> I tried adding an autoload cookie to the cl-defstruct. That almost
> works, but update-directory-autoloads doesn't respect the order implied
> by :include, so wisi-compiler is declared in autoloads.el after it is
> needed.

Could you give details about this ordering problem?

It sounds like a bug (tho fixing it will probably not help you in the
short-run if you need your solution to work in Emacs-26).


        Stefan

Re: autoload cl-defstruct constructor?

[Stephen Leake]

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

>> I tried adding an autoload cookie to the cl-defstruct. That almost
>> works, but update-directory-autoloads doesn't respect the order implied
>> by :include, so wisi-compiler is declared in autoloads.el after it is
>> needed.
>
> Could you give details about this ordering problem?

Hmm. Attempting to reproduce this, I ran into a different but related
problem.

If I add an autoload cookie to a cl-defstruct, running
update-directory-autoloads on the above gives:

---

;;; Generated autoloads from gnat-core.el

(cl-defstruct gnat-compiler "\
Used with wisi-compiler-* generic functions." gpr-file run-buffer-name project-path target runtime gnat-stub-opts gnat-stub-cargs)

---

There is no "autoload" call, as there would be with a normal function.
So even though 'make-gnat-compiler' is defined, calling it does not load
gnat-core.el, so no other functions are defined.

So more autoloads are required. Not really a problem, but unexpected.

> It sounds like a bug (tho fixing it will probably not help you in the
> short-run if you need your solution to work in Emacs-26).

The files are included in autoloads.el in file name alphabetical order
(although there do seem to be exceptions?). So with my real code, I get:

--- 

;;; Generated autoloads from ada-core.el

...

(cl-defstruct (ada-prj (:include wisi-prj) (:copier nil) (:constructor nil) (:constructor make-ada-prj (&key name compile-env (compiler-label ada-compiler) (xref-label ada-xref-tool) source-path plist file-pred &aux (compiler (ada-prj-make-compiler compiler-label)) (xref (ada-prj-make-xref xref-label))))) plist)

...

;;; Generated autoloads from wisi-prj.el

(cl-defstruct wisi-prj name compile-env file-env compiler xref (case-exception-files nil) (case-full-exceptions 'nil) (case-partial-exceptions 'nil) source-path file-pred)


---

At the point where 'ada-prj' is declared, 'wisi-prj' is unknown.

I can create a simple reproducer for a bug report, if you think that's
warrented.

For now, I'll try the renamed constructor approach.

-- 
-- Stephe

Re: autoload cl-defstruct constructor?

[Stefan Monnier]

> If I add an autoload cookie to a cl-defstruct, running
> update-directory-autoloads on the above gives:
>
> ---
>
> ;;; Generated autoloads from gnat-core.el
>
> (cl-defstruct gnat-compiler "\
> Used with wisi-compiler-* generic functions." gpr-file run-buffer-name
> project-path target runtime gnat-stub-opts gnat-stub-cargs)

Oh, right, because when the autoloads are generated `cl-defstruct` is
not defined at all.  If you run update-directory-autoloads from within
a running Emacs where cl-lib is loaded, it will give you a result that's
much closer to what you want.

Making cl-defstruct into an autoloaded macro would solve this part of
the problem.

> The files are included in autoloads.el in file name alphabetical order
> (although there do seem to be exceptions?).

I believe the ordering is deterministic (modulo bugs), but it's
arbitrary, so better not rely on it, indeed.

> (cl-defstruct (ada-prj (:include wisi-prj) (:copier nil) (:constructor nil)
> (:constructor make-ada-prj (&key name compile-env (compiler-label
> ada-compiler) (xref-label ada-xref-tool) source-path plist file-pred &aux
> (compiler (ada-prj-make-compiler compiler-label)) (xref (ada-prj-make-xref
> xref-label))))) plist)
>
> ...
>
> ;;; Generated autoloads from wisi-prj.el
>
> (cl-defstruct wisi-prj name compile-env file-env compiler xref
> (case-exception-files nil) (case-full-exceptions 'nil)
> (case-partial-exceptions 'nil) source-path file-pred)

Once autoload applies to the macro-expanded code, the problem still
exists: there's no remaining dependencies between the various (autoload
'wisi-...) but there's still a dependency between the calls to
`cl-define-struct`.

I think the patch below would solve that second problem.


        Stefan


diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 1ae7266624..05a4192dd9 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -3006,6 +3006,7 @@ cl-defstruct
     `(progn
        (defvar ,tag-symbol)
        ,@(nreverse forms)
+       :autoload-end
        ;; Call cl-struct-define during compilation as well, so that
        ;; a subsequent cl-defstruct in the same file can correctly include this
        ;; struct as a parent.

Re: autoload cl-defstruct constructor?

[Stephen Leake]

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

>> If I add an autoload cookie to a cl-defstruct, running
>> update-directory-autoloads on the above gives:
>>
>> ---
>>
>> ;;; Generated autoloads from gnat-core.el
>>
>> (cl-defstruct gnat-compiler "\
>> Used with wisi-compiler-* generic functions." gpr-file run-buffer-name
>> project-path target runtime gnat-stub-opts gnat-stub-cargs)
>
> Oh, right, because when the autoloads are generated `cl-defstruct` is
> not defined at all.  If you run update-directory-autoloads from within
> a running Emacs where cl-lib is loaded, it will give you a result that's
> much closer to what you want.

Hmm. To build autoloads.el for ada-mode in my development tree, I run:

emacs -Q -batch --eval "(progn (require 'autoload)(require 'cl-lib)(setq generated-autoload-file (expand-file-name \"../autoloads.el\"))(update-directory-autoloads \"../\"))"

I just added the (require 'cl-lib), but I still get the same result for
a cl-defstruct:

--- autoloads.el

;;; Generated autoloads from gnat-core.el

(cl-defstruct gnat-compiler "\
Used with wisi-compiler-* generic functions." gpr-file run-buffer-name project-path target runtime gnat-stub-opts gnat-stub-cargs)

(autoload 'create-gnat-compiler "gnat-core" "\
---

What am I missing?

We'll need to ensure package.el does the same when it generates
autoloads. Or is that done on the elpa server?

> Making cl-defstruct into an autoloaded macro would solve this part of
> the problem.

I guess that means adding a special case in make-autoload?

-- 
-- Stephe

Re: autoload cl-defstruct constructor?

[Stefan Monnier]

> Hmm. To build autoloads.el for ada-mode in my development tree, I run:
>
> emacs -Q -batch --eval "(progn (require 'autoload)(require 'cl-lib)(setq
> generated-autoload-file (expand-file-name
> \"../autoloads.el\"))(update-directory-autoloads \"../\"))"
>
> I just added the (require 'cl-lib), but I still get the same result for
> a cl-defstruct:

Hmm... indeed it looks like you also need the patch below.

> We'll need to ensure package.el does the same when it generates
> autoloads. Or is that done on the elpa server?

Indeed.  The autoloads are built when *installing* a package, so they're
built by the user's Emacs, which may be "old".


        Stefan


diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index 541b22e3ee..8ec5150cca 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -165,7 +165,7 @@ make-autoload
                        define-globalized-minor-mode defun defmacro
 		       easy-mmode-define-minor-mode define-minor-mode
                        define-inline cl-defun cl-defmacro cl-defgeneric
-                       pcase-defmacro))
+                       pcase-defmacro cl-defstruct))
            (macrop car)
 	   (setq expand (let ((load-file-name file)) (macroexpand form)))
 	   (memq (car expand) '(progn prog1 defalias)))

Re: autoload cl-defstruct constructor?

[Stephen Leake]

Applying both patches works nicely.

Should I commit this to master?

Now I have to figure out the minimum workaround for emacs 25, 26
compatibility.

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

>> Hmm. To build autoloads.el for ada-mode in my development tree, I run:
>>
>> emacs -Q -batch --eval "(progn (require 'autoload)(require 'cl-lib)(setq
>> generated-autoload-file (expand-file-name
>> \"../autoloads.el\"))(update-directory-autoloads \"../\"))"
>>
>> I just added the (require 'cl-lib), but I still get the same result for
>> a cl-defstruct:
>
> Hmm... indeed it looks like you also need the patch below.
>
>> We'll need to ensure package.el does the same when it generates
>> autoloads. Or is that done on the elpa server?
>
> Indeed.  The autoloads are built when *installing* a package, so they're
> built by the user's Emacs, which may be "old".
>
>
>         Stefan
>
>
> diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
> index 541b22e3ee..8ec5150cca 100644
> --- a/lisp/emacs-lisp/autoload.el
> +++ b/lisp/emacs-lisp/autoload.el
> @@ -165,7 +165,7 @@ make-autoload
>                         define-globalized-minor-mode defun defmacro
>  		       easy-mmode-define-minor-mode define-minor-mode
>                         define-inline cl-defun cl-defmacro cl-defgeneric
> -                       pcase-defmacro))
> +                       pcase-defmacro cl-defstruct))
>             (macrop car)
>  	   (setq expand (let ((load-file-name file)) (macroexpand form)))
>  	   (memq (car expand) '(progn prog1 defalias)))
>
>
>

-- 
-- Stephe

Re: autoload cl-defstruct constructor?

[Stephen Leake]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> Now I have to figure out the minimum workaround for emacs 25, 26
> compatibility.

This is a problem.

The first thing I have to do is disable the autoload on the cl-defstruct,
because leaving it in makes the autoload file fail due to declaration
order violations. I don't see a way to do that short of actually editing
the file.

The autoload cookie is handled in autoload--print-cookie-text; it does a
simple '(search-forward generate-autoload-cookie)', so surrounding the
cookie with '(if (< emacs-major-version 27)' will not have the desired
effect.

The only workaround I can see for the ELPA package is to create a new
package ada-mode-1 that requires emacs < 27, and bump the emacs require
for ada-mode to 27. Then I can use sed or something to edit the source
files for the ada-mode-1 package.

Of course, ELPA doesn't support an upper limit on a require version.

Another way to handle this and similar issues in the future is to add a
"preprocess files" step to the package.el build process, which is
normally null.

-- 
-- Stephe

Re: autoload cl-defstruct constructor?

[Stefan Monnier]

> Applying both patches works nicely.
> Should I commit this to master?

Sure,

> Now I have to figure out the minimum workaround for emacs 25, 26
> compatibility.

Good luck!  [ I think you're going to need it :-(  ]


        Stefan