Subject: Re: bug#27718: 26.0.50; (cl-typep instance-ab 'class-a) gives wrong answer when compiled

From: [email-masked]
Date: Fri, 21 Jul 2017 08:23:03 -0400

tags 27718 + patch
quit

[email-masked] writes:

>     ~/src/emacs$ .../emacs -Q -batch -f batch-byte-compile cl-typep-subclass.el
>     ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.el
>     (cl-typep eitest-ab ’class-a) => t
>     ~/src/emacs$ .../emacs -Q -batch -l cl-typep-subclass.elc
>     (cl-typep eitest-ab ’class-a) => nil

lisp/emacs-lisp/eieio.el:260:

    (defmacro defclass (name superclasses slots &rest options-and-doc)
      ...
           ;; When using typep, (typep OBJ 'myclass) returns t for objects which
           ;; are subclasses of myclass.  For our predicates, however, it is
           ;; important for EIEIO to be backwards compatible, where
           ;; myobject-p, and myobject-child-p are different.
           ;; "cl" uses this technique to specify symbols with specific typep
           ;; test, so we can let typep have the CLOS documented behavior
           ;; while keeping our above predicate clean.

           (put ',name 'cl-deftype-satisfies #',testsym2)

The problem is that the `put' doesn't take effect until runtime, so the
compiler inlines `cl-typep' incorrectly.  Using `function-put' here (and
`function-get' in `cl-typep') solves it (requires also the patches for
#27016[1]), although it seems a bit coincidental that `class-a' happens
to be a function as well, so I'm not sure if this is the right thing.
Stefan, any thoughts?

>From 4ea34c9da04c8eabc754f2ca64ef4e2ac7c59cac Mon Sep 17 00:00:00 2001
From: Noam Postavsky <[email-masked]>
Date: Fri, 21 Jul 2017 07:59:21 -0400
Subject: [PATCH v1] Fix (cl-typep instance-ab 'class-a) compilation
 (Bug#27718)

Using `function-put' instead of `put' for the `cl-deftype-satisfies'
property lets the compiler know about it while compiling the file so
that any calls to `cl-typep' will be inlined correctly.
* lisp/emacs-lisp/eieio.el (defclass): Use `function-put' for the
`cl-deftype-satisfies' property.
* lisp/emacs-lisp/cl-macs.el (cl-typep): Also use `function-get' to
check the 'cl-deftype-satisfies' property.
---
 lisp/emacs-lisp/cl-macs.el | 7 +++++--
 lisp/emacs-lisp/eieio.el   | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index 451e6490d7..f5a4f99aad 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -2991,8 +2991,11 @@ (define-inline cl-typep (val type)
       ((and (pred symbolp) type (guard (get type 'cl-deftype-handler)))
        (inline-quote
         (cl-typep ,val ',(funcall (get type 'cl-deftype-handler)))))
-      ((and (pred symbolp) type (guard (get type 'cl-deftype-satisfies)))
-       (inline-quote (funcall #',(get type 'cl-deftype-satisfies) ,val)))
+      ((and (pred symbolp) type (guard (or (function-get type 'cl-deftype-satisfies)
+                                           (get type 'cl-deftype-satisfies))))
+       (inline-quote (funcall #',(or (function-get type 'cl-deftype-satisfies)
+                                     (Get type 'cl-deftype-satisfies))
+                              ,val)))
       ((and (or 'nil 't) type) (inline-quote ',type))
       ((and (pred symbolp) type)
        (let* ((name (symbol-name type))
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index 1a7de55fce..2c333c16f4 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -246,7 +246,7 @@ (defmacro defclass (name superclasses slots &rest options-and-doc)
        ;; test, so we can let typep have the CLOS documented behavior
        ;; while keeping our above predicate clean.
 
-       (put ',name 'cl-deftype-satisfies #',testsym2)
+       (function-put ',name 'cl-deftype-satisfies #',testsym2)
 
        (eieio-defclass-internal ',name ',superclasses ',slots ',options-and-doc)
 
-- 
2.11.1

> You can see that tests 23 and 24 will fail when you do 'make
> eieio-tests TEST_LOAD_EL=no'.

This actually isn't true until the patch for #24402[2] is applied.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27016#140
[2]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24402#71