Hey Tim,
> On 4. Jun 2025, at 21:09, Tim Düsterhus <[email protected]> wrote:
>
> Semantically once you involved inheritance it isn't that easy. It is allowed
> to override an “unhooked” property with a hooked property and in the
> “Readonly Amendments” RFC we already decided that inheriting from a
> `readonly` class by a non-`readonly` class should not be valid.
I added tests for this. The behaviour you expect seems confirmed.
>
> So if you would be allowed to override a readonly unhooked property with a
> hooked property that has a `get` hook that is not a pure function, you would
> make the property effectively mutable, which is something that users of the
> class can't expect. It violates the history property of the Liskov
> substitution principle.
>
> Making this legal might also inhibit engine optimization. Currently if you
> know that a property is readonly you can theoretically optimize:
>
> if ($object->foo !== null) {
> do_something($object->foo);
> }
>
> into:
>
> if (($foo = $object->foo) !== null) {
> do_something($foo);
> }
>
> to avoid reading `$object->foo` twice, which for example would need to check
> visibility twice.
Added Zend/tests/property_hooks/readonly_property_backed_inheritance_3.phpt
Personally, I’d argue nothing unexpected happens here and everything is fair
game.
If we overwrite parents from a child it will naturally result in a changed get
hook result.
This, however, does not mean the actual class state has changed.
Opinions?
>> People often say “you can just do things”. So I did, and tried to contribute
>> the code for your existing RFC text:
>> https://github.com/php/php-src/pull/18757
>> Can it really be such a little change? I’d appreciate feedback from people
>> more experienced than I am. Thanks!
>
> Your test cases really only scratch the surface of what should be tested. You
> are basically just verifying that it compiles. In fact you are not even
> testing that reassigning the property is disallowed, because the test fails
> due to a visibility error. In fact it appears that the `readonly` check comes
> before the visibility check, which would imply that the `readonly` doesn't
> have an effect: https://3v4l.org/nqgpL
You are right. I edited the tests accordingly.
—
Cheers,
Nick
