From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: daniel sutton Newsgroups: gmane.emacs.devel Subject: Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...] Date: Sun, 15 Nov 2015 10:42:41 -0600 Message-ID: References: <5645F670.9040601@online.de> <56460E2B.10603@cs.ucla.edu> <87io54c0es.fsf@web.de> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a11407fec340264052496fb2c X-Trace: ger.gmane.org 1447704534 13931 80.91.229.3 (16 Nov 2015 20:08:54 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 16 Nov 2015 20:08:54 +0000 (UTC) Cc: Michael Heerdegen , emacs-devel To: Drew Adams Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Nov 16 21:08:53 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZyQ57-0007Y9-Ad for ged-emacs-devel@m.gmane.org; Mon, 16 Nov 2015 21:08:53 +0100 Original-Received: from localhost ([::1]:48576 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyQ56-0001Fu-TB for ged-emacs-devel@m.gmane.org; Mon, 16 Nov 2015 15:08:52 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44980) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zy0O5-0005EN-0o for emacs-devel@gnu.org; Sun, 15 Nov 2015 11:42:47 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zy0O3-0004OO-7U for emacs-devel@gnu.org; Sun, 15 Nov 2015 11:42:44 -0500 Original-Received: from mail-io0-x231.google.com ([2607:f8b0:4001:c06::231]:33542) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zy0O3-0004OK-1W for emacs-devel@gnu.org; Sun, 15 Nov 2015 11:42:43 -0500 Original-Received: by iouu10 with SMTP id u10so132738397iou.0 for ; Sun, 15 Nov 2015 08:42:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=OQamSysAgEl789UBoi2jGKQn2sNgqTZ0QxmM2MMcnGc=; b=alhrBaPt5pdR8rNwLfcZsaWrUSFCQyQIY93VgHVuTuGi3Su8AfngHzd81PWj/7SSgR CiCb/mHwp/6sQSBaZT4Lwo5YyU/xPWXVkuLCsb+Dyi5C1yAcP7EgC6QW3SkbzX79Yon0 OcAQzhEuttsI7wUzCSEsHKs2T9LIUS6OR2ejsM93f72NT3kPYA5g+5h2C3e5HPdrkRbe b2vlRTGjCDSOThK9qPglfCykmrTlvHq8UjjvOh5bXt2ADzTGgiR2a3/7zxPLemXCioq3 bn/WUNhagIW/BlIEZv1Te9kNesBKQUAT3rycs131Bm3xYaFDew2OLy0oNAlclBaC8biL 0ASw== X-Received: by 10.107.39.193 with SMTP id n184mr20845556ion.14.1447605761735; Sun, 15 Nov 2015 08:42:41 -0800 (PST) Original-Received: by 10.107.15.229 with HTTP; Sun, 15 Nov 2015 08:42:41 -0800 (PST) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2607:f8b0:4001:c06::231 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:194531 Archived-At: --001a11407fec340264052496fb2c Content-Type: text/plain; charset=UTF-8 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 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. > --001a11407fec340264052496fb2c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Drew.

My thinking was very localized= to just this example, but i'll explain how I understand it and why i t= hink it is the way to go.

In the display-completio= ns-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 arg= ument will be removed. Thus consumers of the display-completions-list *shou= ld 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-complet= ions-list, we still maintain legacy behavior until at some point we remove = the functionality, after enough time that others have been able to see warn= ings and fix their own code. The idea is that the warning lets others know = of an api change and update.

So my thinking was th= at the recursive call should not generate a warning since we are only hitti= ng the old two argument function call when somewhere else has called with t= he old style. Presumably, the external call saw the warning and will update= their convention. My understanding is that this function is generating a w= arning because it still handles the outdated way to call this code.=C2=A0

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 th= at all calls to the old style of two arguments generate a warning unless th= ey are called from within the original defun of display-completions-list. I= t 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 th= is older behavior. The line inside of this function is:
=C2= =A0 (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 ag= ree with you that drowning in a sea of worthless warnings is bad, and that&= #39;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-compli= ant code still works until the optional argument is removed. The reason tha= t its important because its in the core is that this error is generated whe= n compiling emacs.=C2=A0

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.=C2=A0
<= br>
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?=C2=A0 Why is that TRT?

Or by "consumers" perhaps you meant new core code as
opposed to old core code?=C2=A0 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).=C2=A0 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.=C2=A0 The current
case is an exception that proves the rule.=C2=A0 And I'm glad
that the "core" occasionally gets a taste of its own
medicine in this regard.=C2=A0 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"?=C2=A0 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
=C2=A0 showing ones that are less important

* be able to have the code itself easily (and
=C2=A0 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.=C2=A0 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.=C2=A0 But misuse would of course mean
missing some relevant ones.=C2=A0 And then there is the
need to explicitly include the macro definitions when
using older Emacs versions that did not define them.

--001a11407fec340264052496fb2c--