> Le 5 juin 2024 à 16:28, Larry Garfield <la...@garfieldtech.com> 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 <la...@garfieldtech.com> 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
