bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

1. emacs -Q
2. (require 'allout) or otherwise load allout.el
3. Byte-compile allout.el with M-x byte-compile-file
4. See warnings:

--8<---------------cut here---------------start------------->8---
In allout-old-expose-topic:
allout.el:5092:29: Warning: ‘allout-old-expose-topic’ is an obsolete function
    (as of 28.1); use ‘allout-expose-topic’ instead.
allout.el:5097:44: Warning: ‘allout-old-expose-topic’ is an obsolete function
    (as of 28.1); use ‘allout-expose-topic’ instead.
allout.el:5106:8: Warning: ‘allout-old-expose-topic’ is an obsolete function
    (as of 28.1); use ‘allout-expose-topic’ instead.
--8<---------------cut here---------------end--------------->8---

These warnings are unhelpful since these are recursive calls within the
definition of the obsolete function itself.  They need not be replaced
with another function as the warnings suggest.  Ideally, recursive calls
to obsolete functions should not produce such warnings.


Best,

Eshel

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Eli Zaretskii]

> Date: Sat, 19 Oct 2024 15:21:22 +0200
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> 1. emacs -Q
> 2. (require 'allout) or otherwise load allout.el
> 3. Byte-compile allout.el with M-x byte-compile-file
> 4. See warnings:
> 
> --8<---------------cut here---------------start------------->8---
> In allout-old-expose-topic:
> allout.el:5092:29: Warning: ‘allout-old-expose-topic’ is an obsolete function
>     (as of 28.1); use ‘allout-expose-topic’ instead.
> allout.el:5097:44: Warning: ‘allout-old-expose-topic’ is an obsolete function
>     (as of 28.1); use ‘allout-expose-topic’ instead.
> allout.el:5106:8: Warning: ‘allout-old-expose-topic’ is an obsolete function
>     (as of 28.1); use ‘allout-expose-topic’ instead.
> --8<---------------cut here---------------end--------------->8---
> 
> These warnings are unhelpful since these are recursive calls within the
> definition of the obsolete function itself.  They need not be replaced
> with another function as the warnings suggest.  Ideally, recursive calls
> to obsolete functions should not produce such warnings.

From where I stand, this could be closed as wontfix, unless someone
sees an easy fix.  I don't see any harm from emitting these warnings
in this scenario.  The warnings are correct.

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 19 Oct 2024 15:21:22 +0200
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> 
>> 1. emacs -Q
>> 2. (require 'allout) or otherwise load allout.el
>> 3. Byte-compile allout.el with M-x byte-compile-file
>> 4. See warnings:
>> 
>> --8<---------------cut here---------------start------------->8---
>> In allout-old-expose-topic:
>> allout.el:5092:29: Warning: ‘allout-old-expose-topic’ is an obsolete function
>>     (as of 28.1); use ‘allout-expose-topic’ instead.
>> allout.el:5097:44: Warning: ‘allout-old-expose-topic’ is an obsolete function
>>     (as of 28.1); use ‘allout-expose-topic’ instead.
>> allout.el:5106:8: Warning: ‘allout-old-expose-topic’ is an obsolete function
>>     (as of 28.1); use ‘allout-expose-topic’ instead.
>> --8<---------------cut here---------------end--------------->8---
>> 
>> These warnings are unhelpful since these are recursive calls within the
>> definition of the obsolete function itself.  They need not be replaced
>> with another function as the warnings suggest.  Ideally, recursive calls
>> to obsolete functions should not produce such warnings.
>
> From where I stand, this could be closed as wontfix, unless someone
> sees an easy fix.  I don't see any harm from emitting these warnings
> in this scenario.  The warnings are correct.

All right, FWIW I find them more distracting then helpful: if I'm
declaring a function as obsolete, that means it's going to stay around
for at least a short while, with its recursive calls, which I have no
interest in adapting.  So these warnings do not tell me anything useful.

As for an easy fix, maybe something like this?

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 29e7882c851..edb8160a250 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -1533,6 +1533,7 @@ byte-compile-arglist-signature-string
 
 (defun byte-compile-function-warn (f nargs def)
   (when (and (get f 'byte-obsolete-info)
+             (not (eq f byte-compile-current-form)) ; Recursive call.
              (not (memq f byte-compile-not-obsolete-funcs)))
     (byte-compile-warn-obsolete f "function"))
 

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: 73881@debbugs.gnu.org
> Date: Sat, 19 Oct 2024 20:11:08 +0200
> 
> >> In allout-old-expose-topic:
> >> allout.el:5092:29: Warning: ‘allout-old-expose-topic’ is an obsolete function
> >>     (as of 28.1); use ‘allout-expose-topic’ instead.
> >> allout.el:5097:44: Warning: ‘allout-old-expose-topic’ is an obsolete function
> >>     (as of 28.1); use ‘allout-expose-topic’ instead.
> >> allout.el:5106:8: Warning: ‘allout-old-expose-topic’ is an obsolete function
> >>     (as of 28.1); use ‘allout-expose-topic’ instead.
> >> --8<---------------cut here---------------end--------------->8---
> >> 
> >> These warnings are unhelpful since these are recursive calls within the
> >> definition of the obsolete function itself.  They need not be replaced
> >> with another function as the warnings suggest.  Ideally, recursive calls
> >> to obsolete functions should not produce such warnings.
> >
> > From where I stand, this could be closed as wontfix, unless someone
> > sees an easy fix.  I don't see any harm from emitting these warnings
> > in this scenario.  The warnings are correct.
> 
> All right, FWIW I find them more distracting then helpful: if I'm
> declaring a function as obsolete, that means it's going to stay around
> for at least a short while, with its recursive calls, which I have no
> interest in adapting.  So these warnings do not tell me anything useful.
> 
> As for an easy fix, maybe something like this?
> 
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index 29e7882c851..edb8160a250 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -1533,6 +1533,7 @@ byte-compile-arglist-signature-string
>  
>  (defun byte-compile-function-warn (f nargs def)
>    (when (and (get f 'byte-obsolete-info)
> +             (not (eq f byte-compile-current-form)) ; Recursive call.
>               (not (memq f byte-compile-not-obsolete-funcs)))
>      (byte-compile-warn-obsolete f "function"))

Thanks, let's see what others think about this.

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> As for an easy fix, maybe something like this?
>> 
>> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
>> index 29e7882c851..edb8160a250 100644
>> --- a/lisp/emacs-lisp/bytecomp.el
>> +++ b/lisp/emacs-lisp/bytecomp.el
>> @@ -1533,6 +1533,7 @@ byte-compile-arglist-signature-string
>>  
>>  (defun byte-compile-function-warn (f nargs def)
>>    (when (and (get f 'byte-obsolete-info)
>> +             (not (eq f byte-compile-current-form)) ; Recursive call.
>>               (not (memq f byte-compile-not-obsolete-funcs)))
>>      (byte-compile-warn-obsolete f "function"))
>
> Thanks, let's see what others think about this.

Hmm... I distinctly remember writing code to try and silence
obsolescence warnings in the code coming from the same file as the
`make-obsolete` call.

If I installed that code into Emacs, clearly it's not doing its job.

..Hmm.. I think I see the problem: the code I wrote was for variables
rather than for functions:

    ;; If foo.el declares `toto' as obsolete, it is likely that foo.el will
    ;; actually use `toto' in order for this obsolete variable to still work
    ;; correctly, so paradoxically, while byte-compiling foo.el, the presence
    ;; of a make-obsolete-variable call for `toto' is an indication that `toto'
    ;; should not trigger obsolete-warnings in foo.el.
    (byte-defop-compiler-1 make-obsolete-variable)
    (defun byte-compile-make-obsolete-variable (form)
      (when (eq 'quote (car-safe (nth 1 form)))
        (push (nth 1 (nth 1 form)) byte-compile-global-not-obsolete-vars))
      (byte-compile-normal-call form))

So maybe we should just do the same for `make-obsolete`?


        Stefan

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi,

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

> Hmm... I distinctly remember writing code to try and silence
> obsolescence warnings in the code coming from the same file as the
> `make-obsolete` call.

Ah good, sounds like a reasonable heuristic.

> If I installed that code into Emacs, clearly it's not doing its job.
>
> ..Hmm.. I think I see the problem: the code I wrote was for variables
> rather than for functions:
>
>     ;; If foo.el declares `toto' as obsolete, it is likely that foo.el will
>     ;; actually use `toto' in order for this obsolete variable to still work
>     ;; correctly, so paradoxically, while byte-compiling foo.el, the presence
>     ;; of a make-obsolete-variable call for `toto' is an indication that `toto'
>     ;; should not trigger obsolete-warnings in foo.el.
>     (byte-defop-compiler-1 make-obsolete-variable)
>     (defun byte-compile-make-obsolete-variable (form)
>       (when (eq 'quote (car-safe (nth 1 form)))
>         (push (nth 1 (nth 1 form)) byte-compile-global-not-obsolete-vars))
>       (byte-compile-normal-call form))
>
> So maybe we should just do the same for `make-obsolete`?

SGTM.


Thanks,

Eshel

bug#73881: 31.0.50; Unexpected warnings about recursive occurrences of obsolete functions

[Stefan Kangas]

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

> ..Hmm.. I think I see the problem: the code I wrote was for variables
> rather than for functions:
>
>     ;; If foo.el declares `toto' as obsolete, it is likely that foo.el will
>     ;; actually use `toto' in order for this obsolete variable to still work
>     ;; correctly, so paradoxically, while byte-compiling foo.el, the presence
>     ;; of a make-obsolete-variable call for `toto' is an indication that `toto'
>     ;; should not trigger obsolete-warnings in foo.el.
>     (byte-defop-compiler-1 make-obsolete-variable)
>     (defun byte-compile-make-obsolete-variable (form)
>       (when (eq 'quote (car-safe (nth 1 form)))
>         (push (nth 1 (nth 1 form)) byte-compile-global-not-obsolete-vars))
>       (byte-compile-normal-call form))
>
> So maybe we should just do the same for `make-obsolete`?

I think that makes sense.