-
-
Notifications
You must be signed in to change notification settings - Fork 36
Effective slots #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Effective slots #159
Conversation
This is causing issues for me with compute-effective-slot-definition here https://github.com/sbcl/sbcl/blob/master/src/pcl/std-class.lisp#L1278 See fukamachi#154 for more details.
|
Work is in progress for the mito.dao.* package classes |
…ach taken to dao-table-view and dao-table-mixin. The actual definitions are in dao-table-column, named dao-table-column-standard-effective-slot-definitions
…definition to the effective slot class definition - changed the reader to be an accessor to facilitate the change
|
Hi, please take a look at I changed it in Besides that there were no changes to how things worked. I've basically copied over to the effective slot the contents of the following slots:
I think that's everything that was defined in the direct slot definitions. Is there anything else that needs to be checked? So with this everything should be working now. We should now be able to replace |
|
Fixed a test which was missing a |
|
So basically this is ready to be merged if it makes sense to you |
…host slots which had no col-type.
… depend on mito work
…operly checking for slots that have the the col-type slot in their class, and that they are actually bound. The first result of the bound results of col-type will be bound to the effective slot.
|
I had to add some extra checks for certain types of slots which do not have a |
|
Hi, I'm just following up here. Just a simple explanation of what this PR is doing. Effective slots are the slots available in the class once the class is initialized. Direct slots are when the class is being initialized, but not after initialization. What we accomplish with this PR is that when looking at a class and inspecting it, we can see the slot options and information in the runtime class that were added during the class definition. For example, doing Is there any information or questions I can answer about this PR? |
|
Just to add more context, this effective slot definition was missing, and the current code is doing all sorts of hacks and workarounds to get functionality that is built in by Common Lisp. For example:
CL already provides a |
|
Just to provide more information on the benefits of this. today I needed to use Which is defined as (defun find-child-columns (table slot)
(let (results)
(map-all-superclasses
(lambda (class)
(when (slot-exists-p class 'parent-column-map)
(maphash (lambda (child parent)
(when (eq parent (c2mop:slot-definition-name slot))
(push child results)))
(slot-value class 'parent-column-map))))
table)
results))But if we just use the built in CL functionality of effective class slots, we can replace that with the simpler (defun find-child-columns (table slot)
(let (results)
(when (slot-exists-p table 'parent-column-map)
(maphash (lambda (child parent)
(when (eq parent (c2mop:slot-definition-name slot))
(push child results)))
(slot-value table 'parent-column-map)))
results))Every call to |
fukamachi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion, and really sorry for the delay.
Could you add this method definition also for dao-table-view (src/core/dao/view.lisp) to take advantage of your changes?
(defmethod c2mop:effective-slot-definition-class ((class dao-table-view) &key)
'dao-table-column-standard-effective-slot-definitions)
Hi, that's there already as: (defmethod c2mop:effective-slot-definition-class ((class dao-table-view) &rest initargs)
(declare (ignorable initargs))
(find-class 'mito.dao.column:dao-table-column-standard-effective-slot-definitions))Not sure why I had to add the |
|
it now looks like: https://github.com/daninus14/mito/blob/a9e020bb7070042b90a2cc4774928181409cd7de/src/core/dao/view.lisp#L24-L26 (defmethod c2mop:effective-slot-definition-class ((class dao-table-view) &rest initargs)
(declare (ignorable initargs))
'dao-table-column-standard-effective-slot-definitions) |
|
|
||
| t/test.db | ||
| .qlot/ | ||
| .emacs* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
| .emacs* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that just ignores emacs desktop and settings files that emacs sometimes adds automatically to the project, just to keep them out of the git workflow. I can remove it as well, just let me know. It's a convenience to be able to do git add . instead of having to add file by file
Co-authored-by: Eitaro Fukamachi <e.arrows@gmail.com>
Co-authored-by: Eitaro Fukamachi <e.arrows@gmail.com>
Co-authored-by: Eitaro Fukamachi <e.arrows@gmail.com>
Co-authored-by: Eitaro Fukamachi <e.arrows@gmail.com>
|
Thanks! |
This is a correct implementation of effective slot definitions to mito.
Please see the issue #158
By the way, are there any other slots besides those which need to be available at runtime?
I noticed
table-column-referencesis a reader and not an accessor. Should it be set? See that I am not copying it below.Take a look at the functionality now:
In particular, notice that now we can properly use the
class-slotsfunction and get the runtime information we want without the workaround mentioned in the issue.