Allow readonly properties to be reinitialized once during cloning #10389
Conversation
kocsismate
force-pushed
the
readonly-clone-reinit
branch
from
01f03db to
d88f16b
Compare
kocsismate
force-pushed
the
readonly-clone-reinit
branch
from
d88f16b to
d6bbf1a
Compare
kocsismate
force-pushed
the
readonly-clone-reinit
branch
from
d6bbf1a to
1aa6fe5
Compare
|
I rebased to current master + pushed a new commit fixing some of the review comments |
kocsismate
force-pushed
the
readonly-clone-reinit
branch
from
c10c4cd to
75d4d9d
Compare
| @@ -833,6 +828,16 @@ ZEND_API zval *zend_std_write_property(zend_object *zobj, zend_string *name, zva | |||
| variable_ptr = &EG(error_zval); | |||
| goto exit; | |||
| } | |||
| if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { | |||
There was a problem hiding this comment.
I just realized that this is actually wrong. if (UNEXPECTED(prop_info->flags & ZEND_ACC_READONLY)) { should be done before zend_verify_property_type, because otherwise side-effects like __toString() will run even if the property cannot be assigned. Z_PROP_FLAG_P(variable_ptr) &= ~IS_PROP_REINITABLE; however should be run after. @kocsismate Can you adjust that?
There was a problem hiding this comment.
@iluuu1994 Yes, sure. I actually had the very same implementation initially for a few minutes, just to switch it to the current implementation. :)
There was a problem hiding this comment.
Adding a test should be simple enough. Create a class with __toString and assign to a readonly property that's already initialized.
There was a problem hiding this comment.
Yep, I have already created a similar test case locally :)
RFC: https://wiki.php.net/rfc/readonly_amendments
Split from #9497