[RFC] Optional interfaces #17288
[RFC] Optional interfaces #17288
Conversation
tontonsb
changed the title
| if (iface) { | ||
| UPDATE_IS_CACHEABLE(iface); | ||
| } | ||
| } | ||
| } | ||
| ce->num_interfaces = num_implementable_interfaces; |
There was a problem hiding this comment.
This line is the reason your tests are segfaulting, this could be an immutable class entry. You may only modify non-immutable and non cached ce's. Move this line to after the lazy class loading code in the zend_try down below.
e.g. right before if (CG(unlinked_uses)) { (probably).
There was a problem hiding this comment.
I also wonder what caching semantics will play in the following scenario:
- You do a request and an optional interface is not available, this class is cached without that interface.
- A future request comes in, and reuses the cache, but since this may be via a different code path it's possible that the interface is now available. Yet because the class was cached, this won't implement the interface now even though it's available at this point in time. I'm not entirely sure if this is a real problem but it should be checked. I also don't know what assumptions in the engine this may break.
| @@ -1048,8 +1048,9 @@ zend_class_entry *zend_persist_class_entry(zend_class_entry *orig_ce) | |||
| for (i = 0; i < ce->num_interfaces; i++) { | |||
| zend_accel_store_interned_string(ce->interface_names[i].name); | |||
| zend_accel_store_interned_string(ce->interface_names[i].lc_name); | |||
| // TODO: should we store .is_optional here? How? | |||
There was a problem hiding this comment.
No special handling is needed, the bool is copied by zend_shared_memdup_free down below.
| @@ -803,6 +803,7 @@ static void zend_file_cache_serialize_class(zval *zv, | |||
| for (i = 0; i < ce->num_interfaces; i++) { | |||
| SERIALIZE_STR(interface_names[i].name); | |||
| SERIALIZE_STR(interface_names[i].lc_name); | |||
| // SERIALIZE_BOOL(interface_names[i].is_optional) ? | |||
There was a problem hiding this comment.
No special handling is needed, this is implicitly handled. The pointers need special handling to change their offsets, but the bool is simply a primitive.
This is an implementation attempt of https://wiki.php.net/rfc/optional-interfaces
It's my first attempt at modifying PHP sources, so I am very open to any suggestions and guidance on how this should be actually solved. But for now I'll briefly explain my approach:
interface_name_listandinterface_nameare added to parser.interface_name_listis used instead ofclass_name_listforimplements_listandinterface_extends_list. Theinterface_nameis a name that can be prefixed by a?in which case it will set aZEND_CLASS_NAME_OPTIONALflag on theattrof that token.zend_interface_nameis defined and used forinterface_names. The difference fromzend_class_nameis that thezend_interface_namealso holds a booleanis_optionalwhich is set according to theZEND_CLASS_NAME_OPTIONALflag.ZEND_FETCH_CLASS_SILENTflag onzend_fetch_class_by_nameif the interface nameis_optional. In case an optional interface fails to fetch, it's just skipped.I'm not really sure what should be done for Opcache here and I haven't yet understood how to work with booleans there. I haven't looked into Reflection at all yet.