On Wed, Nov 30, 2022, at 7:38 PM, Stephen Reay wrote: >>> So please, can you explain to me why consistency with an implementation >>> detail of readonly properties is more important than consistency with >>> declared developer intention for regular properties via the magic >>> setter method? >> >> There's a couple of reasons. >> >> One, and arguably the most important, readonly and aviz being incompatible >> is, hopefully, a temporary situation. There's some fiddly bits to work out >> design-wise, and based on earlier comments in the thread we're going to punt >> on that for now to avoid that dragging down the whole RFC. I believe we >> absolutely should allow them together in the future (maybe in a later 8.3 >> RFC, maybe a future version, TBD), which means ensuring, now, that they are >> compatible in the future. This approach involves the fewest future BC >> breaks. >> >> Second, I wouldn't call the current behavior of readonly a mere >> implementation detail. It's weird and unexpected, I'd agree, but only as a >> side effect of previous design decisions, some of which are even older than >> readonly. But it's an observed behavior that code can rely on, and in some >> cases does. For example: >> >> https://peakd.com/hive-168588/@crell/php-tricks-lazy-public-readonly-properties >> >> The "unset a declared property to force it through __get once" is a trick >> that some ORMs use extensively. readonly just inherited that, leading to >> the current behavior: __set depends on the write/set visibility of the >> property and its settedness. This RFC doesn't change anything there. >> >> The alternative would be to have __set called always for a non-public-set >> property. However, that is a place for bugs, as you then can't not have a >> back-door way to publicly set a property even if it's declared private(set). >> (Or, you have to be very careful in your __set to avoid it.) That is both >> inconsistent with the language today, and error prone. >> >> Finally, we're planning to work in the near-future on property hooks (aka >> property accessors), which would allow per-property custom set routines. >> That would largely remove the issue entirely, as the use of __set would go >> way down and you'd basically never have to use it with a declared property, >> fancy tricks or no, so this issue would never come up at all. >> >> --Larry Garfield >> >> -- >> PHP Internals - PHP Runtime Development Mailing List >> To unsubscribe, visit: https://www.php.net/unsub.php > > Hi Larry, > > > I think there must be some confusion somewhere, based on some of your > comments. > > I’m not suggesting that the “unset to force a **public** property to go > through getter/setter methods” logic should be specifically different. > > I’m suggesting that when the decision is made to call __set or not, the > properties **set** visibility is what should be considered, not it’s > **get** visibility. > > Your own comment even describes this behaviour: "leading to the current > behavior: __set depends on the write/set visibility of the property" > > But your RFC says that __set will depend on the **read/get** visibility > of the property. > > >> you then can't not have a back-door way to publicly set a property even if >> it's declared private(set). (Or, you have to be very careful in your __set >> to avoid it.) That is both inconsistent with the language today, and error >> prone. > > If a developer adds a _set() method that can write to a private(set) > property, I would expect that is working exactly as desired, exactly as > it does **now** where it’s just a “protected” property.
I think we're talking past each other a bit. :-) The logic described in the RFC is, to the best of out knowledge, what already happens in 8.1/8.2 with readonly. Whether that is good or bad, obvious or intuitive, it's what PHP already does, for better or worse. We view readonly as, effectively, a shorthand for "private(set) write-once" (which is what it is), and want to ensure that future RFCs can do that explicitly so that we can allow `public protected(set) readonly` in the future. For that to be possible, not changing the existing behavior is a necessity. Changing the behavior described in the RFC right now is a BC break. That's true whether the logic described in the RFC is good or not, because that's how it already works with `readonly`. Is it weird that __set depends in part on read visibility? Kinda, yeah. But that's already the behavior in 8.1/8.2. We're not changing anything, and this RFC is not the place to break that kind of BC. If someone can demonstrate that the logic described there is not what actually happens now, please let us know, because the goal is to not change any behavior in this regard. Effectively, that whole section is descriptive of PHP today and a comment "and we're not breaking it." --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
