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

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Better make it an actual bug report, so we have a bug-number.


        Stefan


[-- Attachment #2: Type: message/rfc822, Size: 9625 bytes --]

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



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

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

From 595083dac6dd0fe3446e182c710fbb2a5a648994 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 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 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 1c8c372aae..758bddd592 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -740,7 +740,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.
@@ -756,13 +756,15 @@ defclass eieio-default-superclass nil
     (dotimes (i (length slots))
       ;; For each slot, see if we need to evaluate it.
       (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)
+        (unless (or (rassq slot-name
+                           (eieio--class-initarg-tuples this-class))
+                    (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))))))
+          (eieio-oset this slot-name (eval initform t))))))
   ;; Shared initialize will parse our slots for us.
   (shared-initialize this slots))
 
-- 
2.31.1


[-- Attachment #2.1.3: Type: text/plain, Size: 5995 bytes --]


* Summary
According to CLHS,
#+begin_quote
A default initial value form for a slot is defined by using the
:initform slot option to defclass. If no initialization argument
associated with that slot is given as an argument to make-instance or is
defaulted by :default-initargs, this default initial value form is
evaluated in the lexical environment of the defclass form that defined
it, and the resulting value is stored in the slot.
#+end_quote
--- [[clhs::07_a.htm][Object Creation and Initialization]]

This is not what happens in eieio.  Rather, initform is evaluated even
when initarg is present.

* Emacs Lisp examples
Define a class with a slot that has an initarg and an initform involving
undefined function:
#+begin_src emacs-lisp
(defclass test-initform ()
  ((test-slot :initarg :test-slot :initform (identity (non-existent)))))
#+end_src

Now,
#+begin_src emacs-lisp :results code :wrap example emacs-lisp
(condition-case err (make-instance 'test-initform :test-slot 0)
  (t err))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(void-function non-existent)
#+end_example

Even though initform should not be evaluated since initarg is provided.

* Other references from CLHS
#+begin_quote
If the initialization argument has a value in the initialization
argument list, the value is stored into the slot of the newly created
object, overriding any :initform form associated with the slot.
#+end_quote
--- [[clhs::07_aa.htm][Initialization Arguments]]

#+begin_quote
An :initform form is used to initialize a slot only if no initialization
argument associated with that slot is given as an argument to
make-instance or is defaulted by :default-initargs.
#+end_quote
--- [[clhs::07_ac.htm][Defaulting of Initialization Arguments]]

#+begin_quote
If a slot has both an :initform form and an :initarg slot option, and
the initialization argument is defaulted using :default-initargs or is
supplied to make-instance, the captured :initform form is neither used
nor evaluated.
#+end_quote
--- [[clhs::07_ad.htm][Rules for Initialization Arguments]]

* Common Lisp examples
Define a class in exactly the same way as in Emacs Lisp:
#+begin_src lisp :results errors :wrap example lisp
(defclass test-initform ()
  ((test-slot :initarg :test-slot :initform (identity (non-existent)))))
#+end_src

#+RESULTS:
#+begin_example lisp
; in: DEFCLASS TEST-INITFORM
;     (NON-EXISTENT)
; 
; caught STYLE-WARNING:
;   undefined function: COMMON-LISP-USER::NON-EXISTENT
; 
; compilation unit finished
;   Undefined function:
;     NON-EXISTENT
;   caught 1 STYLE-WARNING condition
#+end_example

Appropriately,
#+begin_src lisp :results value verbatim :wrap example lisp
(ignore-errors (make-instance 'test-initform))
#+end_src

#+RESULTS:
#+begin_example lisp
NIL
#<UNDEFINED-FUNCTION NON-EXISTENT {10052CF123}>
#+end_example

But with initarg, there is no need to evaluate initform, and it is not
evaluated:
#+begin_src lisp :results value verbatim :wrap example lisp
(make-instance 'test-initform :test-slot 0)
#+end_src

#+RESULTS:
#+begin_example lisp
#<TEST-INITFORM {1005224E13}>
#+end_example

* Notes
** Initializing to quoted initform
Emacs 27 source claims:
#+begin_quote
Paul Landes said in an email:
> CL evaluates it if it can, and otherwise, leaves it as
> the quoted thing as you already have.  This is by the
> Sonya E. Keene book and other things I've look at on the
> web.
#+end_quote
--- [[file:/usr/share/emacs/27.2/lisp/emacs-lisp/eieio.el::;; Paul Landes said in an email:][EIEIO on initform quoting]]

And eieio does quote initform forms with cars being symbols that are not fbound:
#+begin_src lisp :results none
(defclass test-initform-quote ()
  ((test-slot :initform (non-existent))))
#+end_src

#+begin_src emacs-lisp :results code :wrap example emacs-lisp
(slot-value (make-instance 'test-initform-quote) 'test-slot)
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
(non-existent)
#+end_example

There is however no reference to Sonya E. Keene or anything else.  Meanwhile,
#+begin_src lisp :results errors :wrap example lisp
(defclass test-initform-quote ()
  ((test-slot :initform (non-existent))))
#+end_src

#+RESULTS:
#+begin_example lisp
; in: DEFCLASS TEST-INITFORM-QUOTE
;     (NON-EXISTENT)
; 
; caught STYLE-WARNING:
;   undefined function: COMMON-LISP-USER::NON-EXISTENT
; 
; compilation unit finished
;   Undefined function:
;     NON-EXISTENT
;   caught 1 STYLE-WARNING condition
#+end_example

#+begin_src lisp :results value verbatim :wrap example emacs-lisp
(ignore-errors (make-instance 'test-initform-quote))
#+end_src

#+RESULTS:
#+begin_example emacs-lisp
NIL
#<UNDEFINED-FUNCTION NON-EXISTENT {1003251A33}>
#+end_example

Also, when discussing interplay between initform and default-initargs, Sonya E. Keene mentions:
#+begin_quote
The value of an :initform is evaluated each time it is used to initialize a slot.
#+end_quote
--- Sonya E. Keene, Object Oriented Programming in Common Lisp:
    A Programmer's Guide.  9.3. Controlling Initialization
    With ~defclass~ Options

similarly stated in CLHS, in “Macro DEFCLASS”.

Emacs 28 source removes the claim but adds
#+begin_quote
FIXME: Historically, EIEIO used a heuristic to try and guess
whether the initform is a form to be evaluated or just
a constant.  We use `eieio--eval-default-p' to see what the
heuristic says and if it disagrees with normal evaluation
then tweak the initform to make it fit and emit
a warning accordingly.
#+end_quote
--- [[file:~/src/emacs/lisp/emacs-lisp/eieio.el::;; FIXME: Historically, EIEIO used a heuristic to try and guess][eieio in git]]

which arguably makes matters even more confusing.  There should be no
such heuristic.

* Patch
We offer a patch for Emacs 28 (master) but we can't build Emacs 28 (the
bug had been filed) so it's untested and “works for me”.