* Fixing compilation and byte-compilation warnings before 25.1
@ 2015-11-13 1:47 John Wiegley
2015-11-13 12:18 ` Juanma Barranquero
` (2 more replies)
0 siblings, 3 replies; 29+ messages in thread
From: John Wiegley @ 2015-11-13 1:47 UTC (permalink / raw)
To: emacs-devel
I'm doing an Emacs build now to test a patch, and it occurs to me how many
warnings and byte-compilation warnings I'm seeing. I'd like us to resolve as
many of these warnings as we can before shipping. It makes the build output
look a bit... unkempt.
Anyone interested in becoming a champion for this cause?
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 1:47 Fixing compilation and byte-compilation warnings before 25.1 John Wiegley
@ 2015-11-13 12:18 ` Juanma Barranquero
2015-11-13 14:40 ` Andreas Röhler
2015-11-14 10:55 ` Stephen Leake
2 siblings, 0 replies; 29+ messages in thread
From: Juanma Barranquero @ 2015-11-13 12:18 UTC (permalink / raw)
To: Emacs developers
[-- Attachment #1: Type: text/plain, Size: 241 bytes --]
On Fri, Nov 13, 2015 at 2:47 AM, John Wiegley <jwiegley@gmail.com> wrote:
> Anyone interested in becoming a champion for this cause?
I'm interested in helping. Championing... is difficult because I don't
build on non-Windows environments.
[-- Attachment #2: Type: text/html, Size: 354 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 1:47 Fixing compilation and byte-compilation warnings before 25.1 John Wiegley
2015-11-13 12:18 ` Juanma Barranquero
@ 2015-11-13 14:40 ` Andreas Röhler
2015-11-13 15:37 ` Artur Malabarba
2015-11-13 15:46 ` John Wiegley
2015-11-14 10:55 ` Stephen Leake
2 siblings, 2 replies; 29+ messages in thread
From: Andreas Röhler @ 2015-11-13 14:40 UTC (permalink / raw)
To: emacs-devel; +Cc: John Wiegley, Juanma Barranquero
On 13.11.2015 02:47, John Wiegley wrote:
> I'm doing an Emacs build now to test a patch, and it occurs to me how many
> warnings and byte-compilation warnings I'm seeing. I'd like us to resolve as
> many of these warnings as we can before shipping. It makes the build output
> look a bit... unkempt.
>
> Anyone interested in becoming a champion for this cause?
>
> John
>
Hi John,
keeping a reasonable warning-management IMO this is another crucial
point, because people for now get drowned.
Suggest as a first step to abolish all pure warnings, keeping errors
only. I.e. tell if something is broken or very probably broken. Abolish
all pure warnings, style warnings etc.
Then let's establish a policy for warnings - which needs an api, which
didn't exist or was not right when looked into some years ago.
Cheers,
Andreas
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 14:40 ` Andreas Röhler
@ 2015-11-13 15:37 ` Artur Malabarba
2015-11-14 8:34 ` Andreas Röhler
2015-11-13 15:46 ` John Wiegley
1 sibling, 1 reply; 29+ messages in thread
From: Artur Malabarba @ 2015-11-13 15:37 UTC (permalink / raw)
To: Andreas Röhler; +Cc: Juanma Barranquero, John Wiegley, emacs-devel
2015-11-13 14:40 GMT+00:00 Andreas Röhler <andreas.roehler@online.de>:
> keeping a reasonable warning-management IMO this is another crucial point,
> because people for now get drowned.
> Suggest as a first step to abolish all pure warnings, keeping errors only.
> I.e. tell if something is broken or very probably broken. Abolish all pure
> warnings, style warnings etc.
AFAICT the byte-compiler doesn't throw "style warnings". The closest I
can think of that are the "obsolete function/variable" warnings
Every other warning thrown by the byte-compiler (if memory serves) is
a legitimate "things could go wrong here".
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 14:40 ` Andreas Röhler
2015-11-13 15:37 ` Artur Malabarba
@ 2015-11-13 15:46 ` John Wiegley
2015-11-13 16:22 ` Paul Eggert
1 sibling, 1 reply; 29+ messages in thread
From: John Wiegley @ 2015-11-13 15:46 UTC (permalink / raw)
To: Andreas Röhler; +Cc: Juanma Barranquero, emacs-devel
>>>>> Andreas Röhler <andreas.roehler@online.de> writes:
> Suggest as a first step to abolish all pure warnings, keeping errors only.
> I.e. tell if something is broken or very probably broken. Abolish all pure
> warnings, style warnings etc.
Can you clarify what you mean by "abolish"? Is that fixing, or silencing?
> Then let's establish a policy for warnings - which needs an api, which
> didn't exist or was not right when looked into some years ago.
The policy should be: We never have warnings in our build on GNU Linux. Being
warning free on other platforms is also desirable, but more the responsibility
of contributors to those platforms.
I'm not sure what the "api" is refer you to, though...
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 15:46 ` John Wiegley
@ 2015-11-13 16:22 ` Paul Eggert
2015-11-13 23:00 ` John Wiegley
0 siblings, 1 reply; 29+ messages in thread
From: Paul Eggert @ 2015-11-13 16:22 UTC (permalink / raw)
To: Andreas Röhler, emacs-devel, Juanma Barranquero
On 11/13/2015 07:46 AM, John Wiegley wrote:
> The policy should be: We never have warnings in our build on GNU Linux
That's a good goal, but the devil's in the details and the details
aren't written down. Here's a first cut at the current situation;
comments welcome. I suppose something like this should go into a text
file somewhere....
I run "./configure --enable-gcc-warnings" on Fedora x86-64 with the
latest GCC, and rewrite the code to fix all C-level warnings that this
uncovers. This doesn't fix all warnings for older GCC versions, or for
non-GCC compilers, or for x86 or sparc, or for code that is #ifdeffed
out or not compiled on my plastform, etc. Some of these are not worth
the hassle (e.g., older compilers) as the cost of code-clutter is not
worth the benefit of possibly finding bugs with older compilers. Others
are worth doing (e.g., non-x86-64 platforms, non-Fedora distros) but I
lack the time and/or system access and it'd be nice if someone would
volunteer.
I also occasionally run valgrind on Emacs executables (actually,
temacs), and try to fix the warnings it generates. This is considerably
harder to do, but is a real nice thing to have on our checklist. (Right
now, for example, there are a couple of memory-allocation bugs that I
really would rather be fixing than writing administrative text like
this. :-)
It'd be nice if someone would volunteer to clean up Elisp warnings (and
thanks, Juanma, for volunteering to help). For these I see the following
classes:
* Warnings where people install changes to .el files but don't bother to
fix the warnings. I hope these are rare. We really need to get out of
the habit of doing this.
* Warnings where people change module X but don't clean up the warnings
that this causes to modules other than X. It's a hassle for developers
to catch these ('make compile-always' is little-known and expensive).
Perhaps our autobuild process could catch these and send email to the
developer whose changes to X causes warnings in Y.
* Warnings where people change the byte-compiler to generate new, useful
warnings, but don't clean up all the .el files accordingly. Here it can
be a bit much to expect the developer to clean up everyone else's code.
But it'd be nice if volunteers could strive to get these fixed while
preparing for a release.
* Warnings generated because code is intended to be portable to XEmacs
or to older GNU Emacs, and so uses deprecated interfaces on purpose. As
far as I know there's no convenient way to say "I know FOO is
deprecated, I want to use it anyway, don't issue a warning" in code that
will also work in older Emacs. This should get fixed.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 16:22 ` Paul Eggert
@ 2015-11-13 23:00 ` John Wiegley
2015-11-14 5:54 ` daniel sutton
0 siblings, 1 reply; 29+ messages in thread
From: John Wiegley @ 2015-11-13 23:00 UTC (permalink / raw)
To: Paul Eggert; +Cc: Juanma Barranquero, Andreas Röhler, emacs-devel
>>>>> Paul Eggert <eggert@cs.ucla.edu> writes:
> I also occasionally run valgrind on Emacs executables (actually, temacs),
> and try to fix the warnings it generates. This is considerably harder to do,
> but is a real nice thing to have on our checklist. (Right now, for example,
> there are a couple of memory-allocation bugs that I really would rather be
> fixing than writing administrative text like this. :-)
I'm also working at getting the Coverity scan of Emacs running again.
Warnings-free may not be practically achievement except in terms of some
"reference machine" (i.e., version of Linux + version of GCC), but having some
form of such a goal is better than not.
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 23:00 ` John Wiegley
@ 2015-11-14 5:54 ` daniel sutton
2015-11-14 10:58 ` Michael Heerdegen
2015-11-14 15:23 ` Fixing compilation and byte-compilation warnings before 25.1 Andy Moreton
0 siblings, 2 replies; 29+ messages in thread
From: daniel sutton @ 2015-11-14 5:54 UTC (permalink / raw)
To: Paul Eggert, Andreas Röhler, emacs-devel, Juanma Barranquero
[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]
Hello everyone. I'm a bit new to mucking about in the internals of emacs
but wanted to help out with cleaning up some compiler and byte compiler
warnings.
I'm building and noticing the error
minibuffer.el:1697:12 display-completion-list called with 2 arguments but
accepts only 1.
I'm a little confused as the this function has this signature:
(defun display-completion-list (completions &optional common-substring) ...)
In fact, this is a recursive call inside of the function
display-completion-list.
Can someone help me understand why we are getting a compiler warning about
seemingly valid usage of optional arguments?
On Fri, Nov 13, 2015 at 5:00 PM, John Wiegley <jwiegley@gmail.com> wrote:
> >>>>> Paul Eggert <eggert@cs.ucla.edu> writes:
>
> > I also occasionally run valgrind on Emacs executables (actually, temacs),
> > and try to fix the warnings it generates. This is considerably harder to
> do,
> > but is a real nice thing to have on our checklist. (Right now, for
> example,
> > there are a couple of memory-allocation bugs that I really would rather
> be
> > fixing than writing administrative text like this. :-)
>
> I'm also working at getting the Coverity scan of Emacs running again.
>
> Warnings-free may not be practically achievement except in terms of some
> "reference machine" (i.e., version of Linux + version of GCC), but having
> some
> form of such a goal is better than not.
>
> John
>
>
[-- Attachment #2: Type: text/html, Size: 2057 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 15:37 ` Artur Malabarba
@ 2015-11-14 8:34 ` Andreas Röhler
0 siblings, 0 replies; 29+ messages in thread
From: Andreas Röhler @ 2015-11-14 8:34 UTC (permalink / raw)
To: bruce.connor.am; +Cc: Juanma Barranquero, John Wiegley, emacs-devel
On 13.11.2015 16:37, Artur Malabarba wrote:
> 2015-11-13 14:40 GMT+00:00 Andreas Röhler<andreas.roehler@online.de>:
>> keeping a reasonable warning-management IMO this is another crucial point,
>> because people for now get drowned.
>> Suggest as a first step to abolish all pure warnings, keeping errors only.
>> I.e. tell if something is broken or very probably broken. Abolish all pure
>> warnings, style warnings etc.
> AFAICT the byte-compiler doesn't throw "style warnings". The closest I
> can think of that are the "obsolete function/variable" warnings
Don't remember the precise case, here is another one:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15122
> Every other warning thrown by the byte-compiler (if memory serves) is
> a legitimate "things could go wrong here".
BTW it should be possible to deactivate warnings at a single-symbol base.
For example kept (interactive-p) for a long time and still prefer it,
but got the buffer cluttered with warnings, so changed it just for the
sake of readability of remaining warnings.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-13 1:47 Fixing compilation and byte-compilation warnings before 25.1 John Wiegley
2015-11-13 12:18 ` Juanma Barranquero
2015-11-13 14:40 ` Andreas Röhler
@ 2015-11-14 10:55 ` Stephen Leake
2015-11-14 16:00 ` John Wiegley
2 siblings, 1 reply; 29+ messages in thread
From: Stephen Leake @ 2015-11-14 10:55 UTC (permalink / raw)
To: emacs-devel
John Wiegley <jwiegley@gmail.com> writes:
> I'm doing an Emacs build now to test a patch, and it occurs to me how many
> warnings and byte-compilation warnings I'm seeing. I'd like us to resolve as
> many of these warnings as we can before shipping. It makes the build output
> look a bit... unkempt.
>
> Anyone interested in becoming a champion for this cause?
A lot of the warnings are in lisp/cedet/*, and are caused by the changes
in eieio.el.
I've offered to help fix them, but I'm waiting on the pending merge from
sourceforge CEDET.
--
-- Stephe
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 5:54 ` daniel sutton
@ 2015-11-14 10:58 ` Michael Heerdegen
2015-11-14 18:22 ` daniel sutton
2015-11-14 15:23 ` Fixing compilation and byte-compilation warnings before 25.1 Andy Moreton
1 sibling, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2015-11-14 10:58 UTC (permalink / raw)
To: emacs-devel
daniel sutton <danielsutton01@gmail.com> writes:
> Can someone help me understand why we are getting a compiler warning
> about seemingly valid usage of optional arguments?
The cause is the defined `advertised-calling-convention' for
`display-completion-list'. Declaring an `advertised-calling-convention'
is the standard way to tell people that the signature of a function will
be changed in the future. It has been discussed some while ago whether
this way to provide this kind of information is appropriate.
Anyway, as long as the old signature is supported, the recursive call
will probably have to break the new calling convention.
Michael.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 5:54 ` daniel sutton
2015-11-14 10:58 ` Michael Heerdegen
@ 2015-11-14 15:23 ` Andy Moreton
1 sibling, 0 replies; 29+ messages in thread
From: Andy Moreton @ 2015-11-14 15:23 UTC (permalink / raw)
To: emacs-devel
On Fri 13 Nov 2015, daniel sutton wrote:
> Hello everyone. I'm a bit new to mucking about in the internals of emacs
> but wanted to help out with cleaning up some compiler and byte compiler
> warnings.
>
> I'm building and noticing the error
> minibuffer.el:1697:12 display-completion-list called with 2 arguments but
> accepts only 1.
>
> I'm a little confused as the this function has this signature:
> (defun display-completion-list (completions &optional common-substring) ...)
> In fact, this is a recursive call inside of the function
> display-completion-list.
>
> Can someone help me understand why we are getting a compiler warning about
> seemingly valid usage of optional arguments?
Look at the advertised-calling-convention form at the start of the
function. The calling convention has changed, and the optional argument
has been removed. See (info "(elisp) Declare Form").
AndyM
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 10:55 ` Stephen Leake
@ 2015-11-14 16:00 ` John Wiegley
2015-11-14 18:01 ` Stephen Leake
2015-11-15 9:08 ` David Engster
0 siblings, 2 replies; 29+ messages in thread
From: John Wiegley @ 2015-11-14 16:00 UTC (permalink / raw)
To: Stephen Leake; +Cc: emacs-devel
>>>>> Stephen Leake <stephen_leake@stephe-leake.org> writes:
> I've offered to help fix them, but I'm waiting on the pending merge from
> sourceforge CEDET.
Should we consider allowing this merge into 25.1?
John
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 16:00 ` John Wiegley
@ 2015-11-14 18:01 ` Stephen Leake
2015-11-15 9:08 ` David Engster
1 sibling, 0 replies; 29+ messages in thread
From: Stephen Leake @ 2015-11-14 18:01 UTC (permalink / raw)
To: emacs-devel
John Wiegley <jwiegley@gmail.com> writes:
>>>>>> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>
>> I've offered to help fix them, but I'm waiting on the pending merge from
>> sourceforge CEDET.
>
> Should we consider allowing this merge into 25.1?
Yes. It should be mostly bug fixes, but I guess we should see if there
are significant new features.
So it should be merged into a branch first, so we can decide to then
merge to master or emacs-25.
--
-- Stephe
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 10:58 ` Michael Heerdegen
@ 2015-11-14 18:22 ` daniel sutton
2015-11-15 12:41 ` Michael Heerdegen
2015-11-15 16:07 ` sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Drew Adams
0 siblings, 2 replies; 29+ messages in thread
From: daniel sutton @ 2015-11-14 18:22 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]
Ah thank you. Someone responded and made a new thread and were super
helpful as well. It seems like this warning needs to go to new consumers
but not in the core. Would it be appropriate for the declare statement to
somehow tell the compiler that we are in the core and therefore to suppress
warnings of this type? One suggestion was to add the recursive call into a
(with-no-warnings ...) call, but this could get tedious and invasive.
Perhaps there could be a list of ignorable warnings that could be
suppressed when in the core?
On Sat, Nov 14, 2015 at 4:58 AM, Michael Heerdegen <michael_heerdegen@web.de
> wrote:
> daniel sutton <danielsutton01@gmail.com> writes:
>
> > Can someone help me understand why we are getting a compiler warning
> > about seemingly valid usage of optional arguments?
>
> The cause is the defined `advertised-calling-convention' for
> `display-completion-list'. Declaring an `advertised-calling-convention'
> is the standard way to tell people that the signature of a function will
> be changed in the future. It has been discussed some while ago whether
> this way to provide this kind of information is appropriate.
>
> Anyway, as long as the old signature is supported, the recursive call
> will probably have to break the new calling convention.
>
>
> Michael.
>
>
>
[-- Attachment #2: Type: text/html, Size: 1841 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 16:00 ` John Wiegley
2015-11-14 18:01 ` Stephen Leake
@ 2015-11-15 9:08 ` David Engster
1 sibling, 0 replies; 29+ messages in thread
From: David Engster @ 2015-11-15 9:08 UTC (permalink / raw)
To: Stephen Leake; +Cc: emacs-devel
John Wiegley writes:
>>>>>> Stephen Leake <stephen_leake@stephe-leake.org> writes:
>
>> I've offered to help fix them, but I'm waiting on the pending merge from
>> sourceforge CEDET.
>
> Should we consider allowing this merge into 25.1?
I don't think there will be any major changes in this final merge. But
I'll prepare the merge in a separate branch anyway, so I'd say we can
decide this when that's ready. This will take a bit, though, because I
want to port our test suite first.
-David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-14 18:22 ` daniel sutton
@ 2015-11-15 12:41 ` Michael Heerdegen
2015-11-16 14:15 ` daniel sutton
2015-11-15 16:07 ` sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Drew Adams
1 sibling, 1 reply; 29+ messages in thread
From: Michael Heerdegen @ 2015-11-15 12:41 UTC (permalink / raw)
To: emacs-devel
daniel sutton <danielsutton01@gmail.com> writes:
> Ah thank you. Someone responded and made a new thread and were super
> helpful as well. It seems like this warning needs to go to new
> consumers but not in the core. Would it be appropriate for the declare
> statement to somehow tell the compiler that we are in the core and
> therefore to suppress warnings of this type?
I don't think that suppressing blindly all warnings of any particular
type is a good idea. Some of them might indicate problems.
> One suggestion was to add the recursive call into a (with-no-warnings
> ...) call, but this could get tedious and invasive.
Why not just leave this one particular warning? It reminds us we still
have something to change there in the future.
Michael.
^ permalink raw reply [flat|nested] 29+ messages in thread
* sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-14 18:22 ` daniel sutton
2015-11-15 12:41 ` Michael Heerdegen
@ 2015-11-15 16:07 ` Drew Adams
2015-11-15 16:42 ` daniel sutton
2015-11-16 23:48 ` Artur Malabarba
1 sibling, 2 replies; 29+ messages in thread
From: Drew Adams @ 2015-11-15 16:07 UTC (permalink / raw)
To: daniel sutton, Michael Heerdegen; +Cc: emacs-devel
> It seems like this warning needs to go to new consumers
> but not in the core.
You mean don't bother developers of the core, but just
bother everyone else? Why is that TRT?
Or by "consumers" perhaps you meant new core code as
opposed to old core code? Old code is still hacked on,
so warnings should be as appropriate for it as for new
code that consumes it.
> Perhaps there could be a list of ignorable warnings
> that could be suppressed when in the core?
Perhaps we should just remove one or two of the shiploads
of warnings that have been piled on in the last few years?
Or at least provide clear guidelines for 3rd-party
libraries, as to how to make the code itself warningless
when byte-compiled by its users.
The degree of warning is now overkill in many contexts
(even for novices, it could be argued). But it is
particularly annoying for package authors to have to
reassure users that warnings they see are "normal",
because of code that adapts to multiple Emacs versions.
This kind of warning is far more ubiquitous for 3rd-party
packages than it is for "core" code, which has less
(little) concern/need for such adaptation. The current
case is an exception that proves the rule. And I'm glad
that the "core" occasionally gets a taste of its own
medicine in this regard. Maybe that will help lead to a
saner approach to such warnings in general.
I don't see why the question of drowning in a sea of
typically worthless warnings is relevant only to "when
in the core"? What is so special about "the core" in
this regard?
Please find a solution for all Emacs developers and
users, not just those hacking on the "core".
And yes, it is useful to have (most) such warnings.
What is needed is to:
* be able to notice the important ones, while also
showing ones that are less important
* be able to have the code itself easily (and
conditionally) inhibit them - as a whole or by type
---
FWIW, my crystal ball whispers that just analyzing
the code for sexps that are protected by `fboundp'
might go a long way toward eliminating many spurious
warnings. I'm not saying that such analysis is easy
or that it would be super-accurate, but any such might
be an optional aid that users could take advantage of.
Another possibility might be to come up with a macro
or two that somehow encapsulates the use of things
like `fboundp' and perhaps even `featurep' or Emacs
version tests for accommodating different Emacs
versions.
(There were (are?) such macros for handling code that
is conditional for XEmacs vs GNU Emacs, for instance.)
Code that uses such macros could easily be optionally
(conditionally) made immune to the corresponding
irrelevant warnings. But misuse would of course mean
missing some relevant ones. And then there is the
need to explicitly include the macro definitions when
using older Emacs versions that did not define them.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-15 16:07 ` sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Drew Adams
@ 2015-11-15 16:42 ` daniel sutton
2015-11-15 17:38 ` Drew Adams
2015-11-16 23:48 ` Artur Malabarba
1 sibling, 1 reply; 29+ messages in thread
From: daniel sutton @ 2015-11-15 16:42 UTC (permalink / raw)
To: Drew Adams; +Cc: Michael Heerdegen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 5801 bytes --]
Hi Drew.
My thinking was very localized to just this example, but i'll explain how I
understand it and why i think it is the way to go.
In the display-completions-list, the function can take two arguments. There
is a declare statement that says it should only take 1 argument. My
understanding is that this is a change to the function signature, and that
in the future the optional argument will be removed. Thus consumers of the
display-completions-list *should get a warning*. People writing code that
target this should adapt to the new way of doing this, as should other
calls from inside of the core.
However, inside of the actual function display-completions-list, we still
maintain legacy behavior until at some point we remove the functionality,
after enough time that others have been able to see warnings and fix their
own code. The idea is that the warning lets others know of an api change
and update.
So my thinking was that the recursive call should not generate a warning
since we are only hitting the old two argument function call when somewhere
else has called with the old style. Presumably, the external call saw the
warning and will update their convention. My understanding is that this
function is generating a warning because it still handles the outdated way
to call this code.
My idea of a solution (in broad terms here, with no concrete implementation
yet) was to make sure that handling the antiquated calling from outside did
not itself generate a warning. So the idea was that all calls to the old
style of two arguments generate a warning unless they are called from
within the original defun of display-completions-list. It seems like a
duplicated error, as whoever originally called in could see that it should
only take one argument, but for a period we still support this older
behavior. The line inside of this function is:
(declare (advertised-calling-convention (completions) "24.4"))
So when the byte compiler sees a call that violates this
advertised-calling-convention, generate a warning *unless* it is inside of
the function where this declaration is found.
I agree with you that drowning in a sea of worthless warnings is bad, and
that's why I want to fix them. This is a worthless warning precisely
because, in a way, this recursive call outranks the warning. It ensures
non-compliant code still works until the optional argument is removed. The
reason that its important because its in the core is that this error is
generated when compiling emacs.
In this case, 3rd parties are given information about how to not generate
warnings: this warning is to only call display-completions-list with a
single argument. Once this is followed, the warnings cease.
On Sun, Nov 15, 2015 at 10:07 AM, Drew Adams <drew.adams@oracle.com> wrote:
> > It seems like this warning needs to go to new consumers
> > but not in the core.
>
> You mean don't bother developers of the core, but just
> bother everyone else? Why is that TRT?
>
> Or by "consumers" perhaps you meant new core code as
> opposed to old core code? Old code is still hacked on,
> so warnings should be as appropriate for it as for new
> code that consumes it.
>
> > Perhaps there could be a list of ignorable warnings
> > that could be suppressed when in the core?
>
> Perhaps we should just remove one or two of the shiploads
> of warnings that have been piled on in the last few years?
>
> Or at least provide clear guidelines for 3rd-party
> libraries, as to how to make the code itself warningless
> when byte-compiled by its users.
>
> The degree of warning is now overkill in many contexts
> (even for novices, it could be argued). But it is
> particularly annoying for package authors to have to
> reassure users that warnings they see are "normal",
> because of code that adapts to multiple Emacs versions.
>
> This kind of warning is far more ubiquitous for 3rd-party
> packages than it is for "core" code, which has less
> (little) concern/need for such adaptation. The current
> case is an exception that proves the rule. And I'm glad
> that the "core" occasionally gets a taste of its own
> medicine in this regard. Maybe that will help lead to a
> saner approach to such warnings in general.
>
> I don't see why the question of drowning in a sea of
> typically worthless warnings is relevant only to "when
> in the core"? What is so special about "the core" in
> this regard?
>
> Please find a solution for all Emacs developers and
> users, not just those hacking on the "core".
>
> And yes, it is useful to have (most) such warnings.
> What is needed is to:
>
> * be able to notice the important ones, while also
> showing ones that are less important
>
> * be able to have the code itself easily (and
> conditionally) inhibit them - as a whole or by type
>
> ---
>
> FWIW, my crystal ball whispers that just analyzing
> the code for sexps that are protected by `fboundp'
> might go a long way toward eliminating many spurious
> warnings. I'm not saying that such analysis is easy
> or that it would be super-accurate, but any such might
> be an optional aid that users could take advantage of.
>
> Another possibility might be to come up with a macro
> or two that somehow encapsulates the use of things
> like `fboundp' and perhaps even `featurep' or Emacs
> version tests for accommodating different Emacs
> versions.
>
> (There were (are?) such macros for handling code that
> is conditional for XEmacs vs GNU Emacs, for instance.)
>
> Code that uses such macros could easily be optionally
> (conditionally) made immune to the corresponding
> irrelevant warnings. But misuse would of course mean
> missing some relevant ones. And then there is the
> need to explicitly include the macro definitions when
> using older Emacs versions that did not define them.
>
[-- Attachment #2: Type: text/html, Size: 6686 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-15 16:42 ` daniel sutton
@ 2015-11-15 17:38 ` Drew Adams
2015-11-15 17:47 ` daniel sutton
0 siblings, 1 reply; 29+ messages in thread
From: Drew Adams @ 2015-11-15 17:38 UTC (permalink / raw)
To: daniel sutton; +Cc: Michael Heerdegen, emacs-devel
Hi Daniel,
I don't disagree with what you say, but your reply belongs
in your original topic ("Solving some specific warnings
(was: Fixing compilation and byte-compilation warnings
before 25.1)"), not in the new one I forked from it.
However, it was my bad to introduce this new topic by
asking a general question when replying to your statement
about this particular message. To my mind it brought up a
general problem. My response was not really to what you
were trying to say - sorry. I should have just started a
new topic, without referring to what you said.
And this part of your reply does pertain to the topic I
created:
> I agree with you that drowning in a sea of worthless
> warnings is bad, and that's why I want to fix them.
And perhaps this part:
> This is a worthless warning precisely because, in a
> way, this recursive call outranks the warning. It
> ensures non-compliant code still works until the
> optional argument is removed. The reason that its
> important because its in the core is that this error
> is generated when compiling emacs.
>
> In this case, 3rd parties are given information about
> how to not generate warnings: this warning is to only
> call display-completions-list with a single argument.
> Once this is followed, the warnings cease.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-15 17:38 ` Drew Adams
@ 2015-11-15 17:47 ` daniel sutton
2015-11-15 22:39 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: daniel sutton @ 2015-11-15 17:47 UTC (permalink / raw)
To: Drew Adams; +Cc: Michael Heerdegen, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]
Oh i apologize.
I'm still quite new to all of this and I don't have a firm understanding of
what third party creators go through. Do you have some code in mind that I
could byte compile to see these warnings so I can get a sense of what
issues there are? Granted I see lots of warnings when grabbing from Melpa
but I always dismiss the buffer. So maybe we could use one clear example to
have tangible issues and a canonical example of annoyances that third party
creators go through.
On Sun, Nov 15, 2015 at 11:38 AM, Drew Adams <drew.adams@oracle.com> wrote:
> Hi Daniel,
>
> I don't disagree with what you say, but your reply belongs
> in your original topic ("Solving some specific warnings
> (was: Fixing compilation and byte-compilation warnings
> before 25.1)"), not in the new one I forked from it.
>
> However, it was my bad to introduce this new topic by
> asking a general question when replying to your statement
> about this particular message. To my mind it brought up a
> general problem. My response was not really to what you
> were trying to say - sorry. I should have just started a
> new topic, without referring to what you said.
>
> And this part of your reply does pertain to the topic I
> created:
>
> > I agree with you that drowning in a sea of worthless
> > warnings is bad, and that's why I want to fix them.
>
> And perhaps this part:
>
> > This is a worthless warning precisely because, in a
> > way, this recursive call outranks the warning. It
> > ensures non-compliant code still works until the
> > optional argument is removed. The reason that its
> > important because its in the core is that this error
> > is generated when compiling emacs.
> >
> > In this case, 3rd parties are given information about
> > how to not generate warnings: this warning is to only
> > call display-completions-list with a single argument.
> > Once this is followed, the warnings cease.
>
[-- Attachment #2: Type: text/html, Size: 2508 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-15 17:47 ` daniel sutton
@ 2015-11-15 22:39 ` Drew Adams
0 siblings, 0 replies; 29+ messages in thread
From: Drew Adams @ 2015-11-15 22:39 UTC (permalink / raw)
To: daniel sutton; +Cc: Michael Heerdegen, emacs-devel
> Oh i apologize.
No, it was my bad, not yours.
(And I don't have any special example or library in mind.
There are various kinds of byte-compilation warnings that
can come up because libraries accommodate different other
libraries or different versions of Emacs. They range from
a function or variable being undefined because some library
is not available (loaded), or because it gets renamed, to
changes in arity of a function over time.)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-15 12:41 ` Michael Heerdegen
@ 2015-11-16 14:15 ` daniel sutton
2015-11-16 23:24 ` Artur Malabarba
0 siblings, 1 reply; 29+ messages in thread
From: daniel sutton @ 2015-11-16 14:15 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2790 bytes --]
There could be some value to that. My thinking is that the declaration that
there should be only one argument passed is kinda like a keep off the grass
sign, but then the compiler is throwing a warning that the keep off the
grass sign is actually on the grass. The consumers of this should have a
warning, but since we are handling legacy behavior (of passing two
arguments) there should be no warning inside of this function. When things
change, ie, if we remove the optional argument, then we would immediately
know if we forgot to change things because then this would be an error, as
the argument mismatch was very obvious.
I certainly agree that blindly suppressing all warnings is not a good call.
I'm investigating the way that the compiler works, as I'm very new to this.
My thinking is that we may have local definitions of
byte-compile-enable-warnings that include (not callargs) *only* when
compiling a function that contains a (declare
(advertised-calling-convention (newArgs) "version")). This would make sure
that others that hit this would be warned about the api shift but allow
"bad" calls to it to still work. Then, when the optional argument is
finally removed, if someone forgot to remove the old call, again, inside of
the original defun, it would be a compiler error and not warning. But
presumably, if you are changing the function signature like this you would
be responsible for the entire body of the function.
Again, this is a proposal only for this specific type of warning, and not
just suppressing warnings inside of the core. I want warnings to be
informative and a cause of action, not to be ignored. This warning is
superfluous and should be removed. If the best case is just a
(with-no-warnings ...) so be it, but I think there is a more clever
solution here, and one that actually fits the nature of what is being done
with a advertised-calling-convention change.
On Sun, Nov 15, 2015 at 6:41 AM, Michael Heerdegen <michael_heerdegen@web.de
> wrote:
> daniel sutton <danielsutton01@gmail.com> writes:
>
> > Ah thank you. Someone responded and made a new thread and were super
> > helpful as well. It seems like this warning needs to go to new
> > consumers but not in the core. Would it be appropriate for the declare
> > statement to somehow tell the compiler that we are in the core and
> > therefore to suppress warnings of this type?
>
> I don't think that suppressing blindly all warnings of any particular
> type is a good idea. Some of them might indicate problems.
>
> > One suggestion was to add the recursive call into a (with-no-warnings
> > ...) call, but this could get tedious and invasive.
>
> Why not just leave this one particular warning? It reminds us we still
> have something to change there in the future.
>
>
> Michael.
>
>
>
[-- Attachment #2: Type: text/html, Size: 3415 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Fixing compilation and byte-compilation warnings before 25.1
2015-11-16 14:15 ` daniel sutton
@ 2015-11-16 23:24 ` Artur Malabarba
0 siblings, 0 replies; 29+ messages in thread
From: Artur Malabarba @ 2015-11-16 23:24 UTC (permalink / raw)
To: daniel sutton; +Cc: emacs-devel
daniel sutton <danielsutton01@gmail.com> writes:
> My thinking is that we may have local definitions of
> byte-compile-enable-warnings that include (not callargs) *only* when
> compiling a function that contains a (declare
> (advertised-calling-convention (newArgs) "version")).
This wouldn't do.
This would mean that you would never get a callargs warning inside a
function that declares `advertised-calling-convention', _even_ warnings
that pertain to calls to _other_ functions.
So it would have to be smarter than that, and only suppress the warning
if it pertains to this very function.
> Again, this is a proposal only for this specific type of warning, and
> not just suppressing warnings inside of the core. I want warnings to
> be informative and a cause of action, not to be ignored. This warning
> is superfluous and should be removed. If the best case is just a
> (with-no-warnings ...) so be it, but I think there is a more clever
> solution here, and one that actually fits the nature of what is being
> done with a advertised-calling-convention change.
You should know, I don't think this sitation of “function with obsolete
calling convention that recursively calls itself” is actually that
common.
If you'd like to try your hand at this, by all means, have fun. :-)
The cause sounds reasonable to me, as long as the solution doesn't
turn out horribly complex it should be a nice addition.
I just thought you should know that.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-15 16:07 ` sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Drew Adams
2015-11-15 16:42 ` daniel sutton
@ 2015-11-16 23:48 ` Artur Malabarba
2015-11-16 23:52 ` Drew Adams
2015-11-17 3:59 ` Ivan Andrus
1 sibling, 2 replies; 29+ messages in thread
From: Artur Malabarba @ 2015-11-16 23:48 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
Drew Adams <drew.adams@oracle.com> writes:
> Perhaps we should just remove one or two of the shiploads
> of warnings that have been piled on in the last few years?
>
> Or at least provide clear guidelines for 3rd-party
> libraries, as to how to make the code itself warningless
> when byte-compiled by its users.
In my experience, 90% of the superfluous warnings are silenced by a
`defvar' or a `declare-function'. Finally, with-no-warnings should cover
the last 10. If this isn't well documented it really should be.
The reason we can't just remove one or two shiploads of these warnings
is that they could be pointing to actual problems (in my experience,
about 1/3 of the time they are), such as functions/variables that
weren't required.
> The degree of warning is now overkill in many contexts
> (even for novices, it could be argued).
I don't usually feel overwhelmed by the warnings. Most of my elisp files
have 0 to 3 of these warning-suppressing forms. Only the really large
projects with 10+ files end up having a lot.
I'm curious what are your contexts. I guess you're supporting Emacs
versions < 24.1?
> What is needed is to:
>
> * be able to notice the important ones, while also
> showing ones that are less important
Different warning levels might be nice, but (looking at
`byte-compile-warnings') the only warnings that don't immediately mean
“this code might break” are the `cl-functions', `obsolete', and
`mapcar'. So I don't know how much it would help your warning flood.
> * be able to have the code itself easily (and
> conditionally) inhibit them - as a whole or by type
Like I said, with-no-warnings inhibits them locally, but I perfectly
agree there should be a way to suppress warnings on a file-local (and
maybe dir-local) basis.
> FWIW, my crystal ball whispers that just analyzing
> the code for sexps that are protected by `fboundp'
> might go a long way toward eliminating many spurious
> warnings.
My psychic senses agree.
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-16 23:48 ` Artur Malabarba
@ 2015-11-16 23:52 ` Drew Adams
2015-11-17 0:09 ` Artur Malabarba
2015-11-17 3:59 ` Ivan Andrus
1 sibling, 1 reply; 29+ messages in thread
From: Drew Adams @ 2015-11-16 23:52 UTC (permalink / raw)
To: Artur Malabarba; +Cc: emacs-devel
> In my experience, 90% of the superfluous warnings are silenced by a
> `defvar' or a `declare-function'. Finally, with-no-warnings should cover
> the last 10. If this isn't well documented it really should be.
Those are not supported in all Emacs versions. That is the reason
I don't use them in some libraries.
> > The degree of warning is now overkill in many contexts
> > (even for novices, it could be argued).
>
> I don't usually feel overwhelmed by the warnings. Most of my elisp files
> have 0 to 3 of these warning-suppressing forms. Only the really large
> projects with 10+ files end up having a lot.
Different users will get different floods of warnings. The older
the Emacs versions your libraries support, the deeper the flood,
other things being equal.
The warnings don't really bother me. What bothers me somewhat
are the fact that they bother users of my libraries. I need to
let them know that there is no problem.
> I'm curious what are your contexts. I guess you're supporting Emacs
> versions < 24.1?
I am, for many of my libraries, but not for all. If a new library
makes use of constructs from Emacs N then I generally support only
N and greater.
> > What is needed is to:
> >
> > * be able to notice the important ones, while also
> > showing ones that are less important
>
> Different warning levels might be nice, but (looking at
> `byte-compile-warnings') the only warnings that don't immediately mean
> “this code might break” are the `cl-functions', `obsolete', and
> `mapcar'. So I don't know how much it would help your warning flood.
`obsolete' certainly does. I continue to (and will continue to) use
"obsolete" constructs.
But the topic is not about me or my use. It's a general topic.
> > * be able to have the code itself easily (and
> > conditionally) inhibit them - as a whole or by type
>
> Like I said, with-no-warnings inhibits them locally, but I perfectly
> agree there should be a way to suppress warnings on a file-local (and
> maybe dir-local) basis.
(FWIW, `with-no-warnings' is not available in Emacs 20.
Lots of my code works also in Emacs 20, intentionally.)
> > FWIW, my crystal ball whispers that just analyzing
> > the code for sexps that are protected by `fboundp'
> > might go a long way toward eliminating many spurious
> > warnings.
>
> My psychic senses agree.
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-16 23:52 ` Drew Adams
@ 2015-11-17 0:09 ` Artur Malabarba
2015-11-17 15:45 ` Drew Adams
0 siblings, 1 reply; 29+ messages in thread
From: Artur Malabarba @ 2015-11-17 0:09 UTC (permalink / raw)
To: Drew Adams; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 445 bytes --]
I was going to suggest something in my previous message but apparently I
forgot.
> > > * be able to have the code itself easily (and
> > > conditionally) inhibit them - as a whole or by type
> >
> > Like I said, with-no-warnings inhibits them locally, but I perfectly
> > agree there should be a way to suppress warnings on a file-local (and
> > maybe dir-local) basis.
Have you tried setting byte-compile-warnings as a file-local-variable?
[-- Attachment #2: Type: text/html, Size: 567 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-16 23:48 ` Artur Malabarba
2015-11-16 23:52 ` Drew Adams
@ 2015-11-17 3:59 ` Ivan Andrus
1 sibling, 0 replies; 29+ messages in thread
From: Ivan Andrus @ 2015-11-17 3:59 UTC (permalink / raw)
To: Artur Malabarba; +Cc: Drew Adams, emacs-devel
On Nov 16, 2015, at 4:48 PM, Artur Malabarba <bruce.connor.am@gmail.com> wrote:
>
> Drew Adams <drew.adams@oracle.com> writes:
>> FWIW, my crystal ball whispers that just analyzing
>> the code for sexps that are protected by `fboundp'
>> might go a long way toward eliminating many spurious
>> warnings.
>
> My psychic senses agree.
I thought this seemed like a reasonable (and potentially simple) thing to do, so I went looking and there is already `byte-compile-maybe-guarded’. Perhaps it isn’t working correctly or could be expanded to include more cases. What’s an example of such a spurious warning?
-Ivan
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
2015-11-17 0:09 ` Artur Malabarba
@ 2015-11-17 15:45 ` Drew Adams
0 siblings, 0 replies; 29+ messages in thread
From: Drew Adams @ 2015-11-17 15:45 UTC (permalink / raw)
To: bruce.connor.am; +Cc: emacs-devel
> Have you tried setting byte-compile-warnings as a file-local-variable?
Sounds reasonable. I don't care enough about this for my code, but
for the general issue raised, I suppose library authors could do that.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2015-11-17 15:45 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-13 1:47 Fixing compilation and byte-compilation warnings before 25.1 John Wiegley
2015-11-13 12:18 ` Juanma Barranquero
2015-11-13 14:40 ` Andreas Röhler
2015-11-13 15:37 ` Artur Malabarba
2015-11-14 8:34 ` Andreas Röhler
2015-11-13 15:46 ` John Wiegley
2015-11-13 16:22 ` Paul Eggert
2015-11-13 23:00 ` John Wiegley
2015-11-14 5:54 ` daniel sutton
2015-11-14 10:58 ` Michael Heerdegen
2015-11-14 18:22 ` daniel sutton
2015-11-15 12:41 ` Michael Heerdegen
2015-11-16 14:15 ` daniel sutton
2015-11-16 23:24 ` Artur Malabarba
2015-11-15 16:07 ` sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Drew Adams
2015-11-15 16:42 ` daniel sutton
2015-11-15 17:38 ` Drew Adams
2015-11-15 17:47 ` daniel sutton
2015-11-15 22:39 ` Drew Adams
2015-11-16 23:48 ` Artur Malabarba
2015-11-16 23:52 ` Drew Adams
2015-11-17 0:09 ` Artur Malabarba
2015-11-17 15:45 ` Drew Adams
2015-11-17 3:59 ` Ivan Andrus
2015-11-14 15:23 ` Fixing compilation and byte-compilation warnings before 25.1 Andy Moreton
2015-11-14 10:55 ` Stephen Leake
2015-11-14 16:00 ` John Wiegley
2015-11-14 18:01 ` Stephen Leake
2015-11-15 9:08 ` David Engster
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).