Opened 9 years ago

Closed 8 years ago

#593 closed defect (fixed)

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

Reported by: xxxxxx Owned by: gb
Priority: normal Milestone: Clozure CL 1.4
Component: ANSI CL Compliance Version: trunk
Keywords: Cc: Kalyanov.Dmitry@…

Description (last modified by rme)

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)
     (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))))))

Attachments (1)

ccl-fix-slot-location-assignment.patch (662 bytes) - added by xxxxxx 9 years ago.
Working patch this time?

Download all attachments as: .zip

Change History (10)

Changed 9 years ago by xxxxxx

Working patch this time?

comment:1 Changed 9 years ago by 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))

comment:2 Changed 9 years ago by gb

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

comment:3 Changed 9 years ago by rme

  • Description modified (diff)

comment:4 Changed 9 years ago by 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.

comment:5 Changed 9 years ago by 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.

comment:6 Changed 8 years ago by rme

  • Milestone set to Clozure CL 1.4

comment:7 Changed 8 years ago by 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)
-    (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))

comment:8 Changed 8 years ago by dmitry_vk

  • Cc Kalyanov.Dmitry@… added

comment:9 Changed 8 years ago by rme

  • Resolution set to fixed
  • Status changed from new to closed

Should be fixed as of r12905.

Note: See TracTickets for help on using tickets.