GNU bug report logs - #39169
28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload

Previous Next

Package: emacs;

Reported by: Michael Heerdegen <michael_heerdegen <at> web.de>

Date: Sat, 18 Jan 2020 09:58:02 UTC

Severity: normal

Found in version 28.0.50

Done: Michael Heerdegen <michael_heerdegen <at> web.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 39169 in the body.
You can then email your comments to 39169 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Sat, 18 Jan 2020 09:58:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Michael Heerdegen <michael_heerdegen <at> web.de>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sat, 18 Jan 2020 09:58:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Sat, 18 Jan 2020 10:57:24 +0100
Hello,

I'm currently developing a Gnu Elpa package that makes use of
`defclass'.  These lines in `eieio-defclass-autoload':

#+begin_src emacs-lisp
      ;; turn this into a usable self-pointing symbol
      (when eieio-backward-compatibility
        (set cname cname)
        (make-obsolete-variable cname (format "use \\='%s instead" cname)
                                "25.1"))
#+end_src

(and eieio-backward-compatibility defaults to t) lead to the following
situation: when I have any class, for example, named `buffer-note', and
I have the generated autoloads loaded, whenever I use a variable with
the name `buffer-note' (which is a quite natural name for objects of
that class), I get tons of warnings saying:

| buffer-note.el:136:11:Warning: `buffer-note' is an obsolete
|     variable (as of 25.1); use 'buffer-note

The purpose of these warnings is a backward compatibility one, but it
shoots way over target: these warnings prevent me from using the class
name as a variable name - I keep renaming variables to prevent these
annoying warnings all the time.  They are obviously very confusing if
you don't know the background internals, unless you really have hit the
addressed case (and of course following the instruction is not
expedient).  And it's hard to get rid of them: because the
`make-obsolete-variable' are in the autoloads, not even a file-local
eieio-backward-compatibility setting helps.

Please reconsider how these warnings are implemented.  If no better
solution can be found, the warning message should at least be made more
meaningful and tell which case it addresses, and there should be a
(discoverable!) way to turn off these warnings (file locally).


TIA,

Michael.


In GNU Emacs 28.0.50 (build 23, x86_64-pc-linux-gnu, GTK+ Version 3.24.13, cairo version 1.16.0)
 of 2020-01-17 built on drachen
Repository revision: 4d3ac4cddbe1960f5227d14bd0d357a7b19c1296
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12006000
System Description: Debian GNU/Linux bullseye/sid




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Mon, 24 Aug 2020 20:56:01 GMT) Full text and rfc822 format available.

Message #8 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Mon, 24 Aug 2020 22:55:01 +0200
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> I'm currently developing a Gnu Elpa package that makes use of
> `defclass'.  These lines in `eieio-defclass-autoload':
>
> #+begin_src emacs-lisp
>       ;; turn this into a usable self-pointing symbol
>       (when eieio-backward-compatibility
>         (set cname cname)
>         (make-obsolete-variable cname (format "use \\='%s instead" cname)
>                                 "25.1"))
> #+end_src
>
> (and eieio-backward-compatibility defaults to t) lead to the following
> situation: when I have any class, for example, named `buffer-note', and
> I have the generated autoloads loaded, whenever I use a variable with
> the name `buffer-note' (which is a quite natural name for objects of
> that class), I get tons of warnings saying:
>
> | buffer-note.el:136:11:Warning: `buffer-note' is an obsolete
> |     variable (as of 25.1); use 'buffer-note
>
> The purpose of these warnings is a backward compatibility one, but it
> shoots way over target: these warnings prevent me from using the class
> name as a variable name - I keep renaming variables to prevent these
> annoying warnings all the time.

Yes, that does sound annoying.  eieio got a lot of warnings during the
conversion to more normal cl-* functions the other year, so it made
sense to add warnings like this during the rewrite.  But that's done
now, so perhaps it makes sense to remove these over-enthusiastic
warnings now?  Stefan?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Tue, 25 Aug 2020 02:13:02 GMT) Full text and rfc822 format available.

Message #11 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 39169 <at> debbugs.gnu.org
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Mon, 24 Aug 2020 22:12:13 -0400
>> I'm currently developing a Gnu Elpa package that makes use of
>> `defclass'.  These lines in `eieio-defclass-autoload':
>>
>> #+begin_src emacs-lisp
>>       ;; turn this into a usable self-pointing symbol
>>       (when eieio-backward-compatibility
>>         (set cname cname)
>>         (make-obsolete-variable cname (format "use \\='%s instead" cname)
>>                                 "25.1"))
>> #+end_src
>>
>> (and eieio-backward-compatibility defaults to t) lead to the following
>> situation: when I have any class, for example, named `buffer-note', and
>> I have the generated autoloads loaded, whenever I use a variable with
>> the name `buffer-note' (which is a quite natural name for objects of
>> that class), I get tons of warnings saying:
>>
>> | buffer-note.el:136:11:Warning: `buffer-note' is an obsolete
>> |     variable (as of 25.1); use 'buffer-note
>>
>> The purpose of these warnings is a backward compatibility one, but it
>> shoots way over target: these warnings prevent me from using the class
>> name as a variable name - I keep renaming variables to prevent these
>> annoying warnings all the time.

Indeed that happens when you re-use the identifier `buffer-note` which has
a global binding and our obsolescence warning doesn't know how to
distinguish local bindings from global bindings.

But you could set `eieio-backward-compatibility` to nil.

> Yes, that does sound annoying.  eieio got a lot of warnings during the
> conversion to more normal cl-* functions the other year, so it made
> sense to add warnings like this during the rewrite.  But that's done
> now, so perhaps it makes sense to remove these over-enthusiastic
> warnings now?  Stefan?

You mean we should change the default of `eieio-backward-compatibility`
to nil?

Maybe you're right.

We're currently removing things that were made obsolete in Emacs-23,
whereas those were made obsolete in Emacs-25, but setting that var to
nil is not as drastic as removing those vars and functions, so it's
probably OK to do it already.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Tue, 25 Aug 2020 20:06:02 GMT) Full text and rfc822 format available.

Message #14 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 39169 <at> debbugs.gnu.org
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Tue, 25 Aug 2020 22:05:26 +0200
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> You mean we should change the default of `eieio-backward-compatibility`
> to nil?

No, I meant removing the warning, since we no longer have any in-tree
usage of this stuff (I think).

> Maybe you're right.
>
> We're currently removing things that were made obsolete in Emacs-23,
> whereas those were made obsolete in Emacs-25, but setting that var to
> nil is not as drastic as removing those vars and functions, so it's
> probably OK to do it already.

Hm...  it does sound a bit drastic...  But now I'm wondering why Michael
isn't just setting/binding eieio-backward-compatibility to nil?  Then
the warnings would go away, and he probably aren't using the compat
symbols, anyway?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Tue, 25 Aug 2020 20:08:01 GMT) Full text and rfc822 format available.

Message #17 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, 39169 <at> debbugs.gnu.org
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Tue, 25 Aug 2020 22:07:18 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Hm...  it does sound a bit drastic...  But now I'm wondering why Michael
> isn't just setting/binding eieio-backward-compatibility to nil?  Then
> the warnings would go away, and he probably aren't using the compat
> symbols, anyway?                              ^
                                                |
Insert grammer here -----------------------------

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Wed, 26 Aug 2020 15:32:02 GMT) Full text and rfc822 format available.

Message #20 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Wed, 26 Aug 2020 17:30:56 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> But now I'm wondering why Michael isn't just setting/binding
> eieio-backward-compatibility to nil?  Then the warnings would go away,
> and he probably aren't using the compat symbols, anyway?

AFAIR, the situation is: I can do that file locally in my library, yes.

But any user of the library will still see those warnings.  It's likely
that this user uses the name "buffer-note" for a buffer-note, and the
warning text is meaningless for anybody who doesn't know what's going
on.

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Thu, 27 Aug 2020 13:35:02 GMT) Full text and rfc822 format available.

Message #23 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Thu, 27 Aug 2020 15:34:42 +0200
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> But now I'm wondering why Michael isn't just setting/binding
>> eieio-backward-compatibility to nil?  Then the warnings would go away,
>> and he probably aren't using the compat symbols, anyway?
>
> AFAIR, the situation is: I can do that file locally in my library, yes.
>
> But any user of the library will still see those warnings.  It's likely
> that this user uses the name "buffer-note" for a buffer-note, and the
> warning text is meaningless for anybody who doesn't know what's going
> on.

OK, but then I think the right thing here is just to punt further, to
that user of the library.  :-)  If they don't want warnings about these
obsolete compat variables, then they are the ones that should set
eieio-backward-compatibility to nil?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Thu, 27 Aug 2020 15:08:01 GMT) Full text and rfc822 format available.

Message #26 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Thu, 27 Aug 2020 17:07:33 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> OK, but then I think the right thing here is just to punt further, to
> that user of the library.  :-)  If they don't want warnings about these
> obsolete compat variables, then they are the ones that should set
> eieio-backward-compatibility to nil?

Fine for me - in principle.  Note the word "confusing" in my bug report
title.  How long do you think would a average user (not every programmer
knows the internals of eieio) derive from the warning

  Warning: `buffer-note' is an obsolete variable (as of 25.1); use
  'buffer-note

that emacs wants him to add a file local binding
eieio-backward-compatibility -> nil?


Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Thu, 27 Aug 2020 15:24:02 GMT) Full text and rfc822 format available.

Message #29 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Thu, 27 Aug 2020 17:23:11 +0200
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> that emacs wants him to add a file local binding
> eieio-backward-compatibility -> nil?

Note that I didn't check whether this actually (and always) works: The
problematic obsolete variable declaration is performed in the function
`eieio-defclass-autoload'.  If the value of
`eieio-backward-compatibility' is checked when loading autoload
definitions, will a file local binding in the source library be
considered at all?

Oh, and let me add another important aspect: why does using an obsolete
name as the name of a _lexical_ variable trigger the "variable is
obsolete" warning at all?  If that would not be the case (and I don't
think it is useful) then in source files using lexical binding mode we
would not see the problem.

Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Thu, 27 Aug 2020 21:23:01 GMT) Full text and rfc822 format available.

Message #32 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Thu, 27 Aug 2020 23:22:03 +0200
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Oh, and let me add another important aspect: why does using an obsolete
> name as the name of a _lexical_ variable trigger the "variable is
> obsolete" warning at all?  If that would not be the case (and I don't
> think it is useful) then in source files using lexical binding mode we
> would not see the problem.

Would doing something like this make sense?

[bug.patch (text/x-diff, inline)]
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 966990bac9..418070fabe 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3357,6 +3357,7 @@ byte-compile-check-variable
            (and od
                 (not (memq var byte-compile-not-obsolete-vars))
                 (not (memq var byte-compile-global-not-obsolete-vars))
+                (not (memq var byte-compile-lexical-variables))
                 (or (pcase (nth 1 od)
                       ('set (not (eq access-type 'reference)))
                       ('get (eq access-type 'reference))
[Message part 3 (text/plain, inline)]

BTW, while I looked at this, I found this spurious lookup in
`byte-compile-lexical-variables':

#+begin_src emacs-lisp
(defun byte-compile-form (form &optional for-effect)
  (let ((byte-compile--for-effect for-effect))
    (cond
     ((not (consp form))..10..)
     ((symbolp (car form))
      (let* ((fn (car form))..4..)
        (when (memq fn '(set symbol-value run-hooks..4..)
          (pcase (cdr form)
            (`(',var . ,_)
             (when (assq var byte-compile-lexical-variables) ;; <--- here
               (byte-compile-report-error
                (format-message "%s cannot use lexical var `%s'" fn var)))
             ...)))))))))
#+end_src

shouldn't the `assq' be `memq'?


Michael.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Thu, 27 Aug 2020 21:39:01 GMT) Full text and rfc822 format available.

Message #35 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 39169 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Thu, 27 Aug 2020 17:38:45 -0400
> Note that I didn't check whether this actually (and always) works: The
> problematic obsolete variable declaration is performed in the function
> `eieio-defclass-autoload'.  If the value of
> `eieio-backward-compatibility' is checked when loading autoload
> definitions, will a file local binding in the source library be
> considered at all?

Good question.  I did not pay attention to non-global values of
`eieio-backward-compatibility` when writing that code, so I don't know
whether it would work well (or at all).

>> Oh, and let me add another important aspect: why does using an obsolete
>> name as the name of a _lexical_ variable trigger the "variable is
>> obsolete" warning at all?  If that would not be the case (and I don't
>> think it is useful) then in source files using lexical binding mode we
>> would not see the problem.
>
> Would doing something like this make sense?
>
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index 966990bac9..418070fabe 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -3357,6 +3357,7 @@ byte-compile-check-variable
>             (and od
>                  (not (memq var byte-compile-not-obsolete-vars))
>                  (not (memq var byte-compile-global-not-obsolete-vars))
> +                (not (memq var byte-compile-lexical-variables))
>                  (or (pcase (nth 1 od)
>                        ('set (not (eq access-type 'reference)))
>                        ('get (eq access-type 'reference))

Yes!

> BTW, while I looked at this, I found this spurious lookup in
> `byte-compile-lexical-variables':
>
> #+begin_src emacs-lisp
> (defun byte-compile-form (form &optional for-effect)
>   (let ((byte-compile--for-effect for-effect))
>     (cond
>      ((not (consp form))..10..)
>      ((symbolp (car form))
>       (let* ((fn (car form))..4..)
>         (when (memq fn '(set symbol-value run-hooks..4..)
>           (pcase (cdr form)
>             (`(',var . ,_)
>              (when (assq var byte-compile-lexical-variables) ;; <--- here
>                (byte-compile-report-error
>                 (format-message "%s cannot use lexical var `%s'" fn var)))
>              ...)))))))))
> #+end_src
>
> shouldn't the `assq' be `memq'?

I think you're right.

Both changes should ideally come with corresponding regression tests.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Fri, 28 Aug 2020 14:07:01 GMT) Full text and rfc822 format available.

Message #38 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 39169 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Fri, 28 Aug 2020 16:06:13 +0200
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> Fine for me - in principle.  Note the word "confusing" in my bug report
> title.  How long do you think would a average user (not every programmer
> knows the internals of eieio) derive from the warning
>
>   Warning: `buffer-note' is an obsolete variable (as of 25.1); use
>   'buffer-note
>
> that emacs wants him to add a file local binding
> eieio-backward-compatibility -> nil?

Yeah, that's true.  Could we add that to the obsolete warning (instead
of the confusing "'buffer-note")?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#39169; Package emacs. (Tue, 29 Dec 2020 11:01:01 GMT) Full text and rfc822 format available.

Message #41 received at 39169 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 39169 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Tue, 29 Dec 2020 11:59:59 +0100
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> > diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> > index 966990bac9..418070fabe 100644
> > --- a/lisp/emacs-lisp/bytecomp.el
> > +++ b/lisp/emacs-lisp/bytecomp.el
> > @@ -3357,6 +3357,7 @@ byte-compile-check-variable
> >             (and od
> >                  (not (memq var byte-compile-not-obsolete-vars))
> >                  (not (memq var byte-compile-global-not-obsolete-vars))
> > +                (not (memq var byte-compile-lexical-variables))
> >                  (or (pcase (nth 1 od)
> >                        ('set (not (eq access-type 'reference)))
> >                        ('get (eq access-type 'reference))
>
> Yes!

Coming back to this now... Ok, then let's do that!

> > BTW, while I looked at this, I found this spurious lookup in
> > `byte-compile-lexical-variables':
> >
> > #+begin_src emacs-lisp
> > (defun byte-compile-form (form &optional for-effect)
> >   (let ((byte-compile--for-effect for-effect))
> >     (cond
> >      ((not (consp form))..10..)
> >      ((symbolp (car form))
> >       (let* ((fn (car form))..4..)
> >         (when (memq fn '(set symbol-value run-hooks..4..)
> >           (pcase (cdr form)
> >             (`(',var . ,_)
> >              (when (assq var byte-compile-lexical-variables) ;; <--- here
> >                (byte-compile-report-error
> >                 (format-message "%s cannot use lexical var `%s'" fn var)))
> >              ...)))))))))
> > #+end_src
> >
> > shouldn't the `assq' be `memq'?
>
> I think you're right.

Seems this has been treated meanwhile: Bug#44980 and commit ace6eba036
"Fix byte-compiler warning for failed uses of lexical vars", Stefan
Kangas.

> Both changes should ideally come with corresponding regression tests.

I tried to add an appropriate test for the remaining case, please check
the following patch (my first addition to the test suite) - is it what
you had in mind? - where I also try to give a less confusing (but
longer) compiler warning.  Does it look alright?

[0001-Fix-obsolete-variable-warnings-about-class-names.patch (text/x-diff, inline)]
From 63f3420d91273fecb51095c20c81d9fd0508ee4c Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen <at> web.de>
Date: Tue, 22 Dec 2020 05:44:47 +0100
Subject: [PATCH] Fix obsolete variable warnings about class names

* lisp/emacs-lisp/eieio-core.el (eieio-defclass-autoload): Try to make
the wording of the warning about the obsoleted variable less confusing.
* lisp/emacs-lisp/bytecomp.el (byte-compile-check-variable): Don't
warn for lexical variables (Bug#39169).  Fix spurious `or'.
* test/lisp/emacs-lisp/bytecomp-tests.el
(bytecomp/warn-obsolete-variable-bound\.el): New test.
* test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el:
New file.
---
 lisp/emacs-lisp/bytecomp.el                              | 9 +++++----
 lisp/emacs-lisp/eieio-core.el                            | 3 ++-
 .../bytecomp-resources/warn-obsolete-variable-bound.el   | 7 +++++++
 test/lisp/emacs-lisp/bytecomp-tests.el                   | 3 +++
 4 files changed, 17 insertions(+), 5 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el

diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index f14ad93d2e..aa531c2558 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3441,10 +3441,11 @@ byte-compile-check-variable
            (and od
                 (not (memq var byte-compile-not-obsolete-vars))
                 (not (memq var byte-compile-global-not-obsolete-vars))
-                (or (pcase (nth 1 od)
-                      ('set (not (eq access-type 'reference)))
-                      ('get (eq access-type 'reference))
-                      (_ t)))))
+                (not (memq var byte-compile-lexical-variables))
+                (pcase (nth 1 od)
+                  ('set (not (eq access-type 'reference)))
+                  ('get (eq access-type 'reference))
+                  (_ t))))
 	 (byte-compile-warn-obsolete var))))

 (defsubst byte-compile-dynamic-variable-op (base-op var)
diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el
index 3bc65d0d4c..010ff54d4d 100644
--- a/lisp/emacs-lisp/eieio-core.el
+++ b/lisp/emacs-lisp/eieio-core.el
@@ -215,7 +215,8 @@ eieio-defclass-autoload
       ;; turn this into a usable self-pointing symbol
       (when eieio-backward-compatibility
         (set cname cname)
-        (make-obsolete-variable cname (format "use \\='%s instead" cname)
+        (make-obsolete-variable cname (format "\
+use \\='%s or turn off `eieio-backward-compatibility' instead" cname)
                                 "25.1"))

       (setf (cl--find-class cname) newc)
diff --git a/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el
new file mode 100644
index 0000000000..e65a541e6e
--- /dev/null
+++ b/test/lisp/emacs-lisp/bytecomp-resources/warn-obsolete-variable-bound.el
@@ -0,0 +1,7 @@
+;;; -*- lexical-binding: t -*-
+
+(make-obsolete-variable 'bytecomp--tests-obsolete-var-2 nil "99.99")
+
+(defun foo ()
+  (let ((bytecomp--tests-obsolete-var-2 2))
+    bytecomp--tests-obsolete-var-2))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 47aab563f6..c4a27b9ef8 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -625,6 +625,9 @@ "warn-obsolete-variable-same-file.el"
 (bytecomp--define-warning-file-test "warn-obsolete-variable.el"
                             "bytecomp--tests-obs.*obsolete.*99.99")

+(bytecomp--define-warning-file-test "warn-obsolete-variable-bound.el"
+                            "bytecomp--tests-obs.*obsolete.*99.99" t)
+
 (bytecomp--define-warning-file-test "warn-redefine-defun-as-macro.el"
                             "as both function and macro")

--
2.29.2

[Message part 3 (text/plain, inline)]

TIA,

Michael.

Reply sent to Michael Heerdegen <michael_heerdegen <at> web.de>:
You have taken responsibility. (Wed, 06 Jan 2021 16:57:02 GMT) Full text and rfc822 format available.

Notification sent to Michael Heerdegen <michael_heerdegen <at> web.de>:
bug acknowledged by developer. (Wed, 06 Jan 2021 16:57:02 GMT) Full text and rfc822 format available.

Message #46 received at 39169-done <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 39169-done <at> debbugs.gnu.org
Subject: Re: bug#39169: 28.0.50; Confusing obsolete variable warnings in
 eieio-defclass-autoload
Date: Wed, 06 Jan 2021 17:56:27 +0100
Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> I tried to add an appropriate test for the remaining case, please check
> the following patch (my first addition to the test suite) - is it what
> you had in mind? - where I also try to give a less confusing (but
> longer) compiler warning.  Does it look alright?

Installed.

Then we should be done here, and I'm closing this report.

Thanks to all,

Michael.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 04 Feb 2021 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 81 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.