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

[Eli Zaretskii]

Ping!  Stefan, could you please look into this?

> Cc: 66938@debbugs.gnu.org
> Date: Sat, 11 Nov 2023 12:21:23 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> 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
> > 
> 
> 
> 
>