Conversation
arnaud-lb
left a comment
arnaud-lb
left a comment
There was a problem hiding this comment.
This seems sensible, but I would try to use the new APIs in a few additional use-cases, to validate them
ext/spl/spl_iterators.c
Outdated
| if (object->u.callback_filter.object) { | ||
| zend_get_gc_buffer_add_obj(gc_buffer, object->u.callback_filter.object); | ||
| } | ||
| if (object->u.callback_filter.closure) { | ||
| zend_get_gc_buffer_add_obj(gc_buffer, object->u.callback_filter.closure); | ||
| } |
There was a problem hiding this comment.
There is possibly a use-case for a zend_fcc_gc function
There was a problem hiding this comment.
I tried to add it to zend_gc.h but got some unresolved type issue :/ and I don't understand how it has no issue finding HashTable and zend_object
There was a problem hiding this comment.
It makes sense to define the function outside of zend_gc.h 👍 This is what we do for the get_gc object handler for instance.
I used it in ext-SQLite3 to further validate it. |
ext/sqlite3/sqlite3.c
Outdated
| zend_fcc_addref(&fcc); | ||
| memcpy(&collation->cmp_func, &fcc, sizeof(zend_fcall_info_cache)); |
There was a problem hiding this comment.
Wouldn't it make sense to introduce a macro/inline function for this?
There was a problem hiding this comment.
That make sense, how about zend_fcc_copy()? Or should dup be used instead?
Girgias
force-pushed
the
fcc-closure
branch
2 times, most recently
from
153f531 to
c405f03
Compare
3 participants
This PR improves the FCC struct and API and contains a use case of the improved API.
First, a
closurefield is added to the FCC struct to keep a reference for closure when the callable is an object.Secondly, we add APIs to add/del FCC references to manage userland callables references in a global API. We also add some QoL APIs to call FCCs, compare them, and check that they are initialized.
Thirdly, we use this new API in a concrete refactoring case in SPL.