GNU bug report logs - #78786
31.0.50; eieio-core.el requires byte compilation

Previous Next

To reply to this bug, email your comments to 78786 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; eieio-core.el requires byte compilation
Date: Fri, 13 Jun 2025 15:20:32 +0000

Evaluating eieio-core.el without compilation currently results in
breakage:

(load "eieio-core.el")
(cl-defstruct foo x)
(eieio-oset (make-foo) 'x nil)

results in:

  signal(wrong-type-argument (eieio--class #s(cl-structure-class :name foo :docstring nil :parents (#s(cl-structure-class :name cl-structure-object :docstring "The root parent of all \"normal\" CL structs" :parents (#s(built-in-class :name record :docstring "Abstract type of objects with slots." :parents (...) :slots nil :index-table nil)) :slots [] :index-table #<hash-table eq 0/0 ...> :tag cl-structure-object :type nil :named nil :print t :children-sym cl-struct-cl-structure-object-tags)) :slots [#s(cl-slot-descriptor :name x :initform nil :type t :props nil)] :index-table #<hash-table eq 1/1 ...> :tag foo :type nil :named nil :print t :children-sym cl-struct-foo-tags)))
  (or (let* ((cl-x cl-x)) (and (memq (type-of cl-x) cl-struct-eieio--class-tags) t)) (signal 'wrong-type-argument (list 'eieio--class cl-x)))
  (progn (or (let* ((cl-x cl-x)) (and (memq (type-of cl-x) cl-struct-eieio--class-tags) t)) (signal 'wrong-type-argument (list 'eieio--class cl-x))) (aref cl-x 4))
  (let* ((cl-x class)) (progn (or (let* ((cl-x cl-x)) (and (memq (type-of cl-x) cl-struct-eieio--class-tags) t)) (signal 'wrong-type-argument (list 'eieio--class cl-x))) (aref cl-x 4)))
  (aref (let* ((cl-x class)) (progn (or (let* ((cl-x cl-x)) (and (memq (type-of cl-x) cl-struct-eieio--class-tags) t)) (signal 'wrong-type-argument (list 'eieio--class cl-x))) (aref cl-x 4))) slot-idx)
  (let* ((sd (aref (let* ((cl-x class)) (progn (or (let* ... ...) (signal ... ...)) (aref cl-x 4))) slot-idx)) (st (let* ((cl-x sd)) (progn (or (let* (...) (and ... t)) (signal 'wrong-type-argument (list ... cl-x))) (aref cl-x 3))))) (cond ((not (eieio--perform-slot-validation st value)) (signal 'invalid-slot-type (list (let* ((cl-x class)) (progn (or ... ...) (aref cl-x 1))) slot st value))) ((alist-get :read-only (let* ((cl-x sd)) (progn (or (let* ... ...) (signal ... ...)) (aref cl-x 4)))) (signal 'eieio-read-only (list (let* ((cl-x class)) (progn (or ... ...) (aref cl-x 1))) slot)))))
  (if eieio-skip-typecheck nil (setq slot-idx (- slot-idx '1)) (let* ((sd (aref (let* ((cl-x class)) (progn (or ... ...) (aref cl-x 4))) slot-idx)) (st (let* ((cl-x sd)) (progn (or (let* ... ...) (signal ... ...)) (aref cl-x 3))))) (cond ((not (eieio--perform-slot-validation st value)) (signal 'invalid-slot-type (list (let* (...) (progn ... ...)) slot st value))) ((alist-get :read-only (let* ((cl-x sd)) (progn (or ... ...) (aref cl-x 4)))) (signal 'eieio-read-only (list (let* (...) (progn ... ...)) slot))))))
  eieio--validate-slot-value(#s(cl-structure-class :name foo :docstring nil :parents (#s(cl-structure-class :name cl-structure-object :docstring "The root parent of all \"normal\" CL structs" :parents (#s(built-in-class :name record :docstring "Abstract type of objects with slots." :parents (#s(built-in-class :name atom :docstring "Abstract supertype of anything but cons cells." :parents ... :slots nil :index-table nil)) :slots nil :index-table nil)) :slots [] :index-table #<hash-table eq 0/0 ...> :tag cl-structure-object :type nil :named nil :print t :children-sym cl-struct-cl-structure-object-tags)) :slots [#s(cl-slot-descriptor :name x :initform nil :type t :props nil)] :index-table #<hash-table eq 1/1 ...> :tag foo :type nil :named nil :print t :children-sym cl-struct-foo-tags) 1 nil x)

The expected behavior is for this code to complete without error.

The failure is because eieio--validate-slot-value is called for a class
(object) of type cl-structure-object, but calls eieio--class-slots on
that object; it could call cl--class-slots instead, and then this bug
wouldn't happen.

When byte-compiled, we optimize out the type checks so everything works.
This is achieved by the cl-declaim calls surrounding the cl-defstruct of
eieio--class.

When interpreted directly, cl-defstruct always adds the type checks, so
the error pops up.

However, I have my doubts about whether we might end up calling
eieio--class-class-slots or eieio--class-initarg-tuples on a
cl-structure-object, which would return its "type" or "named" slots
(unless I miscounted, sorry).  So I'd like to fix these while we're
there (in the latter case, we're on slow code paths so this should not
be a problem; in the former case, if there is a performance impact, we
could consider moving the class-slots slot to cl--class instead,
reserving a useless slot in it for cl-structure-object objects).

I propose moving to cl--class-* accessors where we can, and calling
eieio--class-slot-name-index and eieio--class-initarg-to-attribute only
on known/checked eieio--class objects.

But if an alternative is desired (I think this may be the case because I
suggested changing one of the accessors before), and the
initarg/class-slot case is always safe, we could simply error out when
trying to evaluate eieio-core.el directly rather than compiling it.

While this is related to bug#78685, I think there are other,
non-bug-induced circumstances in which we end up interpreting
eieio-core.el, so we should either allow it or make it fail less subtly.

Message #8 received at 78786 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78786 <at> debbugs.gnu.org
Subject: Re: bug#78786: 31.0.50; eieio-core.el requires byte compilation
Date: Sat, 05 Jul 2025 12:23:45 -0400

> I propose moving to cl--class-* accessors where we can, and calling
> eieio--class-slot-name-index and eieio--class-initarg-to-attribute only
> on known/checked eieio--class objects.

Yes, calling `eieio--class-slot-name-index` on non-EIEIO classes is
a bug, we should fix the code to do it only when it is an EIEIO class.


        Stefan

Message #11 received at 78786 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 78786 <at> debbugs.gnu.org
Subject: Re: bug#78786: 31.0.50; eieio-core.el requires byte compilation
Date: Sat, 05 Jul 2025 18:59:56 +0000

"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes:

>> I propose moving to cl--class-* accessors where we can, and calling
>> eieio--class-slot-name-index and eieio--class-initarg-to-attribute only
>> on known/checked eieio--class objects.
>
> Yes, calling `eieio--class-slot-name-index` on non-EIEIO classes is
> a bug, we should fix the code to do it only when it is an EIEIO class.

So this?

diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el
index 72640e88301..6d9a0199ea6 100644
--- a/lisp/emacs-lisp/eieio-core.el
+++ b/lisp/emacs-lisp/eieio-core.el
@@ -740,7 +740,8 @@ eieio-oref
       (if (not c)
 	  ;; It might be missing because it is a :class allocated slot.
 	  ;; Let's check that info out.
-	  (if (setq c (eieio--class-slot-name-index class slot))
+	  (if (and (eieio--class-p class)
+                   (setq c (eieio--class-slot-name-index class slot)))
 	      ;; Oref that slot.
 	      (aref (eieio--class-class-allocation-values class) c)
 	    ;; The slot-missing method is a cool way of allowing an object author
@@ -783,8 +784,9 @@ eieio-oref-default
     (if (not c)
 	;; It might be missing because it is a :class allocated slot.
 	;; Let's check that info out.
-	(if (setq c
-		  (eieio--class-slot-name-index cl slot))
+	(if (and (eieio--class-p cl)
+                 (setq c
+		       (eieio--class-slot-name-index cl slot)))
 	    ;; Oref that slot.
 	    (aref (eieio--class-class-allocation-values cl)
 		  c)
@@ -808,8 +810,9 @@ eieio-oset
       (if (not c)
 	  ;; It might be missing because it is a :class allocated slot.
 	  ;; Let's check that info out.
-	  (if (setq c
-		    (eieio--class-slot-name-index class slot))
+	  (if (and (eieie--class-p class)
+                   (setq c
+		         (eieio--class-slot-name-index class slot)))
 	      ;; Oset that slot.
 	      (progn
 	        (eieio--validate-class-slot-value class c value slot)
@@ -849,7 +852,8 @@ eieio-oset-default
     (if (not c)
         ;; It might be missing because it is a :class allocated slot.
         ;; Let's check that info out.
-        (if (setq c (eieio--class-slot-name-index class slot))
+        (if (and (eieio--class-p class)
+                 (setq c (eieio--class-slot-name-index class slot)))
             (progn
               ;; Oref that slot.
               (eieio--validate-class-slot-value class c value slot)
diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el b/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
index fced6bc3df2..f00914d9fe9 100644
--- a/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
+++ b/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
@@ -1047,7 +1047,8 @@ eieio-test-defstruct-slot-value
                 (slot-value x 'c)))
     (setf (slot-value x 'a) 1)
     (should (eq (eieio-test--struct-a x) 1))
-    (should-error (setf (slot-value x 'c) 3) :type 'eieio-read-only)))
+    (should-error (setf (slot-value x 'c) 3) :type 'eieio-read-only)
+    (should-error (eieio-oref x 'd) :type 'invalid-slot-name)))
 
 (defclass foo-bug-66938 (eieio-instance-inheritor)
   ((x :initarg :x

The new test didn't actually fail before, but I figured it might catch
some future bug.

Pip

Message #14 received at 78786 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78786 <at> debbugs.gnu.org
Subject: Re: bug#78786: 31.0.50; eieio-core.el requires byte compilation
Date: Sat, 05 Jul 2025 17:30:37 -0400

>> Yes, calling `eieio--class-slot-name-index` on non-EIEIO classes is
>> a bug, we should fix the code to do it only when it is an EIEIO class.
>
> So this?

LGTM, yes.


        Stefan


> diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el
> index 72640e88301..6d9a0199ea6 100644
> --- a/lisp/emacs-lisp/eieio-core.el
> +++ b/lisp/emacs-lisp/eieio-core.el
> @@ -740,7 +740,8 @@ eieio-oref
>        (if (not c)
>  	  ;; It might be missing because it is a :class allocated slot.
>  	  ;; Let's check that info out.
> -	  (if (setq c (eieio--class-slot-name-index class slot))
> +	  (if (and (eieio--class-p class)
> +                   (setq c (eieio--class-slot-name-index class slot)))
>  	      ;; Oref that slot.
>  	      (aref (eieio--class-class-allocation-values class) c)
>  	    ;; The slot-missing method is a cool way of allowing an object author
> @@ -783,8 +784,9 @@ eieio-oref-default
>      (if (not c)
>  	;; It might be missing because it is a :class allocated slot.
>  	;; Let's check that info out.
> -	(if (setq c
> -		  (eieio--class-slot-name-index cl slot))
> +	(if (and (eieio--class-p cl)
> +                 (setq c
> +		       (eieio--class-slot-name-index cl slot)))
>  	    ;; Oref that slot.
>  	    (aref (eieio--class-class-allocation-values cl)
>  		  c)
> @@ -808,8 +810,9 @@ eieio-oset
>        (if (not c)
>  	  ;; It might be missing because it is a :class allocated slot.
>  	  ;; Let's check that info out.
> -	  (if (setq c
> -		    (eieio--class-slot-name-index class slot))
> +	  (if (and (eieie--class-p class)
> +                   (setq c
> +		         (eieio--class-slot-name-index class slot)))
>  	      ;; Oset that slot.
>  	      (progn
>  	        (eieio--validate-class-slot-value class c value slot)
> @@ -849,7 +852,8 @@ eieio-oset-default
>      (if (not c)
>          ;; It might be missing because it is a :class allocated slot.
>          ;; Let's check that info out.
> -        (if (setq c (eieio--class-slot-name-index class slot))
> +        (if (and (eieio--class-p class)
> +                 (setq c (eieio--class-slot-name-index class slot)))
>              (progn
>                ;; Oref that slot.
>                (eieio--validate-class-slot-value class c value slot)
> diff --git a/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el b/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
> index fced6bc3df2..f00914d9fe9 100644
> --- a/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
> +++ b/test/lisp/emacs-lisp/eieio-tests/eieio-tests.el
> @@ -1047,7 +1047,8 @@ eieio-test-defstruct-slot-value
>                  (slot-value x 'c)))
>      (setf (slot-value x 'a) 1)
>      (should (eq (eieio-test--struct-a x) 1))
> -    (should-error (setf (slot-value x 'c) 3) :type 'eieio-read-only)))
> +    (should-error (setf (slot-value x 'c) 3) :type 'eieio-read-only)
> +    (should-error (eieio-oref x 'd) :type 'invalid-slot-name)))
>  
>  (defclass foo-bug-66938 (eieio-instance-inheritor)
>    ((x :initarg :x
>
> The new test didn't actually fail before, but I figured it might catch
> some future bug.
>
> Pip

Message #19 received at 78786-done <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 78786-done <at> debbugs.gnu.org
Subject: Re: bug#78786: 31.0.50; eieio-core.el requires byte compilation
Date: Wed, 09 Jul 2025 15:31:53 +0000

"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes:

>>> Yes, calling `eieio--class-slot-name-index` on non-EIEIO classes is
>>> a bug, we should fix the code to do it only when it is an EIEIO class.
>>
>> So this?
>
> LGTM, yes.

Pushed, thanks.  Closing this bug optimistically.

>> @@ -808,8 +810,9 @@ eieio-oset
>>        (if (not c)
>>  	  ;; It might be missing because it is a :class allocated slot.
>>  	  ;; Let's check that info out.
>> -	  (if (setq c
>> -		    (eieio--class-slot-name-index class slot))
>> +	  (if (and (eieie--class-p class)
                        ^

Fixed this one.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.