Re: [RFC] Asymmetric Visibility, with readonly

> On 30 Nov 2022, at 15:41, Alexandru Pătrănescu <drealecs@gmail.com> wrote: > > > On Wed, Nov 30, 2022, 05:25 Stephen Reay <php-lists@koalephant.com > <mailto:php-lists@koalephant.com>> wrote: > > > > On 30 Nov 2022, at 08:27, Larry Garfield <larry@garfieldtech.com > > <mailto:larry@garfieldtech.com>> wrote: > > > > On Tue, Nov 29, 2022, at 5:46 PM, Claude Pache wrote: > > > >> In the RFC, section Permitted visibility > >> (https://wiki.php.net/rfc/asymmetric-visibility#permitted_visibility > >> <https://wiki.php.net/rfc/asymmetric-visibility#permitted_visibility> > >> > >> > >> <https://wiki.php.net/rfc/asymmetric-visibility#permitted_visibility > >> <https://wiki.php.net/rfc/asymmetric-visibility#permitted_visibility>>): > >>> The set visibility, if it differs from the main (get) visibility, MUST be strictly > >>> lesser than the main visibility. That is, the set visibility may only be protected or private. If > >>> the main visibility is protected, set visibility may only be private. Any violation of this rule > >>> will result in a compile time error. > >>> > >> The first sentence does not forbid public public(set), > >> or `protected > >> protected(set), etc. (the set` visibility does not > >> differ from the > >> main visibility), but the rest of the paragraph does not allow it. That > >> should be clarified. > > > > Er. That's exactly what it says: "strictly lesser" than the main > > visibility. The lines after are just restating it. "public public(set)" is not allowed. > > > > (We may relax that in the future to make it compatible with readonly, but that's for > > later.) > > > >> > >> (Because forbidding public public(set), etc., makes it > >> slightly more > >> cumbersome to explain the rules, I am slightly in favour not to forbid > >> it.) > >> > >>> There is one exception, that of a private readonly property. That would > >>> technically expand to private private(set) readonly, which is allowed. > >> > >> That sentence should be deleted, as readonly is now > >> forbidden. > > > > Good catch, fixed. Thanks. > > > > --Larry Garfield > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: https://www.php.net/unsub.php > > <https://www.php.net/unsub.php> > > > > Hi Larry, > > Thank you for clarifying the setter behaviour in more explicit terms, but I have to say I’m > quite disappointed in this continued “use the logic of readonly to apply to something that is > explicitly not readonly” - this is even more stark now that you’ve explicitly made them mutually > exclusive behaviours. > > I’m generally very in favour of maintaining consistency, but this seems like it’s using > technical consistency as an excuse to justify unintuitive behaviour that breaks consistency in > another, much more obvious way. > > > Can you explain why it makes more sense to maintain consistency with “readonly” than it > does to maintain consistency with the existing “__set()” behaviour for properties, particularly > now that you’ve indicated these features (asymmetric visibility and readonly) are mutually > exclusive? > > While it’s stated multiple times that “readonly” introduced a limited form of asymmetric > visibility, and thus this is a continuation, in terms of intuitiveness, the existing __set() rules > are very easy to comprehend even with readonly: > > - if the property is declared as public, __set() is never called; if it’s declared as > protected, __set is called when the property is accessed from outside that class or it’s > hierarchy. Yes, I know that readonly imposes an implicit visibility difference - but that is > essentially an implementation detail, from the point of view of the userland developer, it’s not a > clear statement of intended behaviour on their part, expressed through the code as written. > > For example, with public readonly int $foo it’s quite obvious > why __set() isn’t called, using the exiting well-understood logic: it’s a public property. PHP > applies a kind of asymmetric visibility to the property behind the scenes, but that isn’t what the > developer declared, it’s the implementation. This behaviour matches that of regular, non-readonly > fields: when the field is declared public (or has implicit public visibility) __set() is never > called. > > If we make that field protected, __set() will be called when the property is written to from > outside the class, regardless of whether it’s readonly or not. > > > What you’re proposing changes that, in a way that is completely unintuitive: when attempting > to *write* data to a property that is marked as protected(set), the __set() method will not be > called. > > > 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? > > > Hey, Stephen, Larry, > Just clarifying how I see it: > > Practically, the asymmetric visibility for properties is not more or less than asymmetric > write-access for properties. > That is what is implemented, that is what other languages have as well and this is what's > needed. > Of course, except for relation with __set(), unset() and other php specials, it wouldn't > make a difference. > > If you think it like this, that property is actually visible but only there is a restriction on > writing it, you would probably agree that __set() should not be called. > > And given that, for clarity, I would be happy if the word visibility would be changed in the > rfc with write-access in various places, including the rfc name. > But I don't think it's very important, as long as I've understood the behavior > and I can easily explain it going forward. > > Regards, > Alex > > > > > Cheers > > Stephen > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > <https://www.php.net/unsub.php> Hi Alex, No, I do not agree, and I don't think it’s a great sign that you’re suggesting we need to mentally swap out a significant part of what’s written in the RFC for some different concept to have it make sense. The first paragraph, and the initial section of the proposal make it quite clear that this RFC is about splitting the existing visibility concept into ‘get’ and ’set’ operations. I find it hard to believe that the authors of the RFC would manage to reference the existing visibility system so many times and in so many ways, if they weren’t explicitly talking about a change related to visibility. Cheers Stephen