bug#49291: [akater] Re: [akater] [PATCH] lisp/emacs-lisp/eieio.el (initialize-instance): Fix initform

[akater <nuclearspace@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 300 bytes --]


-------------------- Start of forwarded message --------------------
From: akater <nuclearspace@gmail.com>
To: Stefan Monnier <monnier@iro.umontreal.ca>, 49291@gnu.org
Subject: Re: [akater] [PATCH] lisp/emacs-lisp/eieio.el
 (initialize-instance): Fix initform
Date: Wed, 30 Jun 2021 16:44:12 +0000


[-- Attachment #2.1.1: Type: text/plain, Size: 1600 bytes --]

There are iusses, some stylistic, some related to comments in the code.

- There is a comment here:

> First, see if any of our defaults are `lambda', and
> re-evaluate them and apply the value to our slots.             

I don't observe anything like this happening.  Looks like it refers to
eieio-default-eval-maybe (likely referring to any compound form with
fbound car as to `lambda') which used to be in eieio-core in 27 but now
is gone.  Maybe we should drop this comment?

- There is a comment:

> For each slot, see if we need to evaluate it

Slots are self-evaluating objects; I think it was meant to be “to
evaluate its initform”.

- There is FIXME

> FIXME: We should be able to just do (aset this (+ i <cst>) dflt)!                                                          

Local variable dflt had been removed after Emacs 27 release.  The
comment should likely have been gone too, or at least updated.  It
suggests to assign the value of initform with a low-level `aset' applied
to eieio--class struct but (1) I don't understand the + i shift (2)
eieio--class is not declared to be of :type vector, neither does it
inherit from a struct declared to be of :type vector.  I suggest to
replace the comment with
“TODO: maybe specify eieio--class as vector and use aset here”

- I employ when-let which requires subr-x at compile time.  I can't
check the build cleanly right now, only with some dirty reverts related
to libseccomp issues but aside from that, this subr-x dependency doesn't
break byte-compilation of eieio.el.  I hope it's fine?


[-- Attachment #2.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.2: Fix eval initform --]
[-- Type: text/x-diff, Size: 3253 bytes --]

From 588e7ff1b34ccfbc9e3cb97d06ae3315487d42c9 Mon Sep 17 00:00:00 2001
From: akater <nuclearspace@gmail.com>
Date: Wed, 30 Jun 2021 11:43:23 +0000
Subject: [PATCH] lisp/emacs-lisp/eieio.el (initialize-instance): Fix eval
 initform:

Do not evaluate initform of a slot when initarg for the slot is provided.

According to the following secitons of CLHS:
- Object Creation and Initialization
- Initialization Arguments
- Defaulting of Initialization Arguments
- Rules for Initialization Arguments
---
 lisp/emacs-lisp/eieio.el | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 1c8c372aae..ec05a07307 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -53,6 +53,7 @@ defun eieio-version ()
   (message eieio-version))
 
 (require 'eieio-core)
+(eval-when-compile (require 'subr-x))
 
 \f
 ;;; Defining a new class
@@ -740,7 +741,7 @@ defclass eieio-default-superclass nil
   "Construct the new object THIS based on SLOTS.")
 
 (cl-defmethod initialize-instance ((this eieio-default-superclass)
-				&optional slots)
+				   &optional slots)
   "Construct the new object THIS based on SLOTS.
 SLOTS is a tagged list where odd numbered elements are tags, and
 even numbered elements are the values to store in the tagged slot.
@@ -749,20 +750,25 @@ defclass eieio-default-superclass nil
 to have this constructor called automatically.  If these steps are
 not taken, then new objects of your class will not have their values
 dynamically set from SLOTS."
-  ;; First, see if any of our defaults are `lambda', and
-  ;; re-evaluate them and apply the value to our slots.
   (let* ((this-class (eieio--object-class this))
+         (initargs slots)
          (slots (eieio--class-slots this-class)))
     (dotimes (i (length slots))
-      ;; For each slot, see if we need to evaluate it.
+      ;; For each slot, see if we need to evaluate its initform.
       (let* ((slot (aref slots i))
+             (slot-name (eieio-slot-descriptor-name slot))
              (initform (cl--slot-descriptor-initform slot)))
-        ;; Those slots whose initform is constant already have the right
-        ;; value set in the default-object.
-        (unless (macroexp-const-p initform)
-          ;; FIXME: We should be able to just do (aset this (+ i <cst>) dflt)!
-          (eieio-oset this (cl--slot-descriptor-name slot)
-                      (eval initform t))))))
+        (unless (or (eq eieio--unbound initform)
+                    (when-let ((initarg
+                                (car (rassq slot-name
+                                            (eieio--class-initarg-tuples
+                                             this-class)))))
+                      (plist-get initargs initarg))
+                    ;; Those slots whose initform is constant already have
+                    ;; the right value set in the default-object.
+                    (macroexp-const-p initform))
+          ;; TODO: Maybe specify eieio--class as vector explicitly and use aset
+          (eieio-oset this slot-name (eval initform t))))))
   ;; Shared initialize will parse our slots for us.
   (shared-initialize this slots))
 
-- 
2.31.1


[-- Attachment #3: Type: text/plain, Size: 67 bytes --]

-------------------- End of forwarded message --------------------

[-- Attachment #4.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #4.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]