Slot accessing issues in EIEIO

Slot accessing issues in EIEIO

[Zhu Zihao]

This
thread(https://lists.gnu.org/archive/html/emacs-devel/2020-05/msg00674.html)
reminds me about some issues on slot accessing in EIEIO.

Recently, I'm reading the source of
closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
package. It use eieio object, and put a piece of advice on eieio-oref to
intercept the reading slot action.

I don't think use an advice is the silver bullet. If different package put
different advice on the same function. The order of calling will make user
puzzled. There's slot-value-using-class for CLOS to handle slot accessing, but
there's nothing for EIEIO to do this.

Another question is, user may access the slot without the respect of eieio-oref via
another public interface, the eieio pattern in pcase. eieio pattern directly
access the slot index table to avoid useless type check. But another macro
with-slot, have to check for each binding because it use eieio-oref.

So I suggest to change the structure of eieio-oref. we can separate checking and
accessing into two different function. For macros bind multiple slots, checking
should be done once before accessing. And it's possible to introduce
slot-value-using-class generic in accessing function.

--
Zihao

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> Recently, I'm reading the source of
> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
> package. It use eieio object, and put a piece of advice on eieio-oref to
> intercept the reading slot action.

Yuck!  Can you explain why they need an advice on `eieio-oref`?


        Stefan

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

On Wed, 06 May 2020 21:28:18 +0800,
Stefan Monnier wrote:
> 
> > Recently, I'm reading the source of
> > closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
> > package. It use eieio object, and put a piece of advice on eieio-oref to
> > intercept the reading slot action.
> 
> Yuck!  Can you explain why they need an advice on `eieio-oref`?
> 
> 
>         Stefan

OK, a closql-object(This is a class defined in closql) is a proxy object to a
row in SQL database. So each time user tries to read a slot, closql must sync
values between database and Emacs first.

It's also possible to reinvent accessors for closql specific usage, but Jonas
doesn't do. I guess this way(hack on EIEIO) can reuse some facilities(like with-slots).

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

Zhu Zihao <all_but_last@163.com> writes:

> Recently, I'm reading the source of closql, A Emacs sqlite database
> ORM package.

Uhuh ;) Sorry for the lack of documentation.  It's not really intended
for use by other people at this point, but if you are brave enought to
dig through the implementation, then why not.

> I don't think use an advice is the silver bullet.

I ment to ask for a better mechanism but never got around to it.  What
I had in mind was for `eieio-oref' and `eieio-oset' to be defined as
generic functions.

> There's slot-value-using-class for CLOS to handle slot accessing, but
> there's nothing for EIEIO to do this.

I have no idea how that works.  Can you link to some documentation
and/or implemenation?

  Jonas

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Recently, I'm reading the source of
>> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
>> package. It use eieio object, and put a piece of advice on eieio-oref to
>> intercept the reading slot action.
>
> Yuck!  Can you explain why they need an advice on `eieio-oref`?
>
>
>         Stefan

A right.  That's one of the reasons I never bothered to get eieio
changed to better accommodate my extension; I expected a negative
reaction like this.

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

>>> Recently, I'm reading the source of
>>> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
>>> package. It use eieio object, and put a piece of advice on eieio-oref to
>>> intercept the reading slot action.
>>
>> Yuck!  Can you explain why they need an advice on `eieio-oref`?
>>
>>
>>         Stefan
>
> A right.  That's one of the reasons I never bothered to get eieio
> changed to better accommodate my extension; I expected a negative
> reaction like this.

Note the "Yuck" is about putting an advice on such a core function.
Not about the functionality that it provides (over which I don't have
an opinion yet since I don't know what it does).

IOW the "yuck" is saying "please request an extension so it can be
better accomodated" ;-)


        Stefan

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> Recently, I'm reading the source of
>>>> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
>>>> package. It use eieio object, and put a piece of advice on eieio-oref to
>>>> intercept the reading slot action.
>>>
>>> Yuck!  Can you explain why they need an advice on `eieio-oref`?
>>
>> A right.  That's one of the reasons I never bothered to get eieio
>> changed to better accommodate my extension; I expected a negative
>> reaction like this.
>
> Note the "Yuck" is about putting an advice on such a core function.
> Not about the functionality that it provides (over which I don't have
> an opinion yet since I don't know what it does).
>
> IOW the "yuck" is saying "please request an extension so it can be
> better accomodated" ;-)

Okay understood.  I guess I was feeling a bit insecure about having
written a ORMish thing in first place, and was a bit over sensitive
because of that.  ;D

Re: Slot accessing issues in EIEIO

[Eric Abrahamsen]

Jonas Bernoulli <jonas@bernoul.li> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>>>> Recently, I'm reading the source of
>>>>> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
>>>>> package. It use eieio object, and put a piece of advice on eieio-oref to
>>>>> intercept the reading slot action.
>>>>
>>>> Yuck!  Can you explain why they need an advice on `eieio-oref`?
>>>
>>> A right.  That's one of the reasons I never bothered to get eieio
>>> changed to better accommodate my extension; I expected a negative
>>> reaction like this.
>>
>> Note the "Yuck" is about putting an advice on such a core function.
>> Not about the functionality that it provides (over which I don't have
>> an opinion yet since I don't know what it does).
>>
>> IOW the "yuck" is saying "please request an extension so it can be
>> better accomodated" ;-)
>
> Okay understood.  I guess I was feeling a bit insecure about having
> written a ORMish thing in first place, and was a bit over sensitive
> because of that.  ;D

I would love to see this implemented, so I hope our collective
insecurities do not hinder us :)

I would use this, and have considered writing something similar, and am
very, very glad I won't have to.

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> OK, a closql-object (This is a class defined in closql) is a proxy object to a
> row in SQL database. So each time user tries to read a slot, closql must sync
> values between database and Emacs first.

OK, I'm starting to see vaguely what this is about, but it's still
rather fuzzy.  To better understand, I suggest that I'll throw some
solutions so you can shoot them down explaining why they're
not applicable.

Would it work to use the `slot-missing` generic function?


        Stefan

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

On Thu, 07 May 2020 05:58:11 +0800,
Stefan Monnier wrote:
> 
> > OK, a closql-object (This is a class defined in closql) is a proxy object to a
> > row in SQL database. So each time user tries to read a slot, closql must sync
> > values between database and Emacs first.
> 
> OK, I'm starting to see vaguely what this is about, but it's still
> rather fuzzy.  To better understand, I suggest that I'll throw some
> solutions so you can shoot them down explaining why they're
> not applicable.
> 
> Would it work to use the `slot-missing` generic function?
> 
> 
>         Stefan
slot-missing just work, but I don't think it gets things done right.

If we use slot-missing. We have to manage the column names manually instead of
present them in slots. We should keep the proxy object doesn't have any
slot to make slot-missing work, or your slot name may collide with SQL column
name.

Thus, we have to give up the direct way to describe the structure of SQL
database. We can't use defclass and slots definition now because we wanna use
slot-missing. If I create another type of proxy object(e.g. will log to some file
when accessing a slot), I have to repeat the pattern I mentioned above. It's
harmful for reusability.

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> slot-missing just work, but I don't think it gets things done right.
>
> If we use slot-missing. We have to manage the column names manually instead of
> present them in slots. We should keep the proxy object doesn't have any
> slot to make slot-missing work, or your slot name may collide with SQL column
> name.

You're going too fast here: I have no idea what slots there are currently
in the object, why they're there, what kind of slots the `slot-missing`
thingy would catch, what is the relation between the "real" slots and
the others, ...

In the current solution, when there is a name collision between a "real"
slot and a non-real one, how is it resolved?

> Thus, we have to give up the direct way to describe the structure of
> SQL database.

What is this "direct way" you apparently currently use?

> when accessing a slot), I have to repeat the pattern I mentioned above.

I haven't yet understood what this pattern is.


        Stefan

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

Hmm, could you please take a look at this piece of code?
https://github.com/emacscollective/epkg/blob/master/epkg.el#L137

In this defclass definition. slots prefixed with "closql-" was reserved for
internal usage. and other slots present the columns in SQL database.

Obviously, accessing a slot in these definitions is valid. For example,
accessing slot "foobar" in that object will trigger real slot-missing method
because our definitions don't contain it. And accessing slots will trigger a
sync operation between Emacs VM and SQL database.

When we put the sync logic in slot-missing. How to distinguish really missing slots
and "simulated slots" is a problem.

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

On Thu, 07 May 2020 12:00:52 +0800,
Zhu Zihao wrote:
> 
> Hmm, could you please take a look at this piece of code?
> https://github.com/emacscollective/epkg/blob/master/epkg.el#L137
> 
> In this defclass definition. slots prefixed with "closql-" was reserved for
> internal usage. and other slots present the columns in SQL database.
> 
> Obviously, accessing a slot in these definitions is valid. For example,
> accessing slot "foobar" in that object will trigger real slot-missing method
> because our definitions don't contain it. And accessing slots will trigger a
> sync operation between Emacs VM and SQL database.
> 
> When we put the sync logic in slot-missing. How to distinguish really missing slots
> and "simulated slots" is a problem.

When we put the sync logic in slot-missing. How to distinguish really missing slots
and "simulated slots" is a problem.
^^^
from

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> In this defclass definition. slots prefixed with "closql-" was reserved for
> internal usage. and other slots present the columns in SQL database.

> Obviously, accessing a slot in these definitions is valid. For example,
> accessing slot "foobar" in that object will trigger real slot-missing method
> because our definitions don't contain it. And accessing slots will trigger a
> sync operation between Emacs VM and SQL database.

[ By "VM" I assume you mean just the Emacs process, right?  ]

Can you point me to where/how/when the "sync" operation is done
(e.g. how does Emacs know when the SQL database is modified?  Is the
whole database mirrored during the sync or only a whole column or only
a specific element of a specific column or something else?).

> When we put the sync logic in slot-missing.  How to distinguish really
> missing slots and "simulated slots" is a problem.

The way I'd imagine it is that you'd have 2 objects, one that's a sort
of cache of the DB which could have slots like those listed in the `(defclass
epkg-package` of epkg.el, and another on top (or in front of it) which
is the one that's exposed to the rest of the code and this one only has
closql-internal slots, so `slot-missing` is always triggered for
simulated slots (and it can either answer by looking up
the slot value in the cache object, or fetch the answer from the
database and maybe store the result in the cache object).


        Stefan

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> When we put the sync logic in slot-missing. How to distinguish really missing slots
> and "simulated slots" is a problem.

Maybe another approach is to rely on the `slot-unbound` notion: when
a slot is not in-sync, mark it unbound so any access to it will call the
generic function `slot-unbound`.  I don't think this would support
write-access via closql (or at least I can't see how that could be used
to catch modification of the object slot to propagate them to the DB).


        Stefan

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

>> OK, a closql-object (This is a class defined in closql) is a proxy
>> object to a row in SQL database. So each time user tries to read a
>> slot, closql must sync values between database and Emacs first.

Not every time; only when the slot is still unbound.

> OK, I'm starting to see vaguely what this is about, but it's still
> rather fuzzy.

I'll give it a try...

Every object slot represents a table column.  There is one additional
column at the front of each row, which holds a representation of the
object's class.

Class allocated slots on the other hand are not stored in the database.

Two types of object slots exist (or three depending on how you count):

1. Direct slots whose value is stored directly in the row.
2. Indirect slots whose value is a secondary key identifying a row in
   another table.
   
For example an object that represents an Emacs package has a `name'
slot, which is sufficiently represented by a direct slot.  But the list
of provided features is stored using an indirect slot.  We might want to
SELECT the package that provides a certain feature using SQL instead of
creating a list of every package (which is expensive) and then finding
the desired package using, for example `cl-find-if'.

The elements in the other table, which is accessed indirectly, may be
objects themselves or they may be simple lists.

Once an indirect slot has been accessed its value is cached in the
object, so future uses don't require database queries.

> To better understand, I suggest that I'll throw some solutions so you
> can shoot them down explaining why they're not applicable.
>
> Would it work to use the `slot-missing` generic function?

I think the above addresses that.  But I guess the questions becomes
"Well, what about `slot-unbound' then?".

For `eieio-oref' that *might* work but not for `eieio-oset'.  The user
may wish to change the value of any object slot; it doesn't matter
whether it is direct or indirect and in the latter case whether the slot
is still unbound or has already been resolved.

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

I had not seen this reply of yours before I wrote my reply to an earlier
message a few minutes ago.

>> In this defclass definition. slots prefixed with "closql-" was reserved for
>> internal usage. and other slots present the columns in SQL database.
>
>> Obviously, accessing a slot in these definitions is valid. For example,
>> accessing slot "foobar" in that object will trigger real slot-missing method
>> because our definitions don't contain it. And accessing slots will trigger a
>> sync operation between Emacs VM and SQL database.
>
> [ By "VM" I assume you mean just the Emacs process, right?  ]
>
> Can you point me to where/how/when the "sync" operation is done
> (e.g. how does Emacs know when the SQL database is modified?  Is the
> whole database mirrored during the sync or only a whole column or only
> a specific element of a specific column or something else?).

For read operations there might be no sync at all.

When setting the value of a slot, then only that gets synced to the
database.

There is no guarantee that the database has not changed.  This can be an
issue and one should either avoid writes or be careful about it.

In the case of the Emacsmirror's database, e.g. the of `epkg-package'
objects, this isn't much of a problem because only I am supposed to
write to that database.  Users of `epkg' ("browse the Emacsmirror
package database") and `borg' ("assimilate Emacs packages as Git
submodules") are not supposed to modify the database (or the objects).

I have started using `closql' for `forge' ("Access Git forges from
Magit") as well, and here it is not without problems each user has their
own database which they obviously have to write too.

`closql' is definitely not a silver bullet.  It works nicely for what I
designed it for.  It works well enough for me, its author, in another
context, but I have no idea how well it would work for others for tasks
I did not consider.

>> When we put the sync logic in slot-missing.  How to distinguish really
>> missing slots and "simulated slots" is a problem.
>
> The way I'd imagine it is that you'd have 2 objects, one that's a sort
> of cache of the DB which could have slots like those listed in the `(defclass
> epkg-package` of epkg.el, and another on top (or in front of it) which
> is the one that's exposed to the rest of the code and this one only has
> closql-internal slots, so `slot-missing` is always triggered for
> simulated slots (and it can either answer by looking up
> the slot value in the cache object, or fetch the answer from the
> database and maybe store the result in the cache object).

That sounds too complicated for my use-cases at least, and I don't see
what we gain by doing that except not having to advice `eieio-oref' and
`eieio-oset', which by the way I don't think is all that horrible.

Sure, instead of

   (defun eieio-oset--closql-oset (fn obj slot value)
     (if (closql-object--eieio-childp obj)
         (closql-oset obj slot value)
       (funcall fn obj slot value)))

   (advice-add 'eieio-oset :around #'eieio-oset--closql-oset)

   (defun closql-oset (obj slot value)
     ...)

I would have liked to write

   (cl-defmethod eieio-oset ((obj closql-object) slot value)
     ...)

which, by the way is what I suggest we make possible.

I haven't gotten around to ask for the latter so far because as long as
`closql' supports older Emacs releases it would have to keep doing
something like the former for their benefit anyway.

Re: Slot accessing issues in EIEIO

[Jonas Bernoulli]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> When we put the sync logic in slot-missing. How to distinguish really
>> missing slots and "simulated slots" is a problem.
>
> Maybe another approach is to rely on the `slot-unbound` notion: when
> a slot is not in-sync, mark it unbound so any access to it will call the
> generic function `slot-unbound`.  I don't think this would support
> write-access via closql (or at least I can't see how that could be used
> to catch modification of the object slot to propagate them to the DB).

;D

See reply to sibling.  (You pretty much got it figure out.)

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

On Thu, 07 May 2020 22:13:49 +0800,
Jonas Bernoulli wrote:

> That sounds too complicated for my use-cases at least, and I don't see
> what we gain by doing that except not having to advice `eieio-oref' and
> `eieio-oset', which by the way I don't think is all that horrible.

Yes, any hack based on slot-missing or slot-unbound looks like a kind of
anti-pattern.

I've mentioned slot-value-using-class above, but this was based on the
MOP(metaobject protocol) of CLOS. For EIEIO, a simple solution is make
eieio-oref and (setf eieio-oref) a generic

> I haven't gotten around to ask for the latter so far because as long as
> `closql' supports older Emacs releases it would have to keep doing
> something like the former for their benefit anyway.

I think Emacs maintainers can publish a new version of EIEIO in GNU ELPA for
older Emacs releases, just like they did for cl-generic and seq

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> That sounds too complicated for my use-cases at least,

Maybe it would require a fair bit pf change, but I don't see why it
would be significantly more complicated than what you currently do if
you were to start from scratch with such a design (to clarify: I'm
mostly interested in making sure that we have a good solution for
someone who wants to do that and is starting from scratch; having a good
solution for the existing closql.el is a somewhat secondary concern
since the advice mechanism does the job).

You can structure the two-objects solution in various different ways:
e.g. you could keep your objects as they are, but create
a "proxy/firewall/interposer" object which contains only a single field
(which points to the real object).  Then when accessing fields of this
object you can use the `slot-missing` generic function to do what your
current advice does, simply operating on the underlying real object.

It should be possible to create a "generic interposer" solution like
that which can be used instead of your advice and that is not specific
to closql.

> and I don't see what we gain by doing that except not having to advice
> `eieio-oref' and `eieio-oset', which by the way I don't think is all
> that horrible.

Advising `eieio-oref` means slowing down every access to every slot of
every EIEIO object.  That's the main problem for me.  For some EIEIO
uses it's not a problem, but some EIEIO uses do care about the
performance of accesses to their slots.

> I would have liked to write
>
>    (cl-defmethod eieio-oset ((obj closql-object) slot value)
>      ...)

We could go there, but there's again an issue of performance:

A) We could keep the code as is and just turn the `defun` into a `cl-defgeneric`,
   in which case performance will be unaffected *until* someone comes
   with his own `(cl-defmethod eieio-oref ...)` at which point again all
   accesses to all slots of all EIEIO get impacted (if there's only one
   (default) method, cl-generic optimizes away the method dispatch).

   The advantage compared to your advice is that subsequent
   `(cl-defmethod eieio-oref ...)` don't impose additional slowdown
   (unless they dispatch on another argument).

B) We could try and work harder in the implementation of `eieio-oref` so
   that the added cost discussed in (A) is always present but is made
   useful (by reducing the rest of the cost).  This would likely require
   changes in cl-generic.el as well.
   Currently eieio-oref basically does a gethash to find the slot's offset
   and then an `aref` to get the slot's content.  The method dispatch
   adds another gethash, so we'd want this dispatch-time gethash to
   replace the one that fetches the slot offset.
   This would be great, but it requires a good bit more work.


-- Stefan

Re: Slot accessing issues in EIEIO

[Daniel Colascione]

On 5/6/20 9:43 AM, Jonas Bernoulli wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
>>>>> Recently, I'm reading the source of
>>>>> closql(https://github.com/emacscollective/closql), A Emacs sqlite database ORM
>>>>> package. It use eieio object, and put a piece of advice on eieio-oref to
>>>>> intercept the reading slot action.
>>>>
>>>> Yuck!  Can you explain why they need an advice on `eieio-oref`?
>>>
>>> A right.  That's one of the reasons I never bothered to get eieio
>>> changed to better accommodate my extension; I expected a negative
>>> reaction like this.
>>
>> Note the "Yuck" is about putting an advice on such a core function.
>> Not about the functionality that it provides (over which I don't have
>> an opinion yet since I don't know what it does).
>>
>> IOW the "yuck" is saying "please request an extension so it can be
>> better accomodated" ;-)
> 
> Okay understood.  I guess I was feeling a bit insecure about having
> written a ORMish thing in first place, and was a bit over sensitive
> because of that.  ;D

FWIW, this ORM sounds pretty neat, especially since it works with all 
the EIEIO field sugar.

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

>> To better understand, I suggest that I'll throw some solutions so you
>> can shoot them down explaining why they're not applicable.
>> Would it work to use the `slot-missing` generic function?
> I think the above addresses that.  But I guess the questions becomes
> "Well, what about `slot-unbound' then?".

Here's another solution instead of the pair-of-objects solution:

For each slot, use two different slot names:
- one for the "virtual slot" corresponding to a data base column.
- one for the "real slot" that caches the value from the database.

So when `slot-value` accesses a virtual slot, you get to `slot-unbound`
which can then lookup (and fill if needed) the corresponding real slot
(whose name can presumably be obtained by adding or removing some
prefix/suffix).


        Stefan

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

On Thu, 07 May 2020 22:52:26 +0800,
Stefan Monnier wrote:
> 
> > That sounds too complicated for my use-cases at least,
> 
> Maybe it would require a fair bit pf change, but I don't see why it
> would be significantly more complicated than what you currently do if
> you were to start from scratch with such a design (to clarify: I'm
> mostly interested in making sure that we have a good solution for
> someone who wants to do that and is starting from scratch; having a good
> solution for the existing closql.el is a somewhat secondary concern
> since the advice mechanism does the job).
> 
> You can structure the two-objects solution in various different ways:
> e.g. you could keep your objects as they are, but create
> a "proxy/firewall/interposer" object which contains only a single field
> (which points to the real object).  Then when accessing fields of this
> object you can use the `slot-missing` generic function to do what your
> current advice does, simply operating on the underlying real object.
> 
> It should be possible to create a "generic interposer" solution like
> that which can be used instead of your advice and that is not specific
> to closql.
> 
> > and I don't see what we gain by doing that except not having to advice
> > `eieio-oref' and `eieio-oset', which by the way I don't think is all
> > that horrible.
> 
> Advising `eieio-oref` means slowing down every access to every slot of
> every EIEIO object.  That's the main problem for me.  For some EIEIO
> uses it's not a problem, but some EIEIO uses do care about the
> performance of accesses to their slots.
> 
> > I would have liked to write
> >
> >    (cl-defmethod eieio-oset ((obj closql-object) slot value)
> >      ...)
> 
> We could go there, but there's again an issue of performance:
> 
> A) We could keep the code as is and just turn the `defun` into a `cl-defgeneric`,
>    in which case performance will be unaffected *until* someone comes
>    with his own `(cl-defmethod eieio-oref ...)` at which point again all
>    accesses to all slots of all EIEIO get impacted (if there's only one
>    (default) method, cl-generic optimizes away the method dispatch).
> 
>    The advantage compared to your advice is that subsequent
>    `(cl-defmethod eieio-oref ...)` don't impose additional slowdown
>    (unless they dispatch on another argument).
> 
> B) We could try and work harder in the implementation of `eieio-oref` so
>    that the added cost discussed in (A) is always present but is made
>    useful (by reducing the rest of the cost).  This would likely require
>    changes in cl-generic.el as well.
>    Currently eieio-oref basically does a gethash to find the slot's offset
>    and then an `aref` to get the slot's content.  The method dispatch
>    adds another gethash, so we'd want this dispatch-time gethash to
>    replace the one that fetches the slot offset.
>    This would be great, but it requires a good bit more work.
> 
> 
> -- Stefan
It looks like a diffcult job. Maybe we can fix the eieio pattern of pcase.

In this piece of code:

(defclass test-class () ((slot)))

(pcase (make-instance 'test-class)
  ((eieio slot)
   slot))
;; => unbound

An unbound marker will be returned on the fly without running slot-unbound
in pcase with eieio pattern. Is it reasonable?

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

> Maybe we can fix the eieio pattern of pcase.
>
> In this piece of code:
>
> (defclass test-class () ((slot)))
>
> (pcase (make-instance 'test-class)
>   ((eieio slot)
>    slot))
> ;; => unbound
>
> An unbound marker will be returned on the fly without running slot-unbound
> in pcase with eieio pattern. Is it reasonable?

It should likely obey the bound/unbond distinction, and call
slot-missing when applicable, yes.  Patch welcome.


        Stefan

Re: Slot accessing issues in EIEIO

[Zhu Zihao]

[-- Attachment #1: Type: text/plain, Size: 579 bytes --]

On Fri, 08 May 2020 11:48:22 +0800,
Stefan Monnier wrote:
> 
> > Maybe we can fix the eieio pattern of pcase.
> >
> > In this piece of code:
> >
> > (defclass test-class () ((slot)))
> >
> > (pcase (make-instance 'test-class)
> >   ((eieio slot)
> >    slot))
> > ;; => unbound
> >
> > An unbound marker will be returned on the fly without running slot-unbound
> > in pcase with eieio pattern. Is it reasonable?
> 
> It should likely obey the bound/unbond distinction, and call
> slot-missing when applicable, yes.  Patch welcome.
> 
> 
>         Stefan
I just give it a try ;)


[-- Attachment #2: 0001-Make-pcase-pattern-eieio-respect-slot-access-related.patch --]
[-- Type: text/plain, Size: 1774 bytes --]

From 2fe8fe4c1ae1becb4439bb64576a43ec3fd0b5b7 Mon Sep 17 00:00:00 2001
From: Zhu Zihao <all_but_last@163.com>
Date: Fri, 8 May 2020 17:01:56 +0800
Subject: [PATCH] Make pcase pattern 'eieio' respect slot access related
 functions.

* lisp/emacs-lisp/eieio.el: Make pcase pattern respect slot-missing and
slot-unbound
---
 lisp/emacs-lisp/eieio.el | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index fe2b80be01e..999d75f79e9 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -359,16 +359,13 @@ eieio
     ;; FIXME: `pcase' does not do a good job here of sharing tests&code among
     ;; various branches.
     `(and (pred eieio-object-p)
-          (app eieio-pcase-slot-index-table ,is)
-          ,@(mapcar (lambda (field)
-                      (let* ((name (if (consp field) (car field) field))
-                             (pat (if (consp field) (cadr field) field))
-                             (i (make-symbol "index")))
-                        `(and (let (and ,i (pred natnump))
-                                (eieio-pcase-slot-index-from-index-table
-                                 ,is ',name))
-                              (app (pcase--flip aref ,i) ,pat))))
-                    fields))))
+        ,@(mapcar (lambda (field)
+                    (pcase-exhaustive field
+                      (`(,name ,pat)
+                        `(app (pcase--flip eieio-oref ',name) ,pat))
+                      ((pred symbolp)
+                       `(app (pcase--flip eieio-oref ',field) ,field))))
+                  fields))))
 \f
 ;;; Simple generators, and query functions.  None of these would do
 ;;  well embedded into an object.
-- 
2.26.2

Re: Slot accessing issues in EIEIO

[Stefan Monnier]

>> It should likely obey the bound/unbond distinction, and call
>> slot-missing when applicable, yes.  Patch welcome.
> I just give it a try ;)

LGTM, thanks, pushed to master,


        Stefan