bug#66938: 30.0.50 [PATCH]: Make EIEIO :accessor behave like :reader when reading a slot's value

[Eli Zaretskii]

Stefan, any comments?

> From: Brandon Irizarry <brandon.irizarry@gmail.com>
> Date: Sat, 4 Nov 2023 18:03:06 -0400
> 
> I've included a suggested patch for 'eieio.el', which slightly
> modifies the behavior of EIEIO's 'defclass'. When playing around with
> 'eieio-instance-inheritor' as a base class, I noticed that calls to
> the accessor (when used as a getter) weren't delegating to the parent
> instance (as defined when calling 'clone'). That is, an object clone that 
> doesn't set
> its fields directly will report 'nil' when the accessor method is used as a 
> getter for some slot.
> 
> However, neither 'oref' nor ':reader' methods share this problem.
> 
> The included patch duplicates the code for the reader method defined for
> the ':reader' case. In particular, it removes the 'slot-unboundp' check, 
> which appeared 
> to be the real culprit here. I've also deleted the FIXME comment which asked, 
> "Why is this 
> different from the :reader case?" since it no longer differs from it.
> 
> There is some more context provided (along with an example) in the
> commit message included in the patch, in case it helps.
> 
> I know this isn't earth-shattering, but I couldn't help but notice the issue;
> and, at least to me, it seemed like incorrect behavior. (Otherwise, in order 
> to
> obtain the delegation behavior for a given slot, I'd be forced to define
>  ':reader' and ':writer' separately, which feels rather verbose, given we 
> have ':accessor'
> for this exact purpose.)
> 
> - Brandon
> 
> 
> From 2d40d63d8738dc44b02ca61843fa61956958e84e Mon Sep 17 00:00:00 2001
> From: Brandon <brandon.irizarry@gmail.com>
> Date: Sat, 4 Nov 2023 17:11:32 -0400
> Subject: [PATCH] Make EIEIO ':accessor' behave like ':reader' when reading
> 
> * lisp/emacs-lisp/eieio.el (defclass): Remove 'slot-boundp' check for
> :accessor's getter
> 
> Clones of instances of subclasses of 'eieio-instance-inheritor' don't
> delegate to their ':parent-instance' field when reading object fields
> using ':accessor'.
> 
> Say I have this code:
> 
> (defclass foo (eieio-instance-inheritor)
>   ((x :initarg :x
>       :accessor ref-x
>       :reader get-x)))
> 
> (setq obj1 (foo :x 4))   ; #s(foo eieio--unbound 4)
> (setq obj2 (clone obj1)) ; #s(foo #s(foo eieio--unbound 4) eieio--unbound)
> 
> (ref-x obj1) ; 4, which is correct.
> (ref-x obj2) ; nil. This is what we want to fix.
> (get-x obj2) ; Gives us 4: access via the reader performs delegation.
> 
> My impression is that ':accessor' should behave as if ':reader' and
> ':writer' had been provided separately.
> 
> With this patch, '(ref-x obj2)' now uses the exact same method as
> ':reader', and so would give us 4, as desired.
> ---
>  lisp/emacs-lisp/eieio.el | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
> index 39a5fd5b19c..8224606ec57 100644
> --- a/lisp/emacs-lisp/eieio.el
> +++ b/lisp/emacs-lisp/eieio.el
> @@ -213,9 +213,8 @@ defclass
>                     ,(internal--format-docstring-line
>                       "Retrieve the slot `%S' from an object of class `%S'."
>                       sname name)
> -                   ;; FIXME: Why is this different from the :reader case?
> -                   (if (slot-boundp this ',sname) (eieio-oref this ',sname)))
> -                accessors)
> +                   (slot-value this ',sname))
> +                  accessors)
>            (when (and eieio-backward-compatibility (eq alloc :class))
>              ;; FIXME: How could I declare this *method* as obsolete.
>              (push `(cl-defmethod ,acces ((this (subclass ,name)))
> -- 
> 2.39.2
>