Conversation
Emit ZEND_ACC_PUBLIC_SET / PROTECTED_SET / PRIVATE_SET from the corresponding Modifiers::*_SET flags in generated arginfo, gated to PHP 8.4+ where asymmetric visibility was introduced. Previously private(set) and friends in stubs parsed without error but produced no set-visibility flag.
@readonly on DOM property stubs was documentation only and did not translate to any runtime flag, so reflection reported the properties as writable while the write_property handler threw on assignment. Declaring them public private(set) lets the engine reject external writes via the normal visibility check and lets ReflectionProperty:: isWritable() answer honestly. Also drops @readonly from the three non-virtual HTMLCollection $children properties (Element, DocumentFragment, Document) and applies private(set) there as well. Depends on the assertion removal from phpGH-21769 for isReadable() to stop aborting on these properties.
iluuu1994
left a comment
iluuu1994
left a comment
There was a problem hiding this comment.
Makes sense to me, given Nora's comment that these can change at runtime.
|
I presume you'll need to change the error of |
Emit public(set) / protected(set) / private(set) modifiers in the generated <fieldsynopsis> so the manual reflects set visibility for asymmetrically visible properties.
Good point. |
After converting virtual properties from @readonly to private(set), dom_write_property still threw a readonly-modification error. Since the handler replaces zend_std_write_property, the engine's own asymmetric-visibility check is bypassed on the write path, so the DOM handler now raises it explicitly via zend_asymmetric_visibility_property_modification_error() when the caller lacks set access. The readonly error is kept as a fallback.
| } | ||
| } | ||
|
|
||
| if (UNEXPECTED(!hnd->write_func)) { |
| if (prop && (prop->flags & ZEND_ACC_PPP_SET_MASK) && | ||
| !zend_asymmetric_property_has_set_access(prop)) { | ||
| zend_asymmetric_visibility_property_modification_error(prop, "modify"); | ||
| } else { |
There was a problem hiding this comment.
Hmm, but is this else branch executable? Do we still have readonly properties left??
4 participants
No description provided.