closer-mop breaks defclass in LispWorks
tfeb opened this issue · 4 comments
In LW, if you are in the C2CL-USER
package, then standard-class
is closer-mop:standard-class
, not cl:standard-class
. But defclass
is cl:defclass
. This means that a form like
(defclass my-class () ())
will create a class whose metaclass is cl:standard-class
, not closer-mop:standard-class
. Doom pretty rapidly follows: if you define methods on validate-superclass
to allow a class whose metaclass is not standard-class
to be a subclass or superclass of one which is then this completely fails to do what you expect unless you add suitable metaclass options to all your defclass
forms.
This will, I think, be true for any platform where cl:standard-class
is not closer-mop:standard-class
unless defclass
is modified.
A fix for this might be to add a version of defclass
something like:
(defmacro defclass (class supers slots &rest options)
`(cl:defclass ,class ,supers
,slots
,@(if (assoc ':metaclass options)
options
(cons '(:metaclass standard-class) options))))
If the above seems like an acceptable solution (I'm not sure it is), then I could submit a PR for LW in a bit, although not for other platforms where this is a problem (which I think includes at least Allegro).
Thanks a lot for the report. However, this "design" is on purpose. There were two requirements that I tried to keep balanced with this choice:
- It should be easy to :use :c2cl in your package declarations instead of :cl. So this means you should have the right defclass, standard-class, defgeneric, defmethod, etc. at hand.
- However, there should also be no unnecessary performance penalty when doing so.
Especially point two is relevant: The optimization model of a CLOS implementation is to look out for standard metaclasses and then choose optimized code for these cases. A (cl:defclass (...) (...) ... (:metaclass c2cl:standard-class)) is not an instance of a standard metaclass, so will potentially deviate from an optimized code path. I think it's therefore better that c2cl:defclass is the same as cl:defclass. You will have to change the relevant classes to be explicit about :metaclass c2cl:standard-class.
The design could be the other way around: Make everything c2cl:standard-class by default, and manually use cl:defclass in case you discover a performance issue. It then boils down to which default is more common. This project already exists since quite some time, and your report is the first one that stumbles over this issue, so this seems to be a hint that the chosen default has worked so far.
However, I'm also open to counter arguments. Maybe there is something I'm missing and the default should change. If there are good reasons to do so, I'm interested to hear about them.
It should be easy to :use :c2cl in your package declarations instead of :cl. So this means you should have the right defclass, standard-class, defgeneric, defmethod, etc. at hand.
However, there should also be no unnecessary performance penalty when doing so.
I think these are good points. One option would be to have c2cl/conformant
and c2cl/fast
(but not those names) packages, where if you used ctcl/conformant
everything would be easy but, perhaps, slower as classes would generally not hit the fast-path if there was one, while c2cl/fast
would be what is currently c2cl
.
But that's easy for me to say, as I don't have to maintain all this.
Also (and perhaps more to the point) I wrote a little test thing to demonstrate what I'd like to be able to do and can't ... and it seems to work fine in LW (it always worked in SBCL). So it's possible I was confused because I'd been loading stuff into a warm image which I'd already broken in some way. I need to look at this more to check my report is even real.
OK, I was testing the wrong thing: the problem is real but it only happens when trying to make a class which is a subclass of something with a nonstandard metaclass, when the validate-superclass
method doesn't do what you might think it does.
I think your points about performance are good: certainly CLOS implementations used to to heroic magic to make things not-horribly-slow if the metaclass was cl:standard-class
. In other words what it does now is right, I think.
So I think this is closable (I will close it in fact). I might work on a PR to provide a variant c2cl
package which is less surprising at the cost of possibly bad performance.