Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123521
Message-ID: <10AAAA51-9538-4F97-83D1-91F154C745F7@gmail.com>
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_65A2C20C-881C-4BE5-A0B1-B1FF463D2536"
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3774.600.62\))
Subject: Re: [PHP-DEV] [RFC] Asymmetric Visibility, v2
Date: Wed, 5 Jun 2024 18:56:57 +0200
In-Reply-To: <6d644f0f-cbeb-484c-b267-bf1d97e6d27a@app.fastmail.com>
Cc: php internals <internals@lists.php.net>
To: Larry Garfield <larry@garfieldtech.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>
 <6d644f0f-cbeb-484c-b267-bf1d97e6d27a@app.fastmail.com>
From: claude.pache@gmail.com (Claude Pache)


--Apple-Mail=_65A2C20C-881C-4BE5-A0B1-B1FF463D2536
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8



> Le 5 juin 2024 =C3=A0 16:28, Larry Garfield <larry@garfieldtech.com> a =
=C3=A9crit :
>=20
> 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 =
properties (in 8.3) DO allow redeclaration, essentially adjusting the =
property 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 is different from how inheritance works elsewhere.  When the =
parent writes to the same property, a special check is needed to verify =
the two properties are related.  All that special casing effectively =
means that readonly in 8.4 wouldn't really be "write once + =
private(set)", but "write once + 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.
>>>=20
>>> Another possible option is:
>>>=20
>>> * Make readonly be `protected(set)` by default.
>>>=20
>>> That would weaken the originally intended semantics of readonly, but =
in=20
>>> a compatible and acceptable way?
>>>=20
>>> =E2=80=94Claude
>>=20
>> 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.
>>=20
>> 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.
>>=20
>> --Larry Garfield
>=20
> 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 =
bunch of issues both implementation-wise and expectation-wise.  However, =
that causes an issue for `readonly`. =20
>=20
> The root issue is that if we say "`readonly int $x` is really just =
`private(set) readonly int $x`", that runs into the issue of "whelp, =
you've just made readonly always final, which is a BC break."  So that's =
no good.
>=20
> We see a couple of ways to resolve this, presented below in our order =
of preference.
>=20
> 1. Disallow readonly with aviz =3D> No BC break, and we don't need to =
define readonly in terms of private(set).  The only really useful =
combination anyway would be `readonly protected(set)`, in which case the =
protected(set) is doing 90% of the work already.  There's few cases =
where the readonly 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 =
specify `private(set)` =3D> Would have the most consistent result, and =
this is probably the cleanest in the engine, as `readonly private(set)` =
would mean exactly what it says on the tin, with no inconsistency of =
"well it's kinda sorta `private(set)`" as `readonly` has now.  However, =
this would 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 expect to be extended.  (No existing code will break, but you =
could now do 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 =
visibilities =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 `readonly` and an explicit private(set) behave differently (one is =
final, one is not).  It also unclear if a `readonly` property can be =
overridden with `readonly protected(set)` only, or also `readonly =
private(set)`.  If 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 =
questions as option 3.
>=20
> 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 =
the future as its own RFC as that is technically a behavior change, not =
just addition, so deserves careful consideration.  However, if there is =
a clear consensus to go with option 2 now, we're open to that.
>=20
> --Larry Garfield

Hi Larry and Ilija,

Thanks for your work. Here is my opinion:

First, I do think that `readonly` should integrate with aviz, unless =
that implies truly controversial changes on `readonly`. As Theodore =
Brown commented in the previous version of the RFC: =E2=80=9CProposal =
feels unfinished since it can't be used in conjunction with readonly =
properties/classes. In my opinion the issues with this need to be =
resolved first, to avoid the language moving towards a messy hodgepodge =
of features that don't work well together.=E2=80=9D

Second, I think that making `readonly` implicitly `protected(set)` by =
default (Option 2) is the way to go:
* At first glance it is an expectation change. But, in reality, all =
readonly properties can *already* be written to from a child class as of =
today: it suffices that the child class in question redeclare those =
properties: https://3v4l.org/9AV4r. =46rom the point of view of the =
child class, the only thing that will change, is that it will no longer =
be required to explicitly opt into that possibility by redeclaring the =
readonly properties. =46rom the point of view of the parent class, =
nothing will change, except false expectations=E2=80=94and it is a good =
thing that false expectations are eliminated.
* Relatively of Options 3 and 4, Option 2 leaves the language in a more =
simple and regular state.

=E2=80=94Claude



--Apple-Mail=_65A2C20C-881C-4BE5-A0B1-B1FF463D2536
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
	charset=utf-8

<html><head><meta http-equiv=3D"content-type" content=3D"text/html; =
charset=3Dutf-8"></head><body style=3D"overflow-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;"><br =
id=3D"lineBreakAtBeginningOfMessage"><div><br><blockquote =
type=3D"cite"><div>Le 5 juin 2024 =C3=A0 16:28, Larry Garfield =
&lt;larry@garfieldtech.com&gt; a =C3=A9crit :</div><br =
class=3D"Apple-interchange-newline"><div><div>On Fri, May 31, 2024, at =
8:59 PM, Larry Garfield wrote:<br><blockquote type=3D"cite">On Fri, May =
31, 2024, at 5:45 PM, Claude Pache wrote:<br><blockquote =
type=3D"cite"><blockquote type=3D"cite">Le 31 mai 2024 =C3=A0 18:08, =
Larry Garfield &lt;larry@garfieldtech.com&gt; a =C3=A9crit =
:<br><br>However, this also brings up another interesting issue: =
readonly properties (in 8.3) DO allow redeclaration, essentially =
adjusting the property 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 is different from how inheritance works =
elsewhere. &nbsp;When the parent writes to the same property, a special =
check is needed to verify the two properties are related. &nbsp;All that =
special casing effectively means that readonly in 8.4 wouldn't really be =
"write once + private(set)", but "write once + private(set) - final", =
which is... just kinda screwy. &nbsp;That means our options =
are:<br><br>* A BC break on readonly (not allowing it to be =
overridden)<br>* Make readonly an exception to the implicit final.<br>* =
Just don't allow readonly with aviz after =
all.<br></blockquote><br>Another possible option is:<br><br>* Make =
readonly be `protected(set)` by default.<br><br>That would weaken the =
originally intended semantics of readonly, but in <br>a compatible and =
acceptable way?<br><br>=E2=80=94Claude<br></blockquote><br>Only sort of =
compatible. &nbsp;It would allow readonly props to be <br>redefined, and =
wouldn't break any existing code, I think... but it <br>would mean any =
time you use readonly, suddenly a child class can come <br>along and =
mess with it out from under you.<br><br>In practice that's likely not a =
common concern, but it is a behavior <br>change. &nbsp;I think it's =
possible (I need to confirm with Ilija), if we <br>want that slight BC =
break, but I don't know if we do.<br><br>--Larry =
Garfield<br></blockquote><br>Ilija and I have been discussing this issue =
over the last few days. &nbsp;We agree that `private(set)` should imply =
`final`, as that eliminates a bunch of issues both implementation-wise =
and expectation-wise. &nbsp;However, that causes an issue for =
`readonly`. &nbsp;<br><br>The root issue is that if we say "`readonly =
int $x` is really just `private(set) readonly int $x`", that runs into =
the issue of "whelp, you've just made readonly always final, which is a =
BC break." &nbsp;So that's no good.<br><br>We see a couple of ways to =
resolve this, presented below in our order of preference.<br><br>1. =
Disallow readonly with aviz =3D&gt; No BC break, and we don't need to =
define readonly in terms of private(set). &nbsp;The only really useful =
combination anyway would be `readonly protected(set)`, in which case the =
protected(set) is doing 90% of the work already. &nbsp;There's few cases =
where the readonly is truly necessary at that point. &nbsp;Any other =
oddball edge cases could be handled by a custom hook.<br>2. Make =
`readonly` implicitly `protected(set)` unless you explicitly specify =
`private(set)` =3D&gt; Would have the most consistent result, and this =
is probably the cleanest in the engine, as `readonly private(set)` would =
mean exactly what it says on the tin, with no inconsistency of "well =
it's kinda sorta `private(set)`" as `readonly` has now. &nbsp;However, =
this would be an expectation change, as suddenly all readonly properties =
could be written to from a child class. &nbsp;That may be good in some =
cases but it's possible some objects could have unexpected behavior if =
they didn't expect to be extended. &nbsp;(No existing code will break, =
but you could now do things to it in a child class that the author =
didn't anticipate.)<br>3. You can't mix `readonly` with `private(set)`, =
but can use other visibilities =3D&gt; No BC break, and we don't need to =
define readonly in terms of `private(set)`. &nbsp;However, it means the =
implicit `private(set)` of `readonly` and an explicit private(set) =
behave differently (one is final, one is not). &nbsp;It also unclear if =
a `readonly` property can be overridden with `readonly protected(set)` =
only, or also `readonly private(set)`. &nbsp;If the latter, does it =
become implicitly `final` at that point?<br>4. `readonly` behaves =
differently for an explicit (final) and implicit (not-final) =
`private(set)` =3D&gt; No BC break, but it's kinda weird and non-obvious =
to explain. &nbsp;It also has the same non-obvious inheritance questions =
as option 3.<br><br>We consider only the first two to be really viable. =
&nbsp;For simplicity, we'd favor doing option 1, and if desired option 2 =
could be presented in the future as its own RFC as that is technically a =
behavior change, not just addition, so deserves careful consideration. =
&nbsp;However, if there is a clear consensus to go with option 2 now, =
we're open to that.<br><br>--Larry =
Garfield<br></div></div></blockquote><br></div><div>Hi Larry and =
Ilija,</div><div><br></div><div>Thanks for your work. Here is my =
opinion:</div><div><br></div><div>First, I do think that `readonly` =
should integrate with aviz, unless that implies truly controversial =
changes on `readonly`. As Theodore Brown commented in the previous =
version of the RFC: =E2=80=9CProposal feels unfinished since it can't be =
used in conjunction with readonly properties/classes. In my opinion the =
issues with this need to be resolved first, to avoid the language moving =
towards a messy hodgepodge of features that don't work well =
together.=E2=80=9D</div><div><br></div><div>Second, I think that making =
`readonly` implicitly `protected(set)` by default (Option 2) is the way =
to go:</div><div>* At first glance it is an expectation change. But, in =
reality, all readonly properties can *already* be written to from a =
child class as of today: it suffices that the child class in question =
redeclare those properties:&nbsp;<a =
href=3D"https://3v4l.org/9AV4r">https://3v4l.org/9AV4r</a>. =46rom the =
point of view of the child class, the only thing that will change, is =
that it will no longer be required to explicitly opt into that =
possibility by redeclaring the readonly properties. =46rom the point of =
view of the parent class, nothing will change, except false =
expectations=E2=80=94and it is a good thing that false expectations are =
eliminated.</div><div>* Relatively of Options 3 and 4, Option 2 leaves =
the language in a more simple and regular =
state.</div><div><br></div><div>=E2=80=94Claude</div><div><br></div><div><=
br></div></body></html>=

--Apple-Mail=_65A2C20C-881C-4BE5-A0B1-B1FF463D2536--