Re: [RFC] Asymmetric Visibility, v2

From: Date: Wed, 05 Jun 2024 16:56:57 +0000
Subject: Re: [RFC] Asymmetric Visibility, v2
References: 1 2 3 4 5 6 7 8 9  Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message


> Le 5 juin 2024 à 16:28, Larry Garfield <[email protected]> a écrit :
> 
> 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 à 18:08, Larry Garfield <[email protected]> a écrit :
>>>> 
>>>> 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:
>>>> 
>>>> * 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 
>>> a compatible and acceptable way?
>>> 
>>> —Claude
>> 
>> Only sort of compatible.  It would allow readonly props to be 
>> redefined, and wouldn't break any existing code, I think... but it 
>> would mean any time you use readonly, suddenly a child class can come 
>> along and mess with it out from under you.
>> 
>> In practice that's likely not a common concern, but it is a behavior 
>> change.  I think it's possible (I need to confirm with Ilija), if we 
>> 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 bunch of issues both
> implementation-wise and expectation-wise.  However, that causes an issue for readonly. 
> 
> 
> 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.
> 
> We see a couple of ways to resolve this, presented below in our order of preference.
> 
> 1. Disallow readonly with aviz => 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) => 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 => 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) => 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.
> 
> 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.
> 
> --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: “Proposal 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.”

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. From 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. From the
point of view of the parent class, nothing will change, except false expectations—and 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.

—Claude




Thread (57 messages)

« previous php.internals (#123521) next »