Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123516
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-497-g97f96844c-fm-20240526.001-g97f96844
Precedence: bulk
MIME-Version: 1.0
Message-ID: <6d644f0f-cbeb-484c-b267-bf1d97e6d27a@app.fastmail.com>
In-Reply-To: <89b695d7-661f-4c1b-ac4c-4480ad158e83@app.fastmail.com>
References: <0a6a61cd-f203-4dea-a7f8-97e6b885c52d@app.fastmail.com>
 <CAAwdEzB0rFhrai8W+B3PhnYEoOR-QEZYUm=R9+8+18Yo9WOgyQ@mail.gmail.com>
 <aadcd4ed-5f73-fbb9-93fb-42f2710e61fe@php.net>
 <ED20EE91-9BBB-4FED-8440-BAE748CBF5AB@gmail.com>
 <CAAwdEzBrRJMBQbY0R1Tp8RWpUP_DW6twNvGqnKDp3jjbiBFE-Q@mail.gmail.com>
 <734bb8e8-2fdf-4e50-9039-e53c99ee4930@app.fastmail.com>
 <A87ABAAF-E8DE-4417-BF96-19268CD7F668@gmail.com>
 <89b695d7-661f-4c1b-ac4c-4480ad158e83@app.fastmail.com>
Date: Wed, 05 Jun 2024 14:28:00 +0000
To: "php internals" <internals@lists.php.net>
Subject: Re: [PHP-DEV] [RFC] Asymmetric Visibility, v2
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Fri, May 31, 2024, at 8:59 PM, Larry Garfield wrote:
> On Fri, May 31, 2024, at 5:45 PM, Claude Pache wrote:
>>> Le 31 mai 2024 =C3=A0 18:08, Larry Garfield <larry@garfieldtech.com>=
 a =C3=A9crit :
>>>=20
>>> However, this also brings up another interesting issue: readonly pro=
perties (in 8.3) DO allow redeclaration, essentially adjusting the prope=
rty scope (the class that declares it) to make the visibility check pass=
. That is, the definition of the class it is private to changes, which i=
s different from how inheritance works elsewhere.  When the parent write=
s to the same property, a special check is needed to verify the two prop=
erties are related.  All that special casing effectively means that read=
only in 8.4 wouldn't really be "write once + private(set)", but "write o=
nce + private(set) - final", which is... just kinda screwy.  That means =
our options are:
>>>=20
>>> * A BC break on readonly (not allowing it to be overridden)
>>> * Make readonly an exception to the implicit final.
>>> * Just don't allow readonly with aviz after all.
>>
>> Another possible option is:
>>
>> * Make readonly be `protected(set)` by default.
>>
>> That would weaken the originally intended semantics of readonly, but =
in=20
>> a compatible and acceptable way?
>>
>> =E2=80=94Claude
>
> Only sort of compatible.  It would allow readonly props to be=20
> redefined, and wouldn't break any existing code, I think... but it=20
> would mean any time you use readonly, suddenly a child class can come=20
> along and mess with it out from under you.
>
> In practice that's likely not a common concern, but it is a behavior=20
> change.  I think it's possible (I need to confirm with Ilija), if we=20
> want that slight BC break, but I don't know if we do.
>
> --Larry Garfield

Ilija and I have been discussing this issue over the last few days.  We =
agree that `private(set)` should imply `final`, as that eliminates a bun=
ch of issues both implementation-wise and expectation-wise.  However, th=
at causes an issue for `readonly`. =20

The root issue is that if we say "`readonly int $x` is really just `priv=
ate(set) readonly int $x`", that runs into the issue of "whelp, you've j=
ust made readonly always final, which is a BC break."  So that's no good.

We see a couple of ways to resolve this, presented below in our order of=
 preference.

1. Disallow readonly with aviz =3D> No BC break, and we don't need to de=
fine readonly in terms of private(set).  The only really useful combinat=
ion anyway would be `readonly protected(set)`, in which case the protect=
ed(set) is doing 90% of the work already.  There's few cases where the r=
eadonly is truly necessary at that point.  Any other oddball edge cases =
could be handled by a custom hook.
2. Make `readonly` implicitly `protected(set)` unless you explicitly spe=
cify `private(set)` =3D> Would have the most consistent result, and this=
 is probably the cleanest in the engine, as `readonly private(set)` woul=
d mean exactly what it says on the tin, with no inconsistency of "well i=
t's kinda sorta `private(set)`" as `readonly` has now.  However, this wo=
uld be an expectation change, as suddenly all readonly properties could =
be written to from a child class.  That may be good in some cases but it=
's possible some objects could have unexpected behavior if they didn't e=
xpect to be extended.  (No existing code will break, but you could now d=
o things to it in a child class that the author didn't anticipate.)
3. You can't mix `readonly` with `private(set)`, but can use other visib=
ilities =3D> No BC break, and we don't need to define readonly in terms =
of `private(set)`.  However, it means the implicit `private(set)` of `re=
adonly` and an explicit private(set) behave differently (one is final, o=
ne is not).  It also unclear if a `readonly` property can be overridden =
with `readonly protected(set)` only, or also `readonly private(set)`.  I=
f the latter, does it become implicitly `final` at that point?
4. `readonly` behaves differently for an explicit (final) and implicit (=
not-final) `private(set)` =3D> No BC break, but it's kinda weird and non=
-obvious to explain.  It also has the same non-obvious inheritance quest=
ions as option 3.

We consider only the first two to be really viable.  For simplicity, we'=
d favor doing option 1, and if desired option 2 could be presented in th=
e future as its own RFC as that is technically a behavior change, not ju=
st addition, so deserves careful consideration.  However, if there is a =
clear consensus to go with option 2 now, we're open to that.

--Larry Garfield