unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: daniel sutton <danielsutton01@gmail.com>
To: Drew Adams <drew.adams@oracle.com>
Cc: Michael Heerdegen <michael_heerdegen@web.de>,
	emacs-devel <emacs-devel@gnu.org>
Subject: Re: sea-level rise of byte-compilation warnings [was: Fixing...byte-compilation warnings...]
Date: Sun, 15 Nov 2015 10:42:41 -0600	[thread overview]
Message-ID: <CAOLS0DP+brJBU9wWH8fez5+as7Rhv_gY8GY-SDtuPY0ouHkg1g@mail.gmail.com> (raw)
In-Reply-To: <ad749b9d-df0f-4d46-b098-8a4bd5fe50a8@default>

[-- 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 --]

  reply	other threads:[~2015-11-15 16:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAOLS0DP+brJBU9wWH8fez5+as7Rhv_gY8GY-SDtuPY0ouHkg1g@mail.gmail.com \
    --to=danielsutton01@gmail.com \
    --cc=drew.adams@oracle.com \
    --cc=emacs-devel@gnu.org \
    --cc=michael_heerdegen@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).