bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

`help-make-xrefs' in `help-mode.el' calls `cl-some', which is defined in
`cl-extra.el' as an autoload.  Should we really be loading `cl-extra.el'
now, as soon as a user does, for example, `C-h f'?  In that case, what
is really "extra" about it (for interactive use of Emacs, at least)?

I thought that the point of creating `cl-lib.el' was to give people a
library of the most-used CL constructs and still let them avoid loading
all of `cl.el'.  If we are, in effect, loading `cl-extra.el' now nearly
by default then what's the point of separating out `cl-lib.el'?

Is this use of `cl-some' perhaps just due to laziness?  It's not hard to
get the same effect without using it and loading `cl-extra.el'.



In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-11-15
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --without-compress-install 'CFLAGS=-O2
 -static -g3''

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

It's also not clear to me why replacing the 3 `cond' clauses
used previously by the clause that uses `cl-some' is TRT (an
improvement). Seems to me like a degradation (bug), but I
don't have a test case.

Was that done to fix some bug?

The previous code was able to in some cases determine that
the symbol is a face or that it is a variable or that it is
a function.  That was replaced by the simple `cl-some' clause,
which just treats it as a symbol (producing all possible help
for it).

If that's the approach, why not just do that in all cases?

It looks like someone came up with `describe-symbol-backends'
for some other purpose and decided it could be reused here:
only the second element of each 3-element list entry is used
here.

Could you please state the rationale behind this change,
even if that is not directly related to this bug, which is
about unnecessarily loading `cl-extra.el'.  Thx.

These are the 3 `cond' clauses that were replaced by the
generic `cl-some' clause:

((and
  (facep sym)
  (save-match-data (looking-at "[ \t\n]+face\\W")))
 (help-xref-button 8 'help-face sym))
((and (or (boundp sym)
	  (get sym 'variable-documentation))
      (fboundp sym))
 ;; We can't intuit whether to use the
 ;; variable or function doc -- supply both.
 (help-xref-button 8 'help-symbol sym))
((and
  (or (boundp sym)
      (get sym 'variable-documentation))
  (or
   (documentation-property
    sym 'variable-documentation)
   (documentation-property
    (indirect-variable sym)
    'variable-documentation)))
 (help-xref-button 8 'help-variable sym))
((fboundp sym)
 (help-xref-button 8 'help-function sym)))))))

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[npostavs]

Drew Adams <drew.adams@oracle.com> writes:

> I thought that the point of creating `cl-lib.el' was to give people a
> library of the most-used CL constructs and still let them avoid loading
> all of `cl.el'.  If we are, in effect, loading `cl-extra.el' now nearly
> by default then what's the point of separating out `cl-lib.el'?

AFAIK, the point of cl-lib is to have the CL constructs in a separate
namespace, so that loading cl-lib doesn't change the semantics of
existing code that might not expect it (unlike cl.el).

This change was done by [1: 59b5723c9b], the commit message is a bit
terse.  But it looks like the idea is to use `describe-symbol-backends'
for this in order to make it extendable, specifically to let cl-extra
add an entry for classes.

1: 2015-07-07 02:14:16 -0400 59b5723c9b613f14cd60cd3239cfdbc0d2343b18
  Add online-help support to describe types

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

> > I thought that the point of creating `cl-lib.el' was to give people a
> > library of the most-used CL constructs and still let them avoid loading
> > all of `cl.el'.  If we are, in effect, loading `cl-extra.el' now nearly
> > by default then what's the point of separating out `cl-lib.el'?
> 
> AFAIK, the point of cl-lib is to have the CL constructs in a separate
> namespace, so that loading cl-lib doesn't change the semantics of
> existing code that might not expect it (unlike cl.el).

What part of `cl.el' changes the semantics of existing [non-cl.el]
code?  It too uses prefix `cl-', for exactly the reason of not
changing things.

OK, there are some aliases, such as `case' for `cl-case'.  But I
thought that such was the case only for situations where Emacs
without cl.el did not have such a function/macro/etc. - such as
`case'.

It is true that there is also stuff in cl.el that IMO does not
belong there, because it is not a Common-Lisp emulation.  Stuff
like `lexical-let'.  Such stuff should be elsewhere.

There is also stuff that was misguidely given the `cl-' prefix
which has no counterpart in Common Lisp.  (I've pointed that out
to emacs-devel, but no one seemed to care...)

It is also the case that there is stuff that has the same name
as Common Lisp stuff but which is a very poor emulation of it
and should not use the C-L name.  An example is `flet'.  This
is a really good example of something bad.  Its doc string even
tells you that if you want something closer to Common Lisp use
`cl-flet'.  Unlike, e.g., `case' and `cl-case', `flet' is not
an alias of `cl-flet' (nor should it be).

Aside from such messes (and there are a bunch), I think that
cl.el, like cl-lib.el, separates its stuff from non-cl.el
stuff in Emacs.  I don't think there are cases of changing
semantics.  (But if there are, those should be fixed.)

At any rate, this bug report is about `cl-extra.el'.  It has
long been considered, rightfully, as _extra_ stuff.  (And it
is not loaded by loading `cl.el'.)  If you take a look at what
is in it then I think you'll agree that it is hardly stuff that
needs to be loaded by default.

And when I say "loaded by default" I speak loosely, including
the case of a user using `C-h f' and - whoops! - suddenly
cl-extra gets loaded.  That should not happen.  And it certainly
should not happen just because someone wanted to use `cl-some'.
That use case is such a trivial one to code in other ways.  The
only reasons I can think of for someone using `cl-some' here are
ignorance (that it is defined in cl-extra.el, laziness, or
I-don't-care.

> This change was done by [1: 59b5723c9b], the commit message is a bit
> terse.  But it looks like the idea is to use `describe-symbol-backends'
> for this in order to make it extendable, specifically to let cl-extra
> add an entry for classes.

Yes, I see what `cl-some' is used for, here.  But even if one
wants to do what is done with `describe-symbol-backends' (which
IMO has  a bad name and whose addition didn't gain us anything,
but that's OK), there is no need to use `cl-some' to do it.
One can use `describe-symbol-backends' easily without `cl-some'
(as I know you realize).

> 1: 2015-07-07 02:14:16 -0400 59b5723c9b613f14cd60cd3239cfdbc0d2343b18
>   Add online-help support to describe types

To be clear, my objection is not to the change that makes use of
`describe-symbol-backends'.  It is to the gratuitous use of
`cl-some' which pulls in cl-extra by an autoload.  No need; not
good.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> OK, there are some aliases, such as `case' for `cl-case'.  But I
> thought that such was the case only for situations where Emacs
> without cl.el did not have such a function/macro/etc. - such as
> `case'.

Doesn't cl.el define more than 170 of such aliases?


Michael.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

> > OK, there are some aliases, such as `case' for `cl-case'.  But I
> > thought that such was the case only for situations where Emacs
> > without cl.el did not have such a function/macro/etc. - such as
> > `case'.
> 
> Doesn't cl.el define more than 170 of such aliases?

Dunno, and I don't really care.  The number is not so important.
What can be problematic is if such an alias _clashes_ with a
different construct of the same name for Emacs without cl.el.

I also said:

> > Aside from such messes (and there are a bunch), I think that
> > cl.el, like cl-lib.el, separates its stuff from non-cl.el
> > stuff in Emacs.  I don't think there are cases of changing
> > semantics.  (But if there are, those should be fixed.)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

But again, this bug report is about cl-extra.el, not cl.el.
The single use of `cl-some' here is gratuitous.  There is
no good reason that I'm aware of why a commonly used help
function should use `cl-some'.  There are a zillion simple
ways to do the same thing that is done by `cl-some' here.

If this is not fixed (and even if it is, alas), the door is
open for any number of such abuses.  The direction that leads
is just inclusion of cl.el (or similar that has the same
effect).

Emacs Dev has not decided to include cl.el by default.
If it does, then OK, it does.  But in that case a different
restructuring and probably some other name-change
rationalization will also be needed.

This bug and its fix are simple - trivial even.  You could
even use `cl-loop', no doubt.  That's a macro, so no problem.
(Of course, fixing it without using anything `cl-*' is also
simple.)

I see no reason why anyone would not want this fixed.
Especially considering that if the flood gates are opened
to all cl*.el constructs then there will be a certain amount
of adjusting to do - both for Emacs Dev (cleanup, factoring,
whatever) and for users.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Michael Heerdegen]

Drew Adams <drew.adams@oracle.com> writes:

> But again, this bug report is about cl-extra.el, not cl.el.
> The single use of `cl-some' here is gratuitous.  There is
> no good reason that I'm aware of why a commonly used help
> function should use `cl-some'.  There are a zillion simple
> ways to do the same thing that is done by `cl-some' here.

Yes, it would probably not be wrong to avoid it in this case.


Michael.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Philipp Stephani]

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

Drew Adams <drew.adams@oracle.com> schrieb am Di., 7. Feb. 2017 um
03:23 Uhr:

> > > I thought that the point of creating `cl-lib.el' was to give people a
> > > library of the most-used CL constructs and still let them avoid loading
> > > all of `cl.el'.  If we are, in effect, loading `cl-extra.el' now nearly
> > > by default then what's the point of separating out `cl-lib.el'?
> >
> > AFAIK, the point of cl-lib is to have the CL constructs in a separate
> > namespace, so that loading cl-lib doesn't change the semantics of
> > existing code that might not expect it (unlike cl.el).
>
> What part of `cl.el' changes the semantics of existing [non-cl.el]
> code?


Loading cl.el modifies the observable behavior of dolist.  For example,
insert the following into /tmp/dolist.el:

;;; -*- lexical-binding: t; -*-
(eval-when-compile (require 'cl-lib))
(message "Without cl.el")
(cl-dolist (i '(1 2 3))
  (dolist (j '(a b c))
    (message "i = %s, j = %s" i j)
    (when (= i 2) (cl-return))))
(message "\nWith cl.el")
(eval-when-compile (require 'cl))
(cl-dolist (i '(1 2 3))
  (dolist (j '(a b c))
    (message "i = %s, j = %s" i j)
    (when (= i 2) (cl-return))))

And then run:

$ emacs -Q -batch -l /tmp/dolist.el
Without cl.el
i = 1, j = a
i = 1, j = b
i = 1, j = c
i = 2, j = a

With cl.el
i = 1, j = a
i = 1, j = b
i = 1, j = c
i = 2, j = a
i = 3, j = a
i = 3, j = b
i = 3, j = c

[-- Attachment #2: Type: text/html, Size: 2384 bytes --]

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

>> > > I thought that the point of creating `cl-lib.el' was to give people a
>> > > library of the most-used CL constructs and still let them avoid loading
>> > > all of `cl.el'.  If we are, in effect, loading `cl-extra.el' now nearly
>> > > by default then what's the point of separating out `cl-lib.el'?
>> >
>> > AFAIK, the point of cl-lib is to have the CL constructs in a separate
>> > namespace, so that loading cl-lib doesn't change the semantics of
>> > existing code that might not expect it (unlike cl.el).
>>
>> What part of `cl.el' changes the semantics of existing [non-cl.el]
>> code?
>
> Loading cl.el modifies the observable behavior of dolist.

Yes, such is legacy.  See what I wrote about needing to fix
such problems (name clashes).

The fact that plain Emacs `dolist' is massaged to make it
DTRT for cl.el under that same name is wrong.  And then
cl-macs.el leverages that advice for `cl-dolist'.

The Emacs emulation of Common-Lisp `dolist' should be called
(only) `cl-dolist' - or else the Emacs thing by that name
should be renamed to something else.

Emacs has generally opted to co-opt some names for its own
version of a behavior, instead of giving the Emacs version
a different name.  Because of that we should not allow a
Common-Lisp emulation of such a thing use the same name.
It should use the `cl-' prefix, since the unprefixed name
is already taken.

Aliasing (or doing something that has the same effect, as
for `dolist') the plain name to the name with prefix `cl-'
is the wrong thing to do, in such cases.  We should alias
to remove prefix `cl-' only when the result is not a name
clash.


But again, this bug report is about unnecessarily
autoloading `cl-extra.el'.  It is not about `cl.el'.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Noam Postavsky]

On Tue, Feb 7, 2017 at 6:00 PM, Drew Adams <drew.adams@oracle.com> wrote:
> The Emacs emulation of Common-Lisp `dolist' should be called
> (only) `cl-dolist'

This is exactly what cl-lib.el does. cl.el does the wrong thing, which
is why its use is discouraged.

>
> But again, this bug report is about unnecessarily
> autoloading `cl-extra.el'.  It is not about `cl.el'.

Does the fact that cl-extra sets byte-compile-dynamic help with your
concerns at all?

byte-compile-dynamic is a variable defined in `bytecomp.el'.
[...]
Documentation:
If non-nil, compile function bodies so they load lazily.
They are hidden in comments in the compiled file,
and each one is brought into core when the
function is called.
[...]

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

> > The Emacs emulation of Common-Lisp `dolist' should be called
> > (only) `cl-dolist'
> 
> This is exactly what cl-lib.el does. cl.el does the wrong thing,
> which is why its use is discouraged.

Agreed, as far as that goes.  But `dolist' is an exception
(it is the only one, though).  There are _relatively_ few
such name clashes.

> > But again, this bug report is about unnecessarily
> > autoloading `cl-extra.el'.  It is not about `cl.el'.
> 
> Does the fact that cl-extra sets byte-compile-dynamic help
> with your concerns at all? ...
> byte-compile-dynamic is a variable defined in `bytecomp.el'.

Yes, I think it does.  I wasn't aware of this feature.

From (elisp) `Dynamic Loading' it appears that there is
no bug here.  If you think so too then please close.

Why don't more (even most) of the files distributed with
Emacs have this variable set (to non-nil)?  Searching the
Emacs 25.1 distributed Lisp files, I find only these that
use the variable:

dired-aux.el
dired-x.el
wdired.el
wid-edit.el
calendar.el
cl-extra.el
cl-lib.el
cl-macs.el
cl-seq.el
sendmail.el
executable.el
pcvs-util.el

Why (only) those?

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[npostavs]

tags 25627 notabug
close 25627
quit

Drew Adams <drew.adams@oracle.com> writes:
>
>> > But again, this bug report is about unnecessarily
>> > autoloading `cl-extra.el'.  It is not about `cl.el'.
>> 
>> Does the fact that cl-extra sets byte-compile-dynamic help
>> with your concerns at all? ...
>> byte-compile-dynamic is a variable defined in `bytecomp.el'.
>
> Yes, I think it does.  I wasn't aware of this feature.
>
> From (elisp) `Dynamic Loading' it appears that there is
> no bug here.  If you think so too then please close.
>
> Why don't more (even most) of the files distributed with
> Emacs have this variable set (to non-nil)?

Loading functions one by one can be slower than loading them all at
once.  Some justification would be needed for adding this (e.g., if a
file contains many independent functions that would usually be used in
isolation).

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

> > Why don't more (even most) of the files distributed with
> > Emacs have this variable set (to non-nil)?
> 
> Loading functions one by one can be slower than loading them all at
> once.  Some justification would be needed for adding this (e.g., if a
> file contains many independent functions that would usually be used in
> isolation).

Yes, I read that.  A guess would be that that would be the case
for many more files than the few that use it now.

Files could also be split, to factor out functions for which
that is true.

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[npostavs]

Drew Adams <drew.adams@oracle.com> writes:

>> > Why don't more (even most) of the files distributed with
>> > Emacs have this variable set (to non-nil)?
>> 
>> Loading functions one by one can be slower than loading them all at
>> once.  Some justification would be needed for adding this (e.g., if a
>> file contains many independent functions that would usually be used in
>> isolation).
>
> Yes, I read that.  A guess would be that that would be the case
> for many more files than the few that use it now.
>
> Files could also be split, to factor out functions for which
> that is true.

Possibly, but I would guess the benefits of doing this would be rather
tiny.  I mean, the best we can hope for is to save a few hundred
kilobytes of RAM, right?

bug#25627: 25.1; `help-make-xrefs' loads `cl-extra.el' now

[Drew Adams]

> >> > Why don't more (even most) of the files distributed with
> >> > Emacs have this variable set (to non-nil)?
> >>
> >> Loading functions one by one can be slower than loading them all at
> >> once.  Some justification would be needed for adding this (e.g., if a
> >> file contains many independent functions that would usually be used in
> >> isolation).
> >
> > Yes, I read that.  A guess would be that that would be the case
> > for many more files than the few that use it now.
> >
> > Files could also be split, to factor out functions for which
> > that is true.
> 
> Possibly, but I would guess the benefits of doing this would be rather
> tiny.  I mean, the best we can hope for is to save a few hundred
> kilobytes of RAM, right?

Why is it more important for (all of) the files where it has been
done than for other libraries where there are multiple, essentially
independent pieces (including independent commands)?  What about
things like org, calc, gnus?  If it helps to do it to something like
calendar, an (uninformed) guess would say it might help to do it to
libraries like those.  Org, in particular, has lots of different
kinds of stuff in it.  I'd imagine there are bits that are relatively
independent.  Just guessing.