COMPUTE-SLOTS incorrect for structure-class and condition-class

Bug #1049423 reported by Douglas Katzman on 2012-09-12
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
SBCL
Undecided
Unassigned

Bug Description

For structures, a duplicate effective-slot-definition will appear if a slot is mentioned in the slot-description list that may accompany the :INCLUDE option such as to change the default or further specify the type. But even if nothing is changed, it still manifests as a duplicate slot definition. Example:

* (defstruct animal name size)
ANIMAL
* (defstruct (dog (:include animal (name "Spot") (size))) tail-wag-arc-radians)
DOG
* (sb-pcl:class-slots (find-class 'dog))
(#<SB-PCL::STRUCTURE-EFFECTIVE-SLOT-DEFINITION NAME>
 #<SB-PCL::STRUCTURE-EFFECTIVE-SLOT-DEFINITION SIZE>
 #<SB-PCL::STRUCTURE-EFFECTIVE-SLOT-DEFINITION NAME>
 #<SB-PCL::STRUCTURE-EFFECTIVE-SLOT-DEFINITION SIZE>
 #<SB-PCL::STRUCTURE-EFFECTIVE-SLOT-DEFINITION TAIL-WAG-ARC-RADIANS>)

This affects ~70 structure classes in a pristine image, plus stuff we define using the same approach of overriding a default. It's surprising this escaped attention thus far considering that DESCRIBE is pretty obviously screwy as a consequence.

Here is a probably-working patch for the STRUCTURE-CLASS method:

diff --git a/src/pcl/std-class.lisp b/src/pcl/std-class.lisp
index 9c077f9..ffe7b65 100644
--- a/src/pcl/std-class.lisp
+++ b/src/pcl/std-class.lisp
@@ -1125,14 +1125,22 @@
     (std-compute-slots-around class eslotds)))

 (defmethod compute-slots ((class structure-class))
- (mapcan (lambda (superclass)
- (mapcar (lambda (dslotd)
- (compute-effective-slot-definition
- class
- (slot-definition-name dslotd)
- (list dslotd)))
- (class-direct-slots superclass)))
- (reverse (slot-value class '%class-precedence-list))))
+ (let (eslotds)
+ (mapc (lambda (superclass)
+ (mapc (lambda (dslotd)
+ (let* ((slot-name (slot-definition-name dslotd))
+ (eslotd
+ (compute-effective-slot-definition
+ class slot-name (list dslotd)))
+ (found (member slot-name eslotds
+ :key #'slot-definition-name
+ :test #'string=)))
+ (if found
+ (rplaca found eslotd)
+ (push eslotd eslotds))))
+ (class-direct-slots superclass)))
+ (reverse (slot-value class '%class-precedence-list)))
+ (nreverse eslotds)))

There is something unsettling about the 'string=' and I think it's wrong, but at the time I fixed this I was also looking into what I think is another bug. Namely, if you include a structure and specify a list of slots whose descriptions to alter, it doesn't "work" to refer to the slot using a symbol in the wrong package. So are they compared by print-name or aren't they? Now that I think about it more, this could be a consequence of a bug which subsumes it, namely: if you specify a slot that doesn't exist at all in the ancestor, no complaint is generated. It looks like it's just ignored.

Back to the original problem - For condition-class, the confusion is greater since multiple inheritance yields possibly N instances of a same-named direct-slot-definition that have to be merged, in the manner of standard-class. The COMPUTE-SLOTS method should probably be a hybrid of the one for STANDARD-CLASS and STRUCTURE-CLASS, if it can't just do the STANDARD-CLASS thing entirely.

./src/runtime/sbcl --version
SBCL 1.0.58.28-60bb508-dirty

Douglas Katzman (dougk) wrote :

fixed in 91f7d85a9528eb81d8da70a2900d868ee29b36b7
and then again in 1891fa69bddd2cab9180b3fa9c7fcb3206174790

Changed in sbcl:
status: New → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers