bug#39169: 28.0.50; Confusing obsolete variable warnings in eieio-defclass-autoload

[Stefan Monnier <monnier@iro.umontreal.ca>]

> 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