[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defcla
From: |
Stefan Monnier |
Subject: |
bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload |
Date: |
Thu, 27 Aug 2020 17:38:45 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
> 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
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Lars Ingebrigtsen, 2020/08/24
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Stefan Monnier, 2020/08/24
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Lars Ingebrigtsen, 2020/08/25
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Lars Ingebrigtsen, 2020/08/25
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Michael Heerdegen, 2020/08/26
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Lars Ingebrigtsen, 2020/08/27
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Michael Heerdegen, 2020/08/27
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Michael Heerdegen, 2020/08/27
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Michael Heerdegen, 2020/08/27
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload,
Stefan Monnier <=
- bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload, Lars Ingebrigtsen, 2020/08/28