Solving some specific warnings (was: Fixing compilation and byte-compilation warnings before 25.1)

Solving some specific warnings (was: Fixing compilation and byte-compilation warnings before 25.1)

[Artur Malabarba]

Hi Daniel

2015-11-14 5:54 GMT+00:00 daniel sutton <danielsutton01@gmail.com>:
> 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.

Great!

> 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.

This is because `display-completion-list' includes the line (declare
(advertised-calling-convention (completions) "24.4")).

This means that, since 24.4, the old way of calling this function was
made obsolete, and it should only be called with one argument now.
Instead of just removing the second argument entirely (which would
break backwards compatibility of code out there) we "pretend" that the
function only takes one argument, so that new code won't use the
second argument.

Normally, the way to solve this warning is to update the way that the
function is being called to conform with the new arglist.
However, looking at the source here, it seems that if the clause `(not
(bufferp standard-output))' is true, then the function just wants to
call itself again (with the same arguments) in a temp-buffer, so you
shouldn't change that.

So it is safe to just mute this warning (and add a comment to the
function saying why). To mute the warning, wrap the form in a
`(with-no-warnings ...)'

Re: Solving some specific warnings (was: Fixing compilation and byte-compilation warnings before 25.1)

[daniel sutton]

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

Thank you so much for the response Artur. I actually saw someone post a
link to a blog post of yours about the using # before lambdas today as well
(just saying that your contributions are passed around the community and
are much appreciated).

I can add (with-no-warnings (recursive-call-to-deprecated-args-list
...)...) calls to the warnings of this type. However, do you have any
thoughts about trying to modify the declare statements? It seems like this
declare should be for consumers of the function rather than inside of the
function. I wasn't sure if we could have the compiler check if it was being
called from within the actual original definition of a function and
therefore be smart enough to not issue a warning, or if we could add some
type of declaration that this is the core so the warnings are not for us.

My hesitation are that some of these might be heavy handed and might make
the core more brittle as time goes on and no warnings are issued when
things change more. It also might be as tedious as wrapping each call with
a with-no-warnings.

I see that with-no-warnings is defined like this:
(defun with-no-warnings (&rest body)
  "Like `progn', but prevents compiler warnings in the body."
  (declare (indent 0))
  ;; The implementation for the interpreter is basically trivial.
  (car (last body)))

I see nothing that obviously suppresses warnings, so it seems like the
interpreter checks to see if it is inside of this environment. Could we
change the interpreter to check if its running inside of a function with a
(declare (advertised-calling-convention (signature) "version")) and issue
no warnings about advertised-calling-convention violations?

Lots of rambling and words there but mundane problems can have interesting
solutions. Thanks so much and I hope you're having a great weekend.
dan

On Sat, Nov 14, 2015 at 6:05 AM, Artur Malabarba <bruce.connor.am@gmail.com>
wrote:

> Hi Daniel
>
> 2015-11-14 5:54 GMT+00:00 daniel sutton <danielsutton01@gmail.com>:
> > 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.
>
> Great!
>
> > 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.
>
> This is because `display-completion-list' includes the line (declare
> (advertised-calling-convention (completions) "24.4")).
>
> This means that, since 24.4, the old way of calling this function was
> made obsolete, and it should only be called with one argument now.
> Instead of just removing the second argument entirely (which would
> break backwards compatibility of code out there) we "pretend" that the
> function only takes one argument, so that new code won't use the
> second argument.
>
> Normally, the way to solve this warning is to update the way that the
> function is being called to conform with the new arglist.
> However, looking at the source here, it seems that if the clause `(not
> (bufferp standard-output))' is true, then the function just wants to
> call itself again (with the same arguments) in a temp-buffer, so you
> shouldn't change that.
>
> So it is safe to just mute this warning (and add a comment to the
> function saying why). To mute the warning, wrap the form in a
> `(with-no-warnings ...)'
>

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

Re: Solving some specific warnings (was: Fixing compilation and byte-compilation warnings before 25.1)

[Artur Malabarba]

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

On 14 Nov 2015 6:49 pm, "daniel sutton" <danielsutton01@gmail.com> wrote:
>
> Thank you so much for the response Artur. I actually saw someone post a
link to a blog post of yours about the using # before lambdas today as well
(just saying that your contributions are passed around the community and
are much appreciated).

That's nice to know. ☺

> I can add (with-no-warnings (recursive-call-to-deprecated-args-list
...)...) calls to the warnings of this type. However, do you have any
thoughts about trying to modify the declare statements? I wasn't sure if we
could have the compiler check if it was being called from within the actual
original definition of a function and therefore be smart enough to not
issue a warning,

Should be possible, and I almost agree with it. The problem is that there
are (unfortunately) some pretty long functions in core, so I can perfectly
imagine someone editing a signature and forgetting to change a function
call inside the same function.

Besides, recursive calls like this one are sparse enough that it should be
OK to just suppress warnings on them when the sig changes.

> if we could add some type of declaration that this is the core so the
warnings are not for us.
> My hesitation are that some of these might be heavy handed and might make
the core more brittle as time goes on and no warnings are issued when
things change more.

Precisely. The core is large, it is managed by lots of people and it is
edited by even more people. So this warning does very much apply to core on
many cases. There are no warnings that we can safely disable as a whole in
core.

> I see that with-no-warnings is defined like this:
>...
> I see nothing that obviously suppresses warnings, so it seems like the
interpreter checks to see if it is inside of this environment.

No. The compiler checks. The interpreter doesn't throw warnings. That's why
the definition is so trivial. This function is treated specially by the
compiler.

> Could we change the [compiler] to check if its running inside of a
function with a (declare (advertised-calling-convention (signature)
"version")) and issue no warnings about advertised-calling-convention
violations?

Probably. I'm not against this, but I wouldn't go through the effort
either. I'm also not sure if it's a good idea because we have some really
large functions running around.

> Lots of rambling and words there but mundane problems can have
interesting solutions. Thanks so much and I hope you're having a great
weekend.

No problem. The weekend was nice enough that I forgot to reply to you
yesterday. 😀

Best,

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