Re: [PHP-DEV] Re: Issues with readonly classes
Hi, > If my default assumption (and Larry's) was that such a class would be immutable, it's fair to think that a non-trivial number of other programmers may make the same faulty assumption, and making this distinction obvious in the documentation would probably help. Yes, it makes sense to mention this in the documentation, but fortunately, the manual already makes the behavior clear at https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.readonly-properties : "However, readonly properties do not preclude interior mutability. Objects (or resources) stored in readonly properties may still be modified internally:" Following our discussion, Nicolas and I announced our proposal to fix two of the above mentioned issues: https://externals.io/message/119007 Feedback is welcome! Regards: Máté Jordan LeDoux ezt írta (időpont: 2022. szept. 25., V, 23:01): > > > On Sun, Sep 25, 2022 at 10:57 AM Máté Kocsis > wrote: > >> Hi, >> >> I agree with Tim, and I also think that both reverting and making any last >> minute fundamental change is not a good idea, especially >> because people don't have a clear agreement about how inheritance >> should work. Readonly classes is an optional feature, so anyone >> who wants proxies to work can simply not add the readonly flag to their >> classes. Of course, it's a bit more tricky for library code, >> but package authors should be aware of this gotcha. Having that said, I'll >> try my best to fix the current shortcomings for PHP 7.3. >> >> Regards, >> Máté >> > > I tried hard to make it clear that I don't think this makes it "broken", > it was just a deviation from my expectations and memory, both of which can > obviously be flawed. I was mostly looking for some kind of information > about what the reasoning was for this, given that I obviously (since I > didn't remember it) missed that part of the discussion. The distinction > between "readonly" and "immutable" is subtle, but fair. This distinction > should probably be pointed out quite explicitly in the documentation > though. > > If my default assumption (and Larry's) was that such a class would be > immutable, it's fair to think that a non-trivial number of other > programmers may make the same faulty assumption, and making this > distinction obvious in the documentation would probably help. > > Jordan >
[PHP-DEV] [RFC] [Discussion] Readonly class amendments
Hi Everyone, Following Nicolas' thread about "Issues with readonly classes" ( https://externals.io/message/118554), we created an RFC to fix two issues with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments Please let us know your thoughts! Máté
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
>> >> 1. Relax the set-is-tighter restriction. That would allow `protected >> protected(set)` etc. on any property. It wouldn't be particularly useful >> unless readonly is being used, but it would be syntactically legal and >> behave as you'd expect. We could still disallow "set is more permissive" >> combinations (eg, `private public(set)`), as those have no apparent use case. > > I think that Option 1 is the most reasonable. The meaning of `public > public(set)` and `protected protected(set)` is straightforward; from a user > point-of-view, disallowing them seems more like “it is useless” than “it is > confusing”. Unless there are good technical reason to restrict it, we can > just leave to linting tools the care of reporting it. > > —Claude > To clarify my position: * The set visibility must be either more restrictive or of the same restriction level than the get visibility. * When the set visibility is absent, it is inferred as following: * If `readonly` is present, the set visibility is `private` (as of today); * otherwise, the set visibility is the same as the get visibility (again, as of today). * We don’t judge whether it is reasonable to write `protected protected(set) string $foo;` when you could just write `protected string $foo` for the same effect. Similarly, we don’t judge whether it is reasonable to write `public function()` when you could just write `function()` for the same effect. We leave it to coding styles and linters to decide whether the short form or the long form is preferred. —Claude -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
On Sun, Nov 13, 2022, at 4:08 PM, Deleu wrote:
>>
>>
>> This is untrue. You can declare a private property identically in a
>> parent and child class, even readonly. I'm doing this now in a project.
>> It works, but would be unnecessary if the parent's property were
>> protected(set).
>>
>> --Larry Garfield
>>
>
> I do understand the slight complexity here (perhaps stronger in terms of
> implementation), but my assumption was that if a property is private and
> "the set visibility must be tighter", PHP would never assume `private
> protected(set) readonly`. In other words, if `private readonly` then
> `private private(set) readonly`, therefore nothing changes. You can keep
> the use of disjointed variables in inheritance provided they're all
> private. The only concern is when a property is declared protected, which
> then evaluates to what I mentioned on the previous email.
I don't follow. Right now, this is legal:
class P {
protected readonly string $foo;
public useFoo() {
if ($this->foo == 'beep') { ... }
}
}
class C extends P {
protected readonly string $foo;
public function __construct(string $foo) {
$this->foo = $foo;
}
}
The redeclaration of $foo is necessary because of the implicit private-set on
readonly today. I have code that does this.
If we go with options 1 or 4, then that could be simplified to:
class P {
protected protected(set) readonly string $foo;
public useFoo() {
if ($this->foo == 'beep') { ... }
}
}
class C extends P {
public function __construct(string $foo) {
$this->foo = $foo;
}
}
This would not require "set is wider", just "set is the same".
If we changed readonly to be an implicit protected(set) instead of
private(set), that would also resolve that use case but I do not know if that
would cause other problems with code like the above.
> I'm also under the stated assumption that `public public(set) readonly` is
> not something needed to be supported.
I am not aware of any use cases for it, though if it technically becomes
possible as a side effect of other changes I don't object.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
Hi Derick > > As I understand it, you’re suggesting that a property declared as > > `public protected(set)` would never trigger __set(). > > I would think that that would be against our current practice, which is > easy enough to explain as "if the property isn't visible set, > then use __set()". I would argue that "protected(set)" marks a property > as invisible from a non-inherited class/method, and hence __set should > be called. What you're describing does not match the readonly behavior. https://3v4l.org/X76pV Readonly properties only call __set once they are explicitly unset. The same applies to __unset. I changed this in the asymmetric visibility implementation recently to match this behavior but the RFC has not been adjusted to reflect this yet. AFAIK this behavior is there to allow calls to __set for certain edge cases. While not very intuitive it makes sense to stay consistent. If this behavior is undesirable we should change it in both places. Ilija -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
On Mon, 14 Nov 2022, Stephen Reay wrote: > As I understand it, you’re suggesting that a property declared as > `public protected(set)` would never trigger __set(). I would think that that would be against our current practice, which is easy enough to explain as "if the property isn't visible set, then use __set()". I would argue that "protected(set)" marks a property as invisible from a non-inherited class/method, and hence __set should be called. cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
On Sun, 13 Nov 2022, Larry Garfield wrote: > There's two design decisions we've made at this point, both of which > we think are logical and reasonable: > > 1. If specified, the set visibility must be tighter than the get > visibility. So `protected protected(set)` and `protected public(set)` > are not permitted, for instance. > > 2. `readonly` is a "write once" flag that may be combined with > asymmetric visibility. If no set visibility is specified, `readoly` > implies `private(set)`, but a different set visibility may also be > provided. > > These are both reasonable rules. However, it creates a conflict. > Specifically, in the following cases: > > public public(set) readonly string $foo > > protected protected(set) readonly string $foo Just to see if I understand this correctly, the "protected(set)" overwrites the implicit "private(set)" that "readonly" confers? I think that alone is confusing. > These would be the only way to have a non-private-set readonly > property. While the first is in practice quite unlikely, the second > has valid use cases. (In particular, a base class that provides > properties expected to be set by a child constructor, and then used by > a method in the parent class.) However, it would not be allowed under > the rules above. Working around it would require specifying `public > protected(set) readonly...`, which means exposing a property that > likely should not be exposed. > > That creates an odd situation where readonly and asymmetric visibility > may only be combined "sometimes." That is not deesireable. The only > way to combine them in their current form is to allow `protected > protected(set)` only if readonly is in use, which is excessively > complicated both to implement and to explain/document/use. > > We see two possible ways to resolve this conflict: > > 1. Relax the set-is-tighter restriction. That would allow `protected > protected(set)` etc. on any property. It wouldn't be particularly > useful unless readonly is being used, but it would be syntactically > legal and behave as you'd expect. We could still disallow "set is > more permissive" combinations (eg, `private public(set)`), as those > have no apparent use case. > > 2. Disallow readonly and asymmetric visibility being combined, because > readonly already has a hard-coded implied asymmetric visibility. > This option removes some potential use cases (they would most likely > drop the readonly), but has the upside that it's easier to re-allow at > some point in the future. I am in favour of this option 2. As I already think "protected protected(set) readonly" is a confusing thing to see. If I hadn't read this email, I wouldn't have understood the interacting between the "protected(set)" and "readonly". cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly
> On 14 Nov 2022, at 03:08, Larry Garfield wrote: > > Hi folks. Ilija is nearly done with the implementation for asymmetric > visibility and flushing out edge cases, but we've run into one design > question we'd like feedback on. > > There's two design decisions we've made at this point, both of which we think > are logical and reasonable: > > 1. If specified, the set visibility must be tighter than the get visibility. > So `protected protected(set)` and `protected public(set)` are not permitted, > for instance. > > 2. `readonly` is a "write once" flag that may be combined with asymmetric > visibility. If no set visibility is specified, `readoly` implies > `private(set)`, but a different set visibility may also be provided. > > These are both reasonable rules. However, it creates a conflict. > Specifically, in the following cases: > > public public(set) readonly string $foo > > protected protected(set) readonly string $foo > > These would be the only way to have a non-private-set readonly property. > While the first is in practice quite unlikely, the second has valid use > cases. (In particular, a base class that provides properties expected to be > set by a child constructor, and then used by a method in the parent class.) > However, it would not be allowed under the rules above. Working around it > would require specifying `public protected(set) readonly...`, which means > exposing a property that likely should not be exposed. > > That creates an odd situation where readonly and asymmetric visibility may > only be combined "sometimes." That is not deesireable. The only way to > combine them in their current form is to allow `protected protected(set)` > only if readonly is in use, which is excessively complicated both to > implement and to explain/document/use. > > We see two possible ways to resolve this conflict: > > 1. Relax the set-is-tighter restriction. That would allow `protected > protected(set)` etc. on any property. It wouldn't be particularly useful > unless readonly is being used, but it would be syntactically legal and behave > as you'd expect. We could still disallow "set is more permissive" > combinations (eg, `private public(set)`), as those have no apparent use case. > > 2. Disallow readonly and asymmetric visibility being combined, because > readonly already has a hard-coded implied asymmetric visibility. This option > removes some potential use cases (they would most likely drop the readonly), > but has the upside that it's easier to re-allow at some point in the future. > > 3. Some other brilliant idea we've not thought of. > > > Both are viable approaches with pros and cons. We're split on which way to > go with this, so we throw it out to the group for feedback. Which approach > would you favor, or do you have some other brilliant idea to square this > circle? > > > -- > Larry Garfield > la...@garfieldtech.com > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > Hi Larry, I asked for some clarification / further thought about the magic getters/setters interaction of this RFC back when you announced it, and never heard much besides saying that you’re working on a consolidated response to the feedback, but I’ve not seen anything regarding what I’d asked about, and the RFC still seems to have the (IMO) confusing and inconsistent magic getters/setters interaction described. As I understand it, you’re suggesting that a property declared as `public protected(set)` would never trigger __set(). While I understand that this is meant to “keep consistency with readonly”, I would imagine people would want to use native asymmetric visibility on objects that currently have protected (or private) properties that are accessed via magic getters and setters. I think it would be a natural assumption that asymmetric visibility would allow the user to remove the often boilerplate __get method, while maintaining the ability to have checks/other logic beyond simple type checks when a property is being set. Having the association of a *set* action tied to the *get* visibility is very unintuitive to me. The impact of this “same” limitation on readonly properties is IMO greatly reduced, because of the nature of something being readonly. The ability to run some logic when writing to a property that’s otherwise readable is surely a concept that’s very common and easy to understand, it’s literally the next example after (1) “readonly” and (2) “asymmetric visibility” in Nikita’s original property accessors RFC. The stated reason (on both this and the readonly RFC) for not allowing __set() is "to avoid surprising behaviour”. To me, having a field marked as `public protected(set)` then not call __set() when setting a property from outside the class **is** the surprising behaviour. Cheers Stephen
