extract-instance-and-class-slotds does not respect slots with strange allocations

[xxxxxx]

Slots with overridden allocations (in my case mmap'd) are still (wrongly) assigned a slot location, because they are assumed to be :instance slots.

This is wrong, and here is a fix.

Index: l1-clos.lisp
===================================================================
--- l1-clos.lisp	(revision 12707)
+++ l1-clos.lisp	(working copy)
@@ -43,9 +43,11 @@
   (collect ((instance-slots)
 	    (shared-slots))
     (dolist (s slotds (values (instance-slots) (shared-slots)))
-      (if (eq (%slot-definition-allocation s) :class)
-        (shared-slots s)
-        (instance-slots s)))))
+      (case (%slot-definition-allocation s) 
+	(:class
+	   (shared-slots s))
+	(:instance
+	 (instance-slots s))))))

[xxxxxx]

Working patch this time?

[xxxxxx]

Sorry here is a working patch

Index: l1-clos.lisp =================================================================== --- l1-clos.lisp (revision 12707) +++ l1-clos.lisp (working copy) @@ -255,7 +255,7 @@

(extract-instance-and-class-slotds (call-next-method))

(setq instance-slots (sort-effective-instance-slotds instance-slots class cpl)) (do* ((loc 1 (1+ loc))

- (islotds instance-slots (cdr islotds))) + (islotds (remove-if-not (lambda (x) (eq :instance (%slot-definition-allocation x))) instance-slots) (cdr islotds)))

((null islotds))

(declare (fixnum loc)) (setf (%slot-definition-location (car islotds)) loc))

[gb]

Should be fixed in r12708 (and you're right that other values of :ALLOCATION should be accepted.)

[xxxxxx]

The patch you applied is like the first version (unfortunately incorrect) of my patch.

However, there is a problem with that. As we discussed on IRC, the slots with strange allocations should generally be treated as instance slots. The only difference is that they should have no location allocated in the memory layout of the class; they should allow slot-value-using-class to override how they are handled.

Unfortunately extract-instance-and-class-slotds is used in many places and if it is overridden in this way then the slots with strange allocations simply disappear.

Therefore, I made the second version of the patch which only affects the slot location assignment; for some reason the object length calculation was already correct. Can you test with elephant? Maybe I can open source our library later.

[gb]

The issue is that the :AROUND method on (COMPUTE-SLOTS (STD-CLASS)) needs to return slot-definition objects all of the slots, not merely those that have standard allocation. That used to happen by accident, but that depended on a function whose name suggested that it returned the sets of instance- and class-allocated slots actually returning the sets of non-class- and class-allocated slots (and something like your second patch having to filter the non-instance-allocated slots out of the first return value.) Your second patch worked, but it did so by depending on a bug in/artifact of EXTRACT-INSTANCE-AND-CLASS-SLOTDS and worked around that, and I'd rather not address the original problem that way.

(There are only two real callers of EXTRACT-INSTANCE-AND-CLASS-SLOTDS: this :AROUND method and something used by DESCRIBE and INSPECT, and neither of them should assume that all slots have standard allocation. There are a couple of internal CCL-package functions that call EXTRACT-INSTANCE-AND-CLASS-SLOTDS, but those functions aren't actually used.)

The number of slots in a (FUNCALLABLE-)STANDARD-INSTANCE is a function of the number of effective slot-definitions in its class whose allocation is :INSTANCE; that's never been affected by the behavior of EXTRACT-INSTANCE-AND-CLASS-SLOTDS.

I'll try to fix this today.

[dmitry_vk]

The bug is still present in trunk. This patch fixed it for me:

Index: l1-clos.lisp
===================================================================
--- l1-clos.lisp	(revision 12904)
+++ l1-clos.lisp	(working copy)
@@ -43,7 +43,7 @@
   (collect ((instance-slots)
 	    (shared-slots)
             (other-slots))
-    (dolist (s slotds (values (instance-slots) (shared-slots)))
+    (dolist (s slotds (values (instance-slots) (shared-slots) (other-slots)))
       (case (%slot-definition-allocation s)
         (:instance (instance-slots s))
         (:class (shared-slots s))

[rme]

Should be fixed as of r12905.