Noisy byte compilation on master

Noisy byte compilation on master

[Eli Zaretskii]

Byte-compiling the recent master became really noisy lately, it spews
many dozens, if not hundreds, of warnings like this:

  In semantic-decoration-all-include-summary:
  cedet/semantic/decorate/include.el:803:58:Warning: `eieio-object-name-string'
      is an obsolete function (as of 25.1); use `eieio-named' instead.
  cedet/semantic/decorate/include.el:809:46:Warning: `eieio-object-name-string'
      is an obsolete function (as of 25.1); use `eieio-named' instead.
  `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
  `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
  `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
  `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
  `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
  `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.

and like this:

  Warning: Using brain-dead macro `mm-with-unibyte-current-buffer'!
  utf7.el: `mm-with-unibyte-current-buffer' is an obsolete macro (as of 25.1).

Would those who introduced these warnings please work on lowering the
noise level?  It's now so high that it effectively prevents one from
seeing the real problems flagged by the byte compiler, especially when
bootstrapping.

Thanks in advance.

Re: Noisy byte compilation on master

[Stefan Monnier]

>   In semantic-decoration-all-include-summary:
>   cedet/semantic/decorate/include.el:803:58:Warning: `eieio-object-name-string'
>       is an obsolete function (as of 25.1); use `eieio-named' instead.
>   cedet/semantic/decorate/include.el:809:46:Warning: `eieio-object-name-string'
>       is an obsolete function (as of 25.1); use `eieio-named' instead.
>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.

Not sure what to do about these: CEDET needs to work with older
Emascsen, so we can't change the code to use the cl-defmethod thingies yet.
But in the Emacs master code, `defmethod' is a compatibility layer which
introduces notable inefficiencies, so we do want to mark those as obsolete.

Maybe we should keep those as "not quite obsolete yet" and only add the
obsolescence warnings when we get to Emacs-26?  Or else, (define and)
add extra annotations in those CEDET files to silence those warnings?

>   Warning: Using brain-dead macro `mm-with-unibyte-current-buffer'!
>   utf7.el: `mm-with-unibyte-current-buffer' is an obsolete macro (as of 25.1).

This one is also problematic: I've been trying to reduce the use of this
macro for several years now, but I don't know the Gnus code enough to do
much further progress.  Yet, this macro is sufficiently ill-defined that
I think most of its uses probably suffer from latent bugs.  So spewing
warnings during compilation sounds like the right thing to do (tho
I see they're duplicated: one from the macro itself and one from the
obsolescence warning, so we can drop the "Warning:" message).


        Stefan

Re: Noisy byte compilation on master

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Mon, 02 Feb 2015 11:31:39 -0500
> 
> Not sure what to do about these: CEDET needs to work with older
> Emascsen, so we can't change the code to use the cl-defmethod thingies yet.
> But in the Emacs master code, `defmethod' is a compatibility layer which
> introduces notable inefficiencies, so we do want to mark those as obsolete.
> 
> Maybe we should keep those as "not quite obsolete yet" and only add the
> obsolescence warnings when we get to Emacs-26?  Or else, (define and)
> add extra annotations in those CEDET files to silence those warnings?

I don't have anything intelligent to say about this, except that the
amount of these messages is unbearably large.  Please find some way of
shutting them up.

> >   Warning: Using brain-dead macro `mm-with-unibyte-current-buffer'!
> >   utf7.el: `mm-with-unibyte-current-buffer' is an obsolete macro (as of 25.1).
> 
> This one is also problematic: I've been trying to reduce the use of this
> macro for several years now, but I don't know the Gnus code enough to do
> much further progress.  Yet, this macro is sufficiently ill-defined that
> I think most of its uses probably suffer from latent bugs.  So spewing
> warnings during compilation sounds like the right thing to do (tho
> I see they're duplicated: one from the macro itself and one from the
> obsolescence warning, so we can drop the "Warning:" message).

Again, the problem is the sheer number of those.

Re: Noisy byte compilation on master

[Stephen Leake]

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

>>   In semantic-decoration-all-include-summary:
>>   cedet/semantic/decorate/include.el:803:58:Warning: `eieio-object-name-string'
>>       is an obsolete function (as of 25.1); use `eieio-named' instead.
>>   cedet/semantic/decorate/include.el:809:46:Warning: `eieio-object-name-string'
>>       is an obsolete function (as of 25.1); use `eieio-named' instead.
>>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
>>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
>>   `defmethod' is an obsolete macro (as of 25.1); use `cl-defmethod' instead.
>>   `defgeneric' is an obsolete macro (as of 25.1); use `cl-defgeneric' instead.
>
> Not sure what to do about these: CEDET needs to work with older
> Emascsen, so we can't change the code to use the cl-defmethod thingies
> yet.

It seems to me that means CEDET should not be in core; it should be in
ELPA.

Alternately, non-core CEDET can provide a compatibility file that
defines cl-defmethod etc in terms of defmethod for older emacsen.

-- 
-- Stephe

Re: Noisy byte compilation on master

[Stefan Monnier]

> Alternately, non-core CEDET can provide a compatibility file that
> defines cl-defmethod etc in terms of defmethod for older emacsen.

Hmm... that's an idea.
I could provide a cl-generic.el on GNU ELPA to provide cl-defmethod via
EIEIO, and then CEDET could make use of the new macros.
WDYT?


        Stefan

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>> Alternately, non-core CEDET can provide a compatibility file that
>> defines cl-defmethod etc in terms of defmethod for older emacsen.
>
> Hmm... that's an idea.
> I could provide a cl-generic.el on GNU ELPA to provide cl-defmethod via
> EIEIO, and then CEDET could make use of the new macros.
> WDYT?

Fine with me. We could also put it in CEDET upstream so people don't
have to install it.

Note that I currently have no way of syncing CEDET with Emacs. So if you
have a script or similar to change the calling convention, please tell
me and I run it over the upstream code.

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

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

>>> Alternately, non-core CEDET can provide a compatibility file that
>>> defines cl-defmethod etc in terms of defmethod for older emacsen.
>> Hmm... that's an idea.
>> I could provide a cl-generic.el on GNU ELPA to provide cl-defmethod via
>> EIEIO, and then CEDET could make use of the new macros.
>> WDYT?
> Fine with me. We could also put it in CEDET upstream so people don't
> have to install it.

I have now added a cl-generic forward compatibility package to GNU ELPA.
It should hopefully work for any Emacs with EIEIO.

> Note that I currently have no way of syncing CEDET with Emacs.

Is there something we can do about that?

> So if you have a script or similar to change the calling convention,
> please tell me and I run it over the upstream code.

I used the sed script below along with:

    for f in $(grep -l '(defmethod' lisp/cedet/**/*.el); do
        echo $f
        (rm $f; ./_cl-generic-rewrite.sed >$f) <$f
    done


-- Stefan

[-- Attachment #2: _cl-generic-rewrite.sed --]
[-- Type: text/plain, Size: 586 bytes --]

#!/bin/sed -f

/^(require 'eieio)/a \
(require 'cl-generic)

/^(defmethod.*[^ ] *:[sS][tT][aA][tT][iI][cC] *$/{
   s/^(defmethod/(cl-defmethod/
   s/ *:[sS][tT][aA][tT][iI][cC] *//
   n
   s/^\([ 	]*(([^ ][^ ]* *\)\([^ ][^ ]*\)/\1(subclass \2)/
}
s/^(\(defmethod.*[^ ] *\):[sS][tT][aA][tT][iI][cC] *\((([^ ][^ ]* *\)\([^ ][^ ]*\)/(cl-\1\2(subclass \3)/
s/^(\(defmethod.*[^ ] *\):BEFORE/(cl-\1:before/
s/^(\(defmethod.*[^ ] *\):AFTER/(cl-\1:after/
s/^(defmethod/(cl-defmethod/

s/^(defgeneric/(cl-defgeneric/
s/(call-next-method/(cl-call-next-method/
s/(next-method-p/(cl-next-method-p/

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>>>> Alternately, non-core CEDET can provide a compatibility file that
>>>> defines cl-defmethod etc in terms of defmethod for older emacsen.
>>> Hmm... that's an idea.
>>> I could provide a cl-generic.el on GNU ELPA to provide cl-defmethod via
>>> EIEIO, and then CEDET could make use of the new macros.
>>> WDYT?
>> Fine with me. We could also put it in CEDET upstream so people don't
>> have to install it.
>
> I have now added a cl-generic forward compatibility package to GNU ELPA.
> It should hopefully work for any Emacs with EIEIO.

Well, it just occured to me that it's not that terribly useful to have
it in ELPA, as you need that macro during byte-compilation, and ELPA
packages are not available from 'emacs -Q'. So I guess I'll have to
install in CEDET upstream.

The bigger problem is that now I get a ton of warnings during CEDET
compilation because you use 'labels' instead of 'cl-labels', but with
good reason, as the commentary says. Not sure what to do about that...

>> Note that I currently have no way of syncing CEDET with Emacs.
>
> Is there something we can do about that?

We need a plan how to integrate CEDET easier now that both projects
moved to Git. I'm leaning towards importing lisp/cedet from a stable
upstream branch as a subtree. This would at least take care of merging
from CEDET to Emacs. The other way round is not as easy, but not
terribly difficult either.

The alternative would be that built-in ELPA thingy, but that's not
available yet. Also, if I understand you correctly, this would mean that
core packages cannot depend on CEDET. However, the advantage would be
that I could ditch a beloved pet-peeve of mine, namely editing that file
that shall not be named (hint: it logs changes).

>> So if you have a script or similar to change the calling convention,
>> please tell me and I run it over the upstream code.
>
> I used the sed script below along with:
>
>     for f in $(grep -l '(defmethod' lisp/cedet/**/*.el); do
>         echo $f
>         (rm $f; ./_cl-generic-rewrite.sed >$f) <$f
>     done

Thanks, that's very useful.

I've now pretty much removed EIEIO from CEDET upstream, so its
development is now solely in Emacs. On thing less to merge. Juhu!

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

> Well, it just occured to me that it's not that terribly useful to have
> it in ELPA, as you need that macro during byte-compilation, and ELPA
> packages are not available from 'emacs -Q'. So I guess I'll have to
> install in CEDET upstream.

Of course you could compile with an "emacs -Q --eval '(package-initialize)'"
or something like that, but if you prefer to bundle cl-generic.el that's
OK as well.

> The bigger problem is that now I get a ton of warnings during CEDET
> compilation because you use 'labels' instead of 'cl-labels', but with
> good reason, as the commentary says. Not sure what to do about that...

Try the 100% untested patch below, which should silence those warnings.

> The alternative would be that built-in ELPA thingy, but that's not
> available yet.

I really hope we can get this "bundle some ELPA packages in the tarball"
working for 25.1.

> Also, if I understand you correctly, this would mean that
> core packages cannot depend on CEDET.

Indeed, that'd be the downside.

> However, the advantage would be that I could ditch a beloved pet-peeve
> of mine, namely editing that file that shall not be named (hint: it
> logs changes).

The ChangeLog should disappear from master "any time now", so this
should be resolved in any case soonish.


        Stefan


diff --git a/packages/cl-generic/cl-generic.el b/packages/cl-generic/cl-generic.el
index df09449..8aa87f1 100644
--- a/packages/cl-generic/cl-generic.el
+++ b/packages/cl-generic/cl-generic.el
@@ -101,15 +101,15 @@
            ;; We could just alias `cl-call-next-method' to `call-next-method',
            ;; and that would work, but then files compiled with this cl-generic
            ;; wouldn't work in Emacs-25 any more.
-           ;; Also we use `labels' rather than one of cl-lib's macros, so as to
-           ;; be compatible with older emacsen (and ELPA's cl-lib emulation
-           ;; doesn't provide cl-flet and provides an incomplete cl-labels).
+           ;; Also we fallback on `labels' if `cl-flet' is not available
+           ;; (ELPA's cl-lib emulation doesn't provide cl-flet).
            ,@(if qualifiers
                  ;; Must be :before or :after, so can't call next-method.
                  body
-               `((labels ((cl-call-next-method (&rest args)
-                                               (apply #'call-next-method args))
-                          (cl-next-method-p () (next-method-p)))
+               `((,(if (fboundp 'cl-flet) 'cl-flet 'labels)
+                      ((cl-call-next-method (&rest args)
+                                            (apply #'call-next-method args))
+                       (cl-next-method-p () (next-method-p)))
                    ,@body))))))))
 
 (provide 'cl-generic)

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>> The bigger problem is that now I get a ton of warnings during CEDET
>> compilation because you use 'labels' instead of 'cl-labels', but with
>> good reason, as the commentary says. Not sure what to do about that...
>
> Try the 100% untested patch below, which should silence those warnings.

Thanks, it works.

The next problem are those "Obsolete name arg 'foo' to constructor"
messages. I cannot simply omit the names as this won't work with older
EIEIO.

I'm also trying to get the CEDET tests to work with latest Emacs. One
reason they break is that `child-of-class-p' does not accept symbols
anymore. This here

  (require 'ede/proj-elisp)
  (child-of-class-p 'ede-proj-target-elisp 'ede-target)

used to work fine, but now `child-of-class-p' has those cl-check-types
in it. Is that on purpose?

>> The alternative would be that built-in ELPA thingy, but that's not
>> available yet.
>
> I really hope we can get this "bundle some ELPA packages in the tarball"
> working for 25.1.
>
>> Also, if I understand you correctly, this would mean that
>> core packages cannot depend on CEDET.
>
> Indeed, that'd be the downside.

A pretty major one, I would say. CEDET is more like a framework than an
application. It'd be a shame if no core package could depend on anything
from it.

>> However, the advantage would be that I could ditch a beloved pet-peeve
>> of mine, namely editing that file that shall not be named (hint: it
>> logs changes).
>
> The ChangeLog should disappear from master "any time now", so this
> should be resolved in any case soonish.

That's not what I mean. CEDET has generated its Changelog from the
commit logs for years. The problem is that this generated Changelog has
to be *heavily* edited to confirm to the strict standards that are
required. For instance, there's the requirement that changes have to
appear under the date they entered Emacs trunk, and that you should not
have several entries from the same author for the same day. So you have
to look over all the changes from your merge and combine those that
affect the same files/functions/variables. Then there's the problem that
the Changelog will also contain changes from files that are only
upstream, which you'll have to delete. And on top of that, CEDET's files
are scattered across the Emacs codebase, affecting many different
Changelogs: not only that for 'lisp/cedet', but also 'lisp/emacs-lisp',
'admin', 'etc', 'doc', and there's even one for 'test'! So I have to
make sure to sort the entries into the correct Changelogs. All of this
is a *huge* pain, and - I'm sorry, but I have to repeat myself here -
for something I find to be not only useless, but even hurtful to the
quality of the commit messages of the project. If moving to ELPA gets
rid of Changelogs *completely* (meaning, also in the commit logs), that
would be big incentive for me to consider leaving Emacs trunk, although
I'd consider that a loss for Emacs core.

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

> The next problem are those "Obsolete name arg 'foo' to constructor"
> messages.  I cannot simply omit the names as this won't work with older
> EIEIO.

Indeed, you'd have to use `make-instance'.
Or you could use a macro which either drops the name argument or passes
it depending on the version of EIEIO with which it's compiled.

>   (require 'ede/proj-elisp)
>   (child-of-class-p 'ede-proj-target-elisp 'ede-target)
> used to work fine, but now `child-of-class-p' has those cl-check-types
> in it.  Is that on purpose?

Looks like a bug.  It's been changed to accept class arguments (rather
than class names), but it should also accept class names, as before.

>>> Also, if I understand you correctly, this would mean that
>>> core packages cannot depend on CEDET.
>> Indeed, that'd be the downside.
> A pretty major one, I would say.

Agreed.

> That's not what I mean.

I don't understand, then.

> For instance, there's the requirement that changes have to
> appear under the date they entered Emacs trunk,

Obviously, you won't be affected by this requirement any more once Emacs
starts generating the ChangeLogs mechanically.

> and that you should not have several entries from the same author for
> the same day.

Not sure where you ot that idea, but we don't have such a requirement.
I even often setup such multiple-entries-from-the-guy-same-day *by
hand*, when I feel like it helps structure the ChangeLog.

> Then there's the problem that the Changelog will also contain changes
> from files that are only upstream, which you'll have to delete.

How would that affect you, since "we" (i.e. Paul) write the scripts that
(will) generate the ChangeLogs (IIUC we'll generate a single ChangeLog
for the whole Emacs tree)?

> And on top of that, CEDET's files are scattered across the Emacs
> codebase, affecting many different Changelogs: not only that for
> 'lisp/cedet', but also 'lisp/emacs-lisp', 'admin', 'etc', 'doc', and
> there's even one for 'test'!

But once we start generating the ChangeLogs mechanically you won't need
to care any more (maybe we'll need to care if we want to generate
equivalent ChangeLogs, but so far the intention has been to do
something simpler).

> All of this is a *huge* pain,

I understand it's annoying, but I don't understand why you think that us
moving to auto-generated ChangeLogs won't solve those problems for you.


        Stefan

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>> The next problem are those "Obsolete name arg 'foo' to constructor"
>> messages.  I cannot simply omit the names as this won't work with older
>> EIEIO.
>
> Indeed, you'd have to use `make-instance'.

Rewriting all the constructor calls in CEDET would take me
ages. Besides, `make-instance' is too verbose for my taste.

> Or you could use a macro which either drops the name argument or passes
> it depending on the version of EIEIO with which it's compiled.

I would still have to rewrite the constructor calls, right?

> Not sure where you ot that idea, but we don't have such a requirement.
> I even often setup such multiple-entries-from-the-guy-same-day *by
> hand*, when I feel like it helps structure the ChangeLog.

Remember that I did merges maybe every few months. I had a ton of
entries for the same day as result, containing things like

 (foo): Change 'a' to 'c'.

 (foo): Revert previous change.

 (foo): Change 'a' to 'b'.

I was under the impression that stuff like this had to be cleared up.

[...]

> I understand it's annoying, but I don't understand why you think that us
> moving to auto-generated ChangeLogs won't solve those problems for you.

Sorry, I didn't know that all this could be handled automatically. I was
under the impression that I'd still have to edit the generated
logs. I'll be quiet, then. :-)

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

>> Or you could use a macro which either drops the name argument or passes
>> it depending on the version of EIEIO with which it's compiled.
> I would still have to rewrite the constructor calls, right?

Yes, but it should be a slightly simpler rewrite (no need to find&remove
the name string).

> Remember that I did merges maybe every few months. I had a ton of
> entries for the same day as result, containing things like
>  (foo): Change 'a' to 'c'.
>  (foo): Revert previous change.
>  (foo): Change 'a' to 'b'.
> I was under the impression that stuff like this had to be cleared up.

It's better good if you do, but it's not indispensable.

> Sorry, I didn't know that all this could be handled automatically.  I was
> under the impression that I'd still have to edit the generated logs.

The generation would happen as part of creating the tarball of
a release, so you wouldn't have the opportunity to edit it.


        Stefan

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>>> Or you could use a macro which either drops the name argument or passes
>>> it depending on the version of EIEIO with which it's compiled.
>> I would still have to rewrite the constructor calls, right?
>
> Yes, but it should be a slightly simpler rewrite (no need to find&remove
> the name string).

The real work is finding the constructor calls and making sure the
string isn't actually used for anything (which we'll have to do anyway,
as the string is not saved in Emacs25 unless explicitly set with
eieio-object-set-name-string).

I really do not want to clutter the code with some macro before every
constructor all. That would needlessly also end up in Emacs proper
through merging, and it would eventually have to be removed again when
we stop supporting Emacs24. I'm actually a bit confused, as I thought we
agreed that EIEIO should not emit a warning when the string is provided,
at least not during run-time. I could live with those warnings during
byte-compilation, as I would just shush those as long as we support
Emacs24.

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

> agreed that EIEIO should not emit a warning when the string is provided,
> at least not during run-time. I could live with those warnings during
> byte-compilation, as I would just shush those as long as we support
> Emacs24.

If you get such warnings at run-time, it's a bug.  They should only
appear at byte-compilation time.


        Stefan

Re: Noisy byte compilation on master

[David Engster]

Stefan Monnier writes:
>> agreed that EIEIO should not emit a warning when the string is provided,
>> at least not during run-time. I could live with those warnings during
>> byte-compilation, as I would just shush those as long as we support
>> Emacs24.
>
> If you get such warnings at run-time, it's a bug.  They should only
> appear at byte-compilation time.

Insert

  (require 'eieio)
  (defclass foo () ())
  (foo "test")

then do eval-buffer and you'll see

  Obsolete name arg "test" to constructor foo

-David

Re: Noisy byte compilation on master

[Stefan Monnier]

>> If you get such warnings at run-time, it's a bug.  They should only
>> appear at byte-compilation time.

> Insert

>   (require 'eieio)
>   (defclass foo () ())
>   (foo "test")

> then do eval-buffer and you'll see

>   Obsolete name arg "test" to constructor foo

That's not a bug.  Because `eval-buffer' is the "byte-compilation time".
For it to count as a bug, you'd have to byte-compile the above code and
get the warning when you run the resulting compiled code.


        Stefan