Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127585
Content-Type: text/plain;
	charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.600.51.1.1\))
Subject: Re: [PHP-DEV] Allow hooks in `readonly` promoted properties
Date: Wed, 4 Jun 2025 21:39:01 +0800
References: <B0250A18-3C43-4DFD-9D99-3391C23D9D65@nicksdot.dev>
 <f4d0567f-c364-433b-b5b7-60fae49f9046@app.fastmail.com>
 <28F14D0F-8483-418D-85D1-660564A4EAE2@nicksdot.dev>
 <bfec2063-5304-48c1-bc19-203eb62c1237@app.fastmail.com>
 <26AA81AF-1610-494D-A811-10F7B282CBD4@nicksdot.dev>
 <1c0df916e4cfbc746b5dddf935d165a9@bastelstu.be>
To: internals@lists.php.net
In-Reply-To: <1c0df916e4cfbc746b5dddf935d165a9@bastelstu.be>
Message-ID: <FC67945A-75FE-4C56-B5F7-05E2735AB2D6@nicksdot.dev>
From: php@nicksdot.dev (Nick)

Hey Tim,

> On 4. Jun 2025, at 21:09, Tim D=C3=BCsterhus <tim@bastelstu.be> wrote:
>=20
> Hi
>=20
> 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=E2=80=9D from my side.
>=20
> Semantically once you involved inheritance it isn't that easy. It is =
allowed to override an =E2=80=9Cunhooked=E2=80=9D property with a hooked =
property and in the =E2=80=9CReadonly Amendments=E2=80=9D RFC we already =
decided that inheriting from a `readonly` class by a non-`readonly` =
class should not be valid.
>=20
> 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.
>=20
> Making this legal might also inhibit engine optimization. Currently if =
you know that a property is readonly you can theoretically optimize:
>=20
>    if ($object->foo !=3D=3D null) {
>        do_something($object->foo);
>    }
>=20
> into:
>=20
>    if (($foo =3D $object->foo) !=3D=3D null) {
>        do_something($foo);
>    }
>=20
> to avoid reading `$object->foo` twice, which for example would need to =
check visibility twice.
>=20
>>> 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 =E2=80=9Cyou can just do things=E2=80=9D. 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=E2=80=99d appreciate =
feedback from people more experienced than I am. Thanks!
>=20
> 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-fdc22521b074231ac6de0=
df97aeb23a4759ad9b668d3b9ed3fedf8727f5795a4R21

It checks re-assignment, and expects:

> Cannot modify protected(set) readonly property Test1::$prop from =
global scope

It=E2=80=99s 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?=