Everything
php
[RFC] Asymmetric Visibility, with readonly
php.internals
Larry Garfield
3 years ago
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
larry@garfieldtech.com
Larry Garfield
3 years ago
On Sun, Nov 13, 2022, at 2:08 PM, 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?
Addendum: Ilija and I were talking past each other a bit, it seems. Allowing `protected protected(set) readonly` only in the context of readonly would be straightforward to implement. It's potentially not the easiest to explain that exception, but implementation-wise it's fine.
So that gives another viable option to consider:
4. Allow get and set visibility to be both set explicitly to the same level, iff in the presence of readonly. So `protected protected(set) string $foo` is not allowed, but `protected protected(set) readonly string $foo` is allowed. This gives the most options and the least pointless syntax, but at the cost of having an exception that needs to be documented/explained/understood.
--Larry Garfield
Deleu
3 years ago
>
>
> 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
>
What if the implicit rule for `readonly` is changed into `protected(set)`?
Let's run with this for a minute.
1- It's not truly a CODE breaking change. Right now if you have a
`protected readonly` property, you're either calling the parent constructor
or you're never making use of the variable, otherwise you're getting a
Fatal Error.
2- It's a CONCEPTUAL breaking change. If you've declared a `protected
readonly` property, you may expect anyone in the inheritance chain to be
calling your constructor. A new PHP version will warn you to either change
your property into `protected private(set) readonly` (Rector might be able
to do this) or accept the new behavior.
3- The two logical design choices can be kept.
To be honest, I think asymmetric visibility seems way over complex for very
little benefit, so I'm a bit against the entire feature, but if we were to
have them, it would really be nicer to not have crazy shenanigans involved.
--
Marco Deleu
Larry Garfield
3 years ago
On Sun, Nov 13, 2022, at 3:50 PM, Deleu wrote:
>>
>>
>> 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
>>
>
> What if the implicit rule for `readonly` is changed into `protected(set)`?
> Let's run with this for a minute.
>
> 1- It's not truly a CODE breaking change. Right now if you have a
> `protected readonly` property, you're either calling the parent constructor
> or you're never making use of the variable, otherwise you're getting a
> Fatal Error.
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
Deleu
3 years ago
>
>
> 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'm also under the stated assumption that `public public(set) readonly` is
not something needed to be supported.
--
Marco Deleu
Larry Garfield
3 years ago
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
Deleu
3 years ago
On Sun, Nov 13, 2022 at 5:09 PM Larry Garfield <larry@garfieldtech.com>
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:
>
To unsubscribe, visit: https://www.php.net/unsub.php
>
>
The more I think about this, the more it makes more sense (to me) to
simplify it even further;
1) The set visibility should be the same as the get visibility when one is
not provided. That is:
```
public $var -> public public(set) $var
protected $var -> protected protected(set) $var
private $var -> private private(set) $var
```
The explicit declaration of same visibility can be disallowed e.g. `public
public(set) $var` is not allowed, you must use `public $var` if you want
visibility symmetry. However, if it simplifies implementation to allow it,
why not? The behavior would already be in place anyway.
2) The readonly flag should no longer have an impact on the set visibility.
This is again a conceptual breaking change, but one that can even be fixed
with a SEARCH/REPLACE on a project scope. The "breaking change" is
categorized as follows:
The code `public readonly $var` used to be `public private(set) readonly
$var` should become `public public(set) readonly $var`
The code `protected readonly $var` used to be `protected private(set)
readonly $var` should become `protected protected(set) readonly $var`
The code `private readonly $var` should not have any change.
--
Marco Deleu
Claude Pache
3 years ago
>
> 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.
Both have use cases. Whether they are valid is quite subjective. (I am speaking as someone who suffer from the `match()` construct forbidding having both `default` and some other `case` pointing to the same expression. That restriction seemed reasonable the time the feature was designed and voted.)
>
> 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
Claude Pache
3 years ago
>>
>> 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
Tim Düsterhus
3 years ago
Hi
On 11/14/22 21:02, Claude Pache wrote:
> 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.
>
I agree with that.
--------------
As I'm sending an email anyway: Larry, will there be a a new separate
discussion thread, once all the problems are resolved and once it's
clear what you propose? I just noticed the "Abbreviated Form" section in
the RFC
(https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form) which
I disagree with, which apparently was added in October, but I remember
an email letting readers know of the updated RFC. I didn't follow the
evolution of the RFC too closely, though, because I believed that it
still was in a somewhat early stage and because discussion is already
split into way-to-many threads and also the poll.
Best regards
Tim Düsterhus
Larry Garfield
3 years ago
On Tue, Nov 22, 2022, at 12:08 PM, Tim Düsterhus wrote:
> Hi
>
> On 11/14/22 21:02, Claude Pache wrote:
>> 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.
>>
>
> I agree with that.
>
> --------------
>
> As I'm sending an email anyway: Larry, will there be a a new separate
> discussion thread, once all the problems are resolved and once it's
> clear what you propose? I just noticed the "Abbreviated Form" section in
> the RFC
> (https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form) which
> I disagree with, which apparently was added in October, but I remember
> an email letting readers know of the updated RFC. I didn't follow the
> evolution of the RFC too closely, though, because I believed that it
> still was in a somewhat early stage and because discussion is already
> split into way-to-many threads and also the poll.
>
> Best regards
> Tim Düsterhus
Yes, once we make a decision on this point, I'll respond in this thread.
The abbreviated form was part of the discussion earlier, and the syntax we settled on supported it, so it made sense to include.
I try to highlight on the list any time notable changes are made, even if I don't always have a detailed changelog. Whether it makes sense to use the same thread or a new thread varies, and I'm absolutely certain others will make a different judgement call on that than I do. I'm not aware of any standard convention around that, so until one exists I'll just continue making vaguely educated guesses on that front.
Given the timing, Ilija says he'd prefer to not call the vote until after the new year/holidays. Feedback still welcome at this point but aside from edge cases like this thread's, I think we're pretty well set on the basic features. Baring any major change of direction expect the vote to be called in early January, probably. Time permitting we're going to also start work on property hooks/accessors, which is the intended follow-on to this RFC.
--Larry Garfield
Stephen Reay
3 years ago
> On 14 Nov 2022, at 03:08, Larry Garfield <larry@garfieldtech.com> 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
> larry@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
Derick Rethans
3 years ago
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
Ilija Tovilo
3 years ago
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
Derick Rethans
3 years ago
On Mon, 14 Nov 2022, Ilija Tovilo wrote:
> 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.
Now I have re-read the Assymetric Visibility RFC's "Interaction with
__set"
(https://wiki.php.net/rfc/asymmetric-visibility#interaction_with_set),
I'm slightly more confused. Could you update that section, as what it
currently states seems reasonable.
cheers,
Derick
Derick Rethans
3 years ago
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
Dan Ackroyd
3 years ago
Hi Larry,
Regarding the syntax, up until now PHP has only supported the letters
a-z and underscore in keywords.
I realise this is an aesthetic thing, but "private(set)" looks like a
function to me, and not a keyword. I saw the previous poll, and it
didn't include options for either protected_set/private_set or
protectedset/privateset.
Was there a technical reason for excluding them as an option?
On Sun, 13 Nov 2022 at 20:09, Larry Garfield <larry@garfieldtech.com> wrote:
>
> That creates an odd situation where readonly and asymmetric visibility
> may only be combined "sometimes."...
>
> 2. Disallow readonly and asymmetric visibility being combined, because
> readonly already has a hard-coded implied asymmetric visibility.
This is getting quite complicated.
I think unless someone makes a strong case for allowing the
combination of the two, disallowing them being combined is probably
the best choice.
cheers
Dan
Ack
Rowan Collins
3 years ago
On 20/11/2022 13:20, Dan Ackroyd wrote:
> This is getting quite complicated.
>
> I think unless someone makes a strong case for allowing the
> combination of the two, disallowing them being combined is probably
> the best choice.
I'm inclined to agree. There's going to be a lot to understand and agree
on this RFC as it is, so if some edge cases can reasonably be defined as
forbidden, we're less likely to end up regretting some detail.
Then if a good use case is identified, there can be a follow-up RFC,
either within this release cycle, or in a future release.
Regards,
--
Rowan Tommins
[IMSoP]
Larry Garfield
3 years ago
On Sun, Nov 20, 2022, at 7:20 AM, Dan Ackroyd wrote:
> Hi Larry,
>
> Regarding the syntax, up until now PHP has only supported the letters
> a-z and underscore in keywords.
>
> I realise this is an aesthetic thing, but "private(set)" looks like a
> function to me, and not a keyword. I saw the previous poll, and it
> didn't include options for either protected_set/private_set or
> protectedset/privateset.
>
> Was there a technical reason for excluding them as an option?
Not in particular. We originally proposed `private(set)` because that's what Swift uses. In the discussion people proposed several alternatives that I put into the poll, but `private_set` wasn't one of them. Although several people said they were opposed to parens in the keyword, the two options that had parens (`private(set)` and `pubilc(set: private)`) were the leaders for all rounds. I was quite surprised at that, personally; I expected `private:set` to do a lot better under the circumstances. But that's not how the poll played out.
--Larry Garfield
Dan Ackroyd
3 years ago
On Fri, 25 Nov 2022 at 00:07, Larry Garfield <larry@garfieldtech.com> wrote:
>
> On Sun, Nov 20, 2022, at 7:20 AM, Dan Ackroyd wrote:
> > Hi Larry,
> >
> > Regarding the syntax, up until now PHP has only supported the letters
> > a-z and underscore in keywords.
> >
> > I realise this is an aesthetic thing, but "private(set)" looks like a
> > function to me, and not a keyword. I saw the previous poll, and it
> > didn't include options for either protected_set/private_set or
> > protectedset/privateset.
> >
> > Was there a technical reason for excluding them as an option?
>
> Not in particular. We originally proposed `private(set)` because that's what Swift uses. In the discussion people proposed several alternatives that I put into the poll, but `private_set` wasn't one of them. Although several people said they were opposed to parens in the keyword,
Have you considered updating the RFC to use private_set, and not
introduce new characters for keywords?
As it wasn't in the poll, then sticking to the poll result with at
least some people saying they are opposed to it seems not necessarily
the correct choice. The ()'s look particularly funky to me with
property promotion.
cheers
Dan
Ack
Larry Garfield
3 years ago
On Sun, Nov 13, 2022, at 2:08 PM, 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?
Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
Thanks all.
--Larry Garfield
Claude Pache
3 years ago
> Le 29 nov. 2022 à 21:29, Larry Garfield <larry@garfieldtech.com> a écrit :
>
>
> Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
>
> 1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
>
> 2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
>
> The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
>
> Thanks all.
>
> --Larry Garfield
>
Hi,
In the RFC, section 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.
(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.
—Claude
Larry Garfield
3 years ago
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>):
>> 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
Stephen Reay
3 years ago
> On 30 Nov 2022, at 08:27, Larry Garfield <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>):
>>> 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
>
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?
Cheers
Stephen
Alexandru Pătrănescu
3 years ago
On Wed, Nov 30, 2022, 05:25 Stephen Reay <php-lists@koalephant.com> wrote:
>
>
> > On 30 Nov 2022, at 08:27, Larry Garfield <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>):
> >>> 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
> >
>
> 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
Stephen Reay
3 years ago
> 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
Larry Garfield
3 years ago
On Tue, Nov 29, 2022, at 11:25 PM, Stephen Reay wrote:
> 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?
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
Stephen Reay
3 years ago
> On 30 Nov 2022, at 22:09, Larry Garfield <larry@garfieldtech.com> wrote:
>
> On Tue, Nov 29, 2022, at 11:25 PM, Stephen Reay wrote:
>
>> 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?
>
> 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.
> we're planning to work in the near-future on property hooks
That’s great, but I don’t think weird unintuitive behaviour should be added because “well we’re gonna try and solve this through something else later”
Cheers
Stephen
Stephen Reay
3 years ago
> On 1 Dec 2022, at 08:38, Stephen Reay <php-lists@koalephant.com> wrote:
>
>
>
>> On 30 Nov 2022, at 22:09, Larry Garfield <larry@garfieldtech.com> wrote:
>>
>> On Tue, Nov 29, 2022, at 11:25 PM, Stephen Reay wrote:
>>
>>> 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?
>>
>> 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.
>
>
>> we're planning to work in the near-future on property hooks
>
> That’s great, but I don’t think weird unintuitive behaviour should be added because “well we’re gonna try and solve this through something else later”
>
>
>
> Cheers
>
> Stephen
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
Apologies, I meant “where it’s just a private property” rather than “just a protected property” in the second to last sentence.
Larry Garfield
3 years ago
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
Stephen Reay
3 years ago
> On 2 Dec 2022, at 01:21, Larry Garfield <larry@garfieldtech.com> wrote:
>
> 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
Hi Larry,
Given that you agree the current behaviour is “a bit weird” why not keep the current limitation tied specifically to the readonly keyword then?
I can actually see a scenario (an “edge case” as mentioned as the reasoning for the limitation in the readonly rfc) where weird behaviour could be observed with readonly - logic performed in the __set method that assumes the write/set below will work.
The same isn’t true for non-readonly. I believe all or most edge-cases that the limitation is meant to prevent would likely be related specifically to a field being readonly, not to it being implicitly asymmetrically visible.
So here’s my last attempt:
Please change this behaviour in your rfc.
You are explicitly making it mutually exclusive with readonly now, so that’s not a bc break - if/when it becomes compatible with readonly the authors of that rfc can either keep the limitation as it exists **for readonly properties** (I’m not an expert but I don’t believe this would be a BC break either), or decide to drop the limitation completely (deliberate BC break)
Keeping the limitation now on non-readonly properties makes no sense, and will be confusing to user land developers.
Cheers
Stephen
Ilija Tovilo
3 years ago
Hi Stephen
> So here’s my last attempt:
>
> Please change this behaviour in your rfc.
>
> You are explicitly making it mutually exclusive with readonly now, so that’s not a bc break - if/when it becomes compatible with readonly the authors of that rfc can either keep the limitation as it exists **for readonly properties** (I’m not an expert but I don’t believe this would be a BC break either), or decide to drop the limitation completely (deliberate BC break)
>
> Keeping the limitation now on non-readonly properties makes no sense, and will be confusing to user land developers.
This behavior is already not limited to readonly, it works the same
for normal typed properties.
https://3v4l.org/WpBp4
The set-visibility is not relevant here: When unset has been called,
__set will be called (and __get too). The set-visibility is only
relevant when unset hasn't been called, as that will influence whether
the property is written to or an error is thrown. The RFC is actually
wrong saying this is specifically due to readonly properties. This
behavior has been here for a while.
See the relevant commit:
https://github.com/php/php-src/commit/f1848a4b3f807d21415c5a334b461d240b2a83af
> Assigning to an uninitialized typed property will no longer trigger
> a call to __set(). However, calls to __set() are still triggered if
> the property is explicitly unset().
> This gives us both the behavior people generally expect, and still
> allows ORMs to do lazy initialization by unsetting properties.
> For PHP 8, we should fine a way to forbid unsetting of declared
> properties entirely, and provide a different way to achieve lazy
> initialization.
I'm not sure if anybody actually makes use of this trick. It might be
worth removing this weird behavior by offering an alternative. In any
case, this is out of scope for this RFC.
Regards,
Ilija
Claude Pache
3 years ago
> Le 30 nov. 2022 à 02:27, Larry Garfield <larry@garfieldtech.com> a écrit :
>
> 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>):
>>> 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.
As I understand the first sentence (what it says, not what you meant):
“The set visibility, **if it differs from the main (get) visibility**, {$some_restriction}.”
In `public public(set)`, the set visibility does not differ from the main/get visibility, therefore {$some_restriction} does not apply.
My guess is that you wanted to say:
“The set visibility, **if specified explicitly**, {$some_restriction}.”
—Claude
Larry Garfield
3 years ago
On Wed, Nov 30, 2022, at 4:02 AM, Claude Pache wrote:
>> Le 30 nov. 2022 à 02:27, Larry Garfield <larry@garfieldtech.com> a écrit :
>>
>> 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>):
>>>> 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.
>
> As I understand the first sentence (what it says, not what you meant):
>
> “The set visibility, **if it differs from the main (get) visibility**,
> {$some_restriction}.”
>
> In `public public(set)`, the set visibility does not differ from the
> main/get visibility, therefore {$some_restriction} does not apply.
>
> My guess is that you wanted to say:
>
> “The set visibility, **if specified explicitly**, {$some_restriction}.”
Ahhh... I see what you're getting at. Fair point, I'll adjust it. Thanks.
--Larry Garfield
Larry Garfield
3 years ago
On Tue, Nov 29, 2022, at 2:29 PM, Larry Garfield wrote:
> On Sun, Nov 13, 2022, at 2:08 PM, 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?
>
>
> Thank you everyone for the feedback. Based on this thread, we've made
> two changes to the RFC:
>
> 1. We've moved readonly back to forbidden with a-viz for now. I've
> added a section to Future Scope where we really should sort this out in
> the future, but we'll do that in the future when we can all focus on
> the various nuances of just that piece.
>
> 2. I rewrote the section on __set to make it clearer. That also
> included Ilija and I digging into all the nuances that are already
> present. The text may still look a bit complex, but that's because the
> existing logic is already complex with readonly. Long story short, the
> a-viz RFC does not change anything in the way __set works vis a vis
> asymmetric visibility; it just inherits and continues what readonly
> already started, so it's consistent.
>
> The PR should be updated in the next week or two with the latest
> changes. Baring any major need for change, we expect to call a vote
> for it shortly after New Years.
>
> Thanks all.
>
> --Larry Garfield
One other update: Nicolas poked me off list to remind me that the RFC didn't mention inheritance at all. Oops. :-) The patch already has inheritance well handled and tested, it just wasn't described in the RFC. I have added a section that documents the behavior. Short version: You can continue to expand, but not contract, the visibility in child classes, but you can do it for get and set separately. It's pretty much exactly what you'd expect it to do.
--Larry Garfield
Claude Pache
3 years ago
Hi,
What is the behaviour of the following code?
```php
class Foo {
public private(set) array $bar = [ ];
}
$foo = new Foo;
$foo->bar['x'] = 'y'; // error?
var_dump(isset($foo->bar['x'])); // true?, false?
```
I think that modification of an array should require write access? (That should be clarified in the RFC.)
—Claude
Larry Garfield
3 years ago
On Wed, Nov 30, 2022, at 12:10 PM, Claude Pache wrote:
> Hi,
>
> What is the behaviour of the following code?
>
> ```php
> class Foo {
> public private(set) array $bar = [ ];
> }
>
> $foo = new Foo;
>
> $foo->bar['x'] = 'y'; // error?
>
> var_dump(isset($foo->bar['x'])); // true?, false?
> ```
>
> I think that modification of an array should require write access?
> (That should be clarified in the RFC.)
Correct, that would not be allowed. Array writes are always indirect modification, because it's two steps to get to it. That will error out in this case, as one would expect from `private(set)`.
--Larry Garfield
Tim Düsterhus
3 years ago
Hi
On 11/29/22 21:29, Larry Garfield wrote:
> Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
>
> 1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
>
> 2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
>
> The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
>
Okay, then I'd like to officially "request" that the abbreviated form
[1] is dropped:
I believe 'protected(set) string $foo' is easily confused with
'protected string $foo' at a simple glance.
Also any implicit rules are something developers will need to learn by
heart, whereas an explicit 'public protected(set) string $foo' could
reasonably be understood by someone without any PHP experience and some
basic experience of OO concepts.
Having two separate explicit keywords also makes it much clearer that
asymmetric visibility is involved, because it's also asymmetric in the code.
I believe the only benefit of the abbreviated form is saving 6
keystrokes (+ one hit to the spacebar) and I don't believe it's worth
the lack of clarity for an important property of the defined property.
Best regards
Tim Düsterhus
[1] https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form
Larry Garfield
3 years ago
On Thu, Dec 1, 2022, at 12:31 PM, Tim Düsterhus wrote:
> Hi
>
> On 11/29/22 21:29, Larry Garfield wrote:
>> Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
>>
>> 1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
>>
>> 2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
>>
>> The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
>>
>
> Okay, then I'd like to officially "request" that the abbreviated form
> [1] is dropped:
>
> I believe 'protected(set) string $foo' is easily confused with
> 'protected string $foo' at a simple glance.
>
> Also any implicit rules are something developers will need to learn by
> heart, whereas an explicit 'public protected(set) string $foo' could
> reasonably be understood by someone without any PHP experience and some
> basic experience of OO concepts.
>
> Having two separate explicit keywords also makes it much clearer that
> asymmetric visibility is involved, because it's also asymmetric in the code.
>
> I believe the only benefit of the abbreviated form is saving 6
> keystrokes (+ one hit to the spacebar) and I don't believe it's worth
> the lack of clarity for an important property of the defined property.
>
> Best regards
> Tim Düsterhus
>
> [1] https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form
Does anyone else have feelings on this point? IMO, the shorthand makes a lot of sense when used with readonly to avoid lines getting just annoyingly long, but without it I can see the argument for not allowing it; it's about a wash in terms of length with readonly today. I'm comfortable going with the consensus on this one for now.
--Larry Garfield
Stephen Reay
3 years ago
> On 12 Dec 2022, at 02:18, Larry Garfield <larry@garfieldtech.com> wrote:
>
> On Thu, Dec 1, 2022, at 12:31 PM, Tim Düsterhus wrote:
>> Hi
>>
>> On 11/29/22 21:29, Larry Garfield wrote:
>>> Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
>>>
>>> 1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
>>>
>>> 2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
>>>
>>> The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
>>>
>>
>> Okay, then I'd like to officially "request" that the abbreviated form
>> [1] is dropped:
>>
>> I believe 'protected(set) string $foo' is easily confused with
>> 'protected string $foo' at a simple glance.
>>
>> Also any implicit rules are something developers will need to learn by
>> heart, whereas an explicit 'public protected(set) string $foo' could
>> reasonably be understood by someone without any PHP experience and some
>> basic experience of OO concepts.
>>
>> Having two separate explicit keywords also makes it much clearer that
>> asymmetric visibility is involved, because it's also asymmetric in the code.
>>
>> I believe the only benefit of the abbreviated form is saving 6
>> keystrokes (+ one hit to the spacebar) and I don't believe it's worth
>> the lack of clarity for an important property of the defined property.
>>
>> Best regards
>> Tim Düsterhus
>>
>> [1] https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form
>
>
> Does anyone else have feelings on this point? IMO, the shorthand makes a lot of sense when used with readonly to avoid lines getting just annoyingly long, but without it I can see the argument for not allowing it; it's about a wash in terms of length with readonly today. I'm comfortable going with the consensus on this one for now.
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
I’ll just re-iterate what I said in chat a while ago - I think requiring the dev to be explicit about making it public is a safer first step, and it has no BC risk if you wanted to later allow the implicit variant, and without the capability to combine with the readonly keyword (yet) it doesn’t seem like a huge problem to be explicit IMO.
Cheers
Stephen
Claude Pache
3 years ago
> Le 11 déc. 2022 à 20:18, Larry Garfield <larry@garfieldtech.com> a écrit :
>
> On Thu, Dec 1, 2022, at 12:31 PM, Tim Düsterhus wrote:
>> Hi
>>
>> On 11/29/22 21:29, Larry Garfield wrote:
>>> Thank you everyone for the feedback. Based on this thread, we've made two changes to the RFC:
>>>
>>> 1. We've moved readonly back to forbidden with a-viz for now. I've added a section to Future Scope where we really should sort this out in the future, but we'll do that in the future when we can all focus on the various nuances of just that piece.
>>>
>>> 2. I rewrote the section on __set to make it clearer. That also included Ilija and I digging into all the nuances that are already present. The text may still look a bit complex, but that's because the existing logic is already complex with readonly. Long story short, the a-viz RFC does not change anything in the way __set works vis a vis asymmetric visibility; it just inherits and continues what readonly already started, so it's consistent.
>>>
>>> The PR should be updated in the next week or two with the latest changes. Baring any major need for change, we expect to call a vote for it shortly after New Years.
>>>
>>
>> Okay, then I'd like to officially "request" that the abbreviated form
>> [1] is dropped:
>>
>> I believe 'protected(set) string $foo' is easily confused with
>> 'protected string $foo' at a simple glance.
>>
>> Also any implicit rules are something developers will need to learn by
>> heart, whereas an explicit 'public protected(set) string $foo' could
>> reasonably be understood by someone without any PHP experience and some
>> basic experience of OO concepts.
>>
>> Having two separate explicit keywords also makes it much clearer that
>> asymmetric visibility is involved, because it's also asymmetric in the code.
>>
>> I believe the only benefit of the abbreviated form is saving 6
>> keystrokes (+ one hit to the spacebar) and I don't believe it's worth
>> the lack of clarity for an important property of the defined property.
>>
>> Best regards
>> Tim Düsterhus
>>
>> [1] https://wiki.php.net/rfc/asymmetric-visibility#abbreviated_form
>
>
> Does anyone else have feelings on this point? IMO, the shorthand makes a lot of sense when used with readonly to avoid lines getting just annoyingly long, but without it I can see the argument for not allowing it; it's about a wash in terms of length with readonly today. I'm comfortable going with the consensus on this one for now.
>
> --Larry Garfield
Hi,
As of today, the following declarations are rejected as syntax errors:
``php
class C {
$a;
int $b;
}
```
while the following declarations are accepted with implicit `public` visibility:
``php
class C {
static $c;
readonly int $d;
}
```
It would be reasonable to propose to allow to consistently omit the `public` keyword. But allowing to omit it in some cases (including the most controversial one: `protected(set)`) and not in other cases...? Because of this inconsistency, people are incited to always write explicitly `public` anyway.
—Claude
Claude Pache
3 years ago
> Le 13 déc. 2022 à 16:34, Claude Pache <claude.pache@gmail.com> a écrit :
>
>
> Hi,
>
> As of today, the following declarations are rejected as syntax errors:
>
> ```php
> class C {
> $a;
> int $b;
> }
> ```
>
> while the following declarations are accepted with implicit `public` visibility:
>
> ```php
> class C {
> static $c;
> readonly int $d;
> }
> ```
>
> It would be reasonable to propose to allow to consistently omit the `public` keyword. But allowing to omit it in some cases (including the most controversial one: `protected(set)`) and not in other cases...? Because of this inconsistency, people are incited to always write explicitly `public` anyway.
>
> —Claude
However, I’m just realising that omitting `public` in declarations like `public $a` and `public int $b` is probably not a good idea, because it is incompatible with constructor property promotion, as `function __construct(public int $b) { }`, and `function __construct(int $b) { }` have different meanings.
—Claude
Larry Garfield
3 years ago
On Tue, Dec 13, 2022, at 11:03 AM, Claude Pache wrote:
>> Le 13 déc. 2022 à 16:34, Claude Pache <claude.pache@gmail.com> a écrit :
>>
>>
>> Hi,
>>
>> As of today, the following declarations are rejected as syntax errors:
>>
>> ```php
>> class C {
>> $a;
>> int $b;
>> }
>> ```
>>
>> while the following declarations are accepted with implicit `public` visibility:
>>
>> ```php
>> class C {
>> static $c;
>> readonly int $d;
>> }
>> ```
>>
>> It would be reasonable to propose to allow to consistently omit the `public` keyword. But allowing to omit it in some cases (including the most controversial one: `protected(set)`) and not in other cases...? Because of this inconsistency, people are incited to always write explicitly `public` anyway.
>>
>> —Claude
>
>
> However, I’m just realising that omitting `public` in declarations like
> `public $a` and `public int $b` is probably not a good idea, because it
> is incompatible with constructor property promotion, as `function
> __construct(public int $b) { }`, and `function __construct(int $b) { }`
> have different meanings.
>
> —Claude
Well, it seems the only people who have opinions on the abbreviated form at all dislike it, so we've removed it for now and left a mention in future-scope. A future RFC can add that if desired.
That should, I think, be the final change to the asymmetric visibility RFC. Baring anything else coming up, I expect to call the vote sometime the week of 2 January.
Happy $holiday!
--Larry Garfield
Larry Garfield
3 years ago
On Wed, Dec 21, 2022, at 3:23 PM, Larry Garfield wrote:
> On Tue, Dec 13, 2022, at 11:03 AM, Claude Pache wrote:
>>> Le 13 déc. 2022 à 16:34, Claude Pache <claude.pache@gmail.com> a écrit :
>>>
>>>
>>> Hi,
>>>
>>> As of today, the following declarations are rejected as syntax errors:
>>>
>>> ```php
>>> class C {
>>> $a;
>>> int $b;
>>> }
>>> ```
>>>
>>> while the following declarations are accepted with implicit `public` visibility:
>>>
>>> ```php
>>> class C {
>>> static $c;
>>> readonly int $d;
>>> }
>>> ```
>>>
>>> It would be reasonable to propose to allow to consistently omit the `public` keyword. But allowing to omit it in some cases (including the most controversial one: `protected(set)`) and not in other cases...? Because of this inconsistency, people are incited to always write explicitly `public` anyway.
>>>
>>> —Claude
>>
>>
>> However, I’m just realising that omitting `public` in declarations like
>> `public $a` and `public int $b` is probably not a good idea, because it
>> is incompatible with constructor property promotion, as `function
>> __construct(public int $b) { }`, and `function __construct(int $b) { }`
>> have different meanings.
>>
>> —Claude
>
> Well, it seems the only people who have opinions on the abbreviated
> form at all dislike it, so we've removed it for now and left a mention
> in future-scope. A future RFC can add that if desired.
>
> That should, I think, be the final change to the asymmetric visibility
> RFC. Baring anything else coming up, I expect to call the vote
> sometime the week of 2 January.
>
> Happy $holiday!
>
> --Larry Garfield
Happy New Year, PHP. Fair warning, I will be opening the vote for asymmetric visibility on Friday, baring any sudden new inputs in the next 40-ish hours.
--Larry Garfield
Robert Landers
3 years ago
One quick question,
Will changing the property of a protected/private set via reflection
be allowed? Or will you have to do some shenanigans like you currently
have to do with readonly?
Robert Landers
Software Engineer
Utrecht NL
On Wed, Jan 4, 2023 at 9:29 PM Larry Garfield <larry@garfieldtech.com> wrote:
Ilija Tovilo
3 years ago
Hi Robert
On Thu, Jan 5, 2023 at 3:54 PM Robert Landers <landers.robert@gmail.com> wrote:
>
> One quick question,
>
> Will changing the property of a protected/private set via reflection
> be allowed? Or will you have to do some shenanigans like you currently
> have to do with readonly?
Since PHP 8.1 ReflectionProperty::setValue() allows writing to any
property from any scope by default (assuming it is not readonly)
without calling ReflectionProperty::setAccessible() first.
https://wiki.php.net/rfc/make-reflection-setaccessible-no-op
Essentially, there's no change to the existing behavior. You can write
to an asymmetric property using setValue() just like you could to a
protected or private property as the visibility check is skipped for
these cases.
I added a sentence to the RFCs reflection section to clarify.
Thanks for pointing this out.
Ilija