Hey Tim,
> On 4. Jun 2025, at 21:09, Tim Düsterhus <t...@bastelstu.be> wrote:
>
> Hi
>
> Am 2025-06-04 14:19, schrieb Nick:
>> Fair. If someone really wants to add random_int(): "well, that assumption
>> doesn't hold anymore, deal” from my side.
>
> 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.
>
> 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.
>
>>> I believe at the moment that RFC text is all there is. :-) I don't know
>>> that it's worth opening a discussion without at least a mostly-done
>>> implementation. Also, Ilija is rather busy on other tasks at the moment,
>>> as am I. (Unless someone else wants to jump in to implement it, which
>>> would be fine.)
>> 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
https://github.com/php/php-src/pull/18757/files#diff-fdc22521b074231ac6de0df97aeb23a4759ad9b668d3b9ed3fedf8727f5795a4R21
It checks re-assignment, and expects:
> Cannot modify protected(set) readonly property Test1::$prop from global scope
It’s tested for readonly props, readonly class and promoted properties.
Correct me, but this is what you are talking about, isn't it? Anything else you
are missing?
