[RFC] Extend #[\Override] to target properties #19061
Conversation
|
Thanks for looking over this. I think I've addressed all the comments, please let me know if there's anything else. The RFC has been approved, so if the PR looks ok as is, I'll rebase the branch and regenerate the conflicting arginfo. |
|
@jiripudil Thank you. I've updated the PR to the latest master, fixed the conflict and added 52af261 to make the loop more robust when adding additional checks after the hook checks. |
| zend_attribute *override_attribute = zend_get_attribute_str(prop->attributes, "override", sizeof("override")-1); | ||
| if (override_attribute) { | ||
| prop->flags |= ZEND_ACC_OVERRIDE; | ||
| } |
There was a problem hiding this comment.
Side-note: In the long term, I would prefer if zend_internal_attribute.validator would add these flags. I'm not sure this is currently possible, because validator doesn't receive the original target, but just the ce. This would also work automatically for internal classes.
Regardless, that can be done in a second step. Also means a small BC break for validator.
Zend/zend_inheritance.c
Outdated
| @@ -2316,6 +2318,7 @@ static void zend_do_implement_interfaces(zend_class_entry *ce, zend_class_entry | |||
| void zend_inheritance_check_override(const zend_class_entry *ce) | |||
| { | |||
| zend_function *f; | |||
| zend_property_info *prop; | |||
There was a problem hiding this comment.
Please declare new variables inline.
There was a problem hiding this comment.
I stumbled upon this as well, but thought I'd not comment on it for consistency. I'll fix both of them when preparing the PR for the merge.
|
@jiripudil Now merged, thank you. Don't forget to adjust the RFC status / move it to the correct section. |
|
Will do, thank you! |
https://wiki.php.net/rfc/override_properties