Fload does not set the 'autoload property

Fload does not set the 'autoload property

[Juanma Barranquero]

A bug (I think) reported in the pre-22.1 past:

Let's assume a simple test.el with:

;;;; test.el ;;;;
(defun test-fun () (interactive) t)
(provide 'test)
;;;;;;;;;;;;;;;;;

Then, if you define an interactive autoload for the function, and load
it through the autoload mechanism:

 M-: (autoload 'test-fun "test" nil t) [RET]
 M-x test-fun [RET]

the autoload spec gets saved:

 (get 'test-fun 'autoload) => ("test" nil t nil)

However, if you load test-fun by loading or requiring test.el:

 M-: (autoload 'test-fun "test" nil t) [RET]
 M-x load-library [RET] test [RET]

the autoload spec is not saved:

 (get 'test-fun 'autoload) => nil

             Juanma

Re: Fload does not set the 'autoload property

[Stefan Monnier]

> A bug (I think) reported in the pre-22.1 past:
> Let's assume a simple test.el with:

> ;;;; test.el ;;;;
> (defun test-fun () (interactive) t)
> (provide 'test)
> ;;;;;;;;;;;;;;;;;

> Then, if you define an interactive autoload for the function, and load
> it through the autoload mechanism:

>  M-: (autoload 'test-fun "test" nil t) [RET]
>  M-x test-fun [RET]

> the autoload spec gets saved:

>  (get 'test-fun 'autoload) => ("test" nil t nil)

> However, if you load test-fun by loading or requiring test.el:

>  M-: (autoload 'test-fun "test" nil t) [RET]
>  M-x load-library [RET] test [RET]

> the autoload spec is not saved:

>  (get 'test-fun 'autoload) => nil

Similarly, if your file contains 2 autoloads (foo1 and foo2), and the file
gets autoloaded via foo1, only foo1's autoload gets saved in the
`autoload' property.

The way autoloads are saved/restored is currently fundamentally done at the
wrong place.


        Stefan

Re: Fload does not set the 'autoload property

[Juanma Barranquero]

On 10/11/07, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Similarly, if your file contains 2 autoloads (foo1 and foo2), and the file
> gets autoloaded via foo1, only foo1's autoload gets saved in the
> `autoload' property.

Ah, I hadn't checked that. I found the autoload/load problem while
trying to determine why unload-feature fails to restore many
autoloads.

> The way autoloads are saved/restored is currently fundamentally done at the
> wrong place.

Not urgent enough for admin/FOR-RELEASE, but it should be noted in
etc/TODO then, shouldn't it?

             Juanma

Re: Fload does not set the `autoload' property

[Richard Stallman]

There is a deliberate reason for treating autoloading differently
from calls to `load' or `require'.

Calling an autoload function is a request to use that function, not a
request to load the file.  If loading the file fails to define the
function that you tried to call, then Emacs did not do what you asked
for.  So we put the function back as it was.

By contrast, calling `load' and `require' is an explicit request to
load the file.  Whatever the file does, that's what you asked for.  So
there is no particular need to try to undo anything that the file did.

However, it might be a bug that this affects the `autoload' property.
Maybe we should set the `autoload' property for every definition of a
function that was previously autoloaded.

This would probably mean setting the `autoload' property in Ffset
instead of in do_autoload.

If we change this, we should consider it a bug fix,
so we should do it now and fix it in Emacs 22.2.
Would someone like to try implementing that change?


Stefan wrote:

    Similarly, if your file contains 2 autoloads (foo1 and foo2), and the file
    gets autoloaded via foo1, only foo1's autoload gets saved in the
    `autoload' property.

That surprises me -- the code seems to do this for every autoloaded
function defined by the file, if it was loaded by do_autoload.

Re: Fload does not set the `autoload' property

[Stefan Monnier]

>     Similarly, if your file contains 2 autoloads (foo1 and foo2), and the file
>     gets autoloaded via foo1, only foo1's autoload gets saved in the
>     `autoload' property.

> That surprises me -- the code seems to do this for every autoloaded
> function defined by the file, if it was loaded by do_autoload.

Maybe my recollection is wrong, then.


        Stefan

Re: Fload does not set the `autoload' property

[Juanma Barranquero]

On 10/12/07, Richard Stallman <rms@gnu.org> wrote:

> However, it might be a bug that this affects the `autoload' property.
> Maybe we should set the `autoload' property for every definition of a
> function that was previously autoloaded.

That's what I think; it is necessary in order to do a proper unload-feature.

> If we change this, we should consider it a bug fix,
> so we should do it now and fix it in Emacs 22.2.
> Would someone like to try implementing that change?

I would lie if I said I know anything about that part of the code.
Perhaps the attached patch is similar to what you're thinking about?

             Juanma


Index: src/data.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/data.c,v
retrieving revision 1.278
diff -c -r1.278 data.c
*** src/data.c	10 Sep 2007 09:41:44 -0000	1.278
--- src/data.c	13 Oct 2007 01:13:10 -0000
***************
*** 663,674 ****
       (symbol, definition)
       register Lisp_Object symbol, definition;
  {
    CHECK_SYMBOL (symbol);
    if (NILP (symbol) || EQ (symbol, Qt))
      xsignal1 (Qsetting_constant, symbol);
!   if (!NILP (Vautoload_queue) && !EQ (XSYMBOL (symbol)->function, Qunbound))
!     Vautoload_queue = Fcons (Fcons (symbol, XSYMBOL (symbol)->function),
! 			     Vautoload_queue);
    XSYMBOL (symbol)->function = definition;
    /* Handle automatic advice activation */
    if (CONSP (XSYMBOL (symbol)->plist) && !NILP (Fget (symbol,
Qad_advice_info)))
--- 663,682 ----
       (symbol, definition)
       register Lisp_Object symbol, definition;
  {
+   register Lisp_Object function;
+
    CHECK_SYMBOL (symbol);
    if (NILP (symbol) || EQ (symbol, Qt))
      xsignal1 (Qsetting_constant, symbol);
!
!   function = XSYMBOL (symbol)->function;
!
!   if (!NILP (Vautoload_queue) && !EQ (function, Qunbound))
!     Vautoload_queue = Fcons (Fcons (symbol, function), Vautoload_queue);
!
!   if (CONSP (function) && EQ (XCAR (function), Qautoload))
!     Fput (symbol, Qautoload, XCDR (function));
!
    XSYMBOL (symbol)->function = definition;
    /* Handle automatic advice activation */
    if (CONSP (XSYMBOL (symbol)->plist) && !NILP (Fget (symbol,
Qad_advice_info)))
Index: src/eval.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/eval.c,v
retrieving revision 1.291
diff -c -r1.291 eval.c
*** src/eval.c	9 Oct 2007 08:52:52 -0000	1.291
--- src/eval.c	13 Oct 2007 01:07:20 -0000
***************
*** 2178,2184 ****
       Lisp_Object fundef, funname;
  {
    int count = SPECPDL_INDEX ();
!   Lisp_Object fun, queue, first, second;
    struct gcpro gcpro1, gcpro2, gcpro3;

    /* This is to make sure that loadup.el gives a clear picture
--- 2178,2184 ----
       Lisp_Object fundef, funname;
  {
    int count = SPECPDL_INDEX ();
!   Lisp_Object fun;
    struct gcpro gcpro1, gcpro2, gcpro3;

    /* This is to make sure that loadup.el gives a clear picture
***************
*** 2199,2218 ****
    Vautoload_queue = Qt;
    Fload (Fcar (Fcdr (fundef)), Qnil, Qt, Qnil, Qt);

-   /* Save the old autoloads, in case we ever do an unload.  */
-   queue = Vautoload_queue;
-   while (CONSP (queue))
-     {
-       first = XCAR (queue);
-       second = Fcdr (first);
-       first = Fcar (first);
-
-       if (SYMBOLP (first) && CONSP (second) && EQ (XCAR (second), Qautoload))
- 	Fput (first, Qautoload, (XCDR (second)));
-
-       queue = XCDR (queue);
-     }
-
    /* Once loading finishes, don't undo it.  */
    Vautoload_queue = Qt;
    unbind_to (count, Qnil);
--- 2199,2204 ----

Re: Fload does not set the `autoload' property

[Richard Stallman]

Your change looks right.  Please install it.