As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
- The section on magic methods has been greatly simplified. The implementation itself hasn't changed, but the explanation is a lot less confusing now.
- We've explained how aviz interacts with hooks (they don't, really) and with interface properties (in the obvious way), which didn't exist at the time of the last draft.
- We've added a section with examples of how aviz is a concrete improvement, even in a world with readonly and hooks.
- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.com
Hi
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
The most common use case is that 'get' and 'set' are symmetric. Any
divergence from that should stand out and I think that the hamming
distance between
protected string $foo;
and
protected(set) string $foo;
is too small.
Other than that the proposal looks good to me. Ship it.
One note regarding the text. You already confirmed to me in private that:
class Foo {
private $dontTouchMe;
}
$backdoor = function ($key, $value) { $this->{$key} = $value; };
$f = new Foo();
$backdoor->call($f, 'dontTouchMe', 'butIDid');
var_dump($f);
would work as expected with aviz. It would make sense to explicitly
spell that out, just like it's explicitly spelled out that
ReflectionProperty::setValue()
works.
Best regards
Tim Düsterhus
Hi
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
The most common use case is that 'get' and 'set' are symmetric.
OK, fair, I meant in the most common case where you're using aviz at all, get is probably public.
That said, with both hooks and aviz, I can see data objects, for instance, becoming commonly public-get, private-set. It's already common to have them be public readonly, so this is just an extension of that.
Any
divergence from that should stand out and I think that the hamming
distance betweenprotected string $foo;
and
protected(set) string $foo;
is too small.
I can only respectfully disagree here. I think it's reasonably self-evident, made moreso by the (), which as Andreas noted looks weird when you're not used to it (but we should get used to fairly quickly). And the benefit of not having to type "public" on every property outweighs any initial confusion, much the same as readonly classes just reduces boilerplate.
One note regarding the text. You already confirmed to me in private that:
class Foo { private $dontTouchMe; } $backdoor = function ($key, $value) { $this->{$key} = $value; }; $f = new Foo(); $backdoor->call($f, 'dontTouchMe', 'butIDid'); var_dump($f);
would work as expected with aviz. It would make sense to explicitly
spell that out, just like it's explicitly spelled out that
ReflectionProperty::setValue()
works.
Added a note to that effect in the Reflection section as well. Thanks.
--Larry Garfield
Hello Larry,
just a quick thought.
Is there a reason why we cannot just make it "public private string
$x" instead of "public private(set) string $x"?
We would define that the second visibility specifier is for write.
The current proposal with "public private(set)" is less ambiguous, and
it is immediately obvious that this is something new, and not just
somebody accidentally added two modifiers.
At the same time, it feels a bit alien and cluttered.
Other options could be something like "public:private" or "public-private".
A consequence of such options would be that you always need to specify
the read visibility along with the write visibility.
But this seems ok to me.
This is not a final opinion, just a thought.
-- Andreas
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
- The section on magic methods has been greatly simplified. The implementation itself hasn't changed, but the explanation is a lot less confusing now.
- We've explained how aviz interacts with hooks (they don't, really) and with interface properties (in the obvious way), which didn't exist at the time of the last draft.
- We've added a section with examples of how aviz is a concrete improvement, even in a world with readonly and hooks.
- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.com
Hello Larry,
just a quick thought.
Is there a reason why we cannot just make it "public private string
$x" instead of "public private(set) string $x"?
We would define that the second visibility specifier is for write.The current proposal with "public private(set)" is less ambiguous, and
it is immediately obvious that this is something new, and not just
somebody accidentally added two modifiers.
At the same time, it feels a bit alien and cluttered.Other options could be something like "public:private" or "public-private".
A consequence of such options would be that you always need to specify
the read visibility along with the write visibility.
But this seems ok to me.This is not a final opinion, just a thought.
-- Andreas
We went through a bunch of syntax variations last year, including "public private", "public:private", and "public private:set", plus a few others. In an RCV poll, public private(set) was the favorite. (See link at the end of the RFC.) It also allows for extension to other operations and scopes, and for the short-hand syntax. Many of the other options did not support those. Thus we stuck with the known syntax that had the most flexibility and most support.
By other operations, I mean, suppose we allow varying the visibility for obtaining a reference separate from a set (for some reason). It's obvious how that would look with the current syntax: public protected(ref) private(set). With "public private" or "public:private", it's really not clear how we'd even do that.
--Larry Garfield
-----Original Message-----
From: Larry Garfield larry@garfieldtech.com
Sent: Wednesday, May 29, 2024 10:03 PMHello Larry,
just a quick thought.
Is there a reason why we cannot just make it "public private string
$x" instead of "public private(set) string $x"?
We would define that the second visibility specifier is for write.The current proposal with "public private(set)" is less ambiguous, and
it is immediately obvious that this is something new, and not just
somebody accidentally added two modifiers.
At the same time, it feels a bit alien and cluttered.Other options could be something like "public:private" or "public-
private".A consequence of such options would be that you always need to specify
the read visibility along with the write visibility.
But this seems ok to me.This is not a final opinion, just a thought.
We went through a bunch of syntax variations last year, including "public
private", "public:private", and "public private:set", plus a few others.
In an RCV poll, public private(set) was the favorite. (See link at the end
of the RFC.) It also allows for extension to other operations and scopes,
and for the short-hand syntax. Many of the other options did not support
those. Thus we stuck with the known syntax that had the most flexibility
and most support.
Would it make sense to do another RCV poll now that hooks are accepted, after lengthy discussion over its syntax?
Personally, I would prefer the HookEmbeddedStyle form. I don't think the argument that the visibility would potentially be separated from the hook definition is very strong, as you would have to scan for the existence of the hook anyway. For me, asymmetric visibility and property hooks are mentally more related than the RFC technically defines them to be.
Furthermore, I see a lot of reasoning in favour of the prefix syntax in relation to limitations of other language constructs like property promotion. While I don't think it is fair to expect this RFC should fix those, to me it feels we are compounding 'errors'.
Thanks to both you and Ilija for all the hard work on these RFC's, it is much appreciated!
--
Vincent de Lau
Le 30 mai 2024 à 12:16, Vincent de Lau vincent@delau.nl a écrit :
We went through a bunch of syntax variations last year, including "public
private", "public:private", and "public private:set", plus a few others.
In an RCV poll, public private(set) was the favorite. (See link at the end
of the RFC.) It also allows for extension to other operations and scopes,
and for the short-hand syntax. Many of the other options did not support
those. Thus we stuck with the known syntax that had the most flexibility
and most support.Would it make sense to do another RCV poll now that hooks are accepted, after lengthy discussion over its syntax?
At the time the poll was conducted, it was already known that a hooks RFC was in preparation, that could be compatible with either option, syntax-wise. Now, we have hooks that are compatible with both options, syntax-wise. I don’t think that would change the aesthetic preferences of people.
But now, we have indeed more information, namely detailed technical information on how the two features (hooks and aviz) interact effectively with references/arrays/readonly. At the time the poll was conducted, I was moderately in favour of the Swift-style syntax, mostly based on the general principle that things that are logically orthogonal should be implemented as orthogonal. If the same poll is done today, I will be strongly in favour of the Swift-style syntax, because I know more precisely how both features interact with arrays and readonly.
—Claude
Hi
Is there a reason why we cannot just make it "public private string
$x" instead of "public private(set) string $x"?
We would define that the second visibility specifier is for write.The current proposal with "public private(set)" is less ambiguous, and
it is immediately obvious that this is something new, and not just
somebody accidentally added two modifiers.
At the same time, it feels a bit alien and cluttered.Other options could be something like "public:private" or "public-private".
The variant with the parentheses is the most future-proof one, should
additional operations be added. It would allow for:
public private(set,unset,frobnicate) string $foo;
Even if tools would not yet understand whatever 'frobnicate' does, they
would know that this is an operation that may be performed on $foo and
would be able to syntax highlight the visibility definition properly and
they would also know how to autoformat it, without needing to add
support for the new keyword.
Best regards
Tim Düsterhus
On Wed, May 29, 2024 at 10:18 PM Larry Garfield larry@garfieldtech.com
wrote:
As promised, Ilija and I offer this revised version of asymmetric
visibility.
Hey Larry, Ilija,
I have one concern so far, and it's related to the inheritance section.
If in a class I define the property as private,
I know that there is no way for external scope or extending classes scope
to read or write to the property.
(of course, ignoring reading/writing using reflection or re-binded closures)
If an extending class defines the property with a wider visibility,
protected or public, it will shadow the initial one and not change its
visibility.
Now, if I define the property as public private(set) (or protected
private(set)) with similar intentions,
to make sure that there is no way for external scope or extending classes
scope to write to the property,
while allowing reading from external scope (or extending classes scope).
But the problem is that an extending class can define the property as
public protected(set) (or protected protected(set)),
and that will easily allow the property that I wanted to make sure it is
private for writing to be changed by an extending class to be protected.
The main suggestion I can think of, is to now allow widening the write
visibility when it is private.
I mean, in general, to make sure it is not possible for other extending
classes to access private parent class properties/methods we can use two
mechanisms:
- disallowing to change the visibility and making it an error
- shadowing the parent property/method
While shadowing works for symmetric visibility, it doesn't really work for
asymmetric visibility, so disallowing seems to be a good option here.
Also, maybe marking the properties as final could play a role here,
not allowing private visibility for write on properties to be widened or
completely not allowing it to change/redefined.
Regards,
Alex
On Wed, May 29, 2024 at 10:18 PM Larry Garfield larry@garfieldtech.com
wrote:As promised, Ilija and I offer this revised version of asymmetric
visibility.Hey Larry, Ilija,
I have one concern so far, and it's related to the inheritance section.
If in a class I define the property as private,
I know that there is no way for external scope or extending classes scope
to read or write to the property.
(of course, ignoring reading/writing using reflection or re-binded closures)If an extending class defines the property with a wider visibility,
protected or public, it will shadow the initial one and not change its
visibility.
private and protected differ here already, even without async
visibility:
A protected property does not create a new bag to store data in, which
does happen for a private property.
Now, if I define the property as public private(set) with similar
intentions, to make sure that there is no way for external scope or
extending classes scope to write to the property, while allowing
reading from external scope (or extending classes scope).But the problem is that an extending class can define the property as
public protected(set), and that will easily allow the property that I
wanted to make sure it is private for writing to be changed by an
extending class to be protected.
public private(set) properties aren't really private, so you don't get
the shadowing, but you do have a point wrt to the expectation that an
inherited class can't easily override the private(set) part (with
protected(set) or public(set)).
Hopefully Ilija or Larry can explain :-)
cheers,
Derick
Le 30 mai 2024 à 17:07, Derick Rethans derick@php.net a écrit :
Now, if I define the property as public private(set) with similar
intentions, to make sure that there is no way for external scope or
extending classes scope to write to the property, while allowing
reading from external scope (or extending classes scope).But the problem is that an extending class can define the property as
public protected(set), and that will easily allow the property that I
wanted to make sure it is private for writing to be changed by an
extending class to be protected.public private(set) properties aren't really private, so you don't get
the shadowing, but you do have a point wrt to the expectation that an
inherited class can't easily override the private(set) part (with
protected(set) or public(set)).
Note that the issue already exists today with readonly properties: those are basically private(set); but if you redeclare a non-private readonly property in a subclass, you can in fact initialise it from the subclass bypassing the initial private(set) restriction of the superclass: https://3v4l.org/9AV4r
If you want a property not to be overridable, end of story, you can mark it as final
(the final marker for properties was added as part of the hooks RFC, but it works also with non-hooked properties).
—Claude
On Fri, May 31, 2024 at 10:30 AM Claude Pache claude.pache@gmail.com
wrote:
Le 30 mai 2024 à 17:07, Derick Rethans derick@php.net a écrit :
Now, if I define the property as public private(set) with similar
intentions, to make sure that there is no way for external scope or
extending classes scope to write to the property, while allowing
reading from external scope (or extending classes scope).But the problem is that an extending class can define the property as
public protected(set), and that will easily allow the property that I
wanted to make sure it is private for writing to be changed by an
extending class to be protected.public private(set) properties aren't really private, so you don't get
the shadowing, but you do have a point wrt to the expectation that an
inherited class can't easily override the private(set) part (with
protected(set) or public(set)).Note that the issue already exists today with readonly properties: those
are basically private(set); but if you redeclare a non-private readonly
property in a subclass, you can in fact initialise it from the subclass
bypassing the initial private(set) restriction of the superclass:
https://3v4l.org/9AV4r
Yes, thank you; that's a good point.
Seems like another issue with readonly, but then again, the private(set)
part of readonly is not really explicitly designed, I guess.
If you want a property not to be overridable, end of story, you can mark
it asfinal
(the final marker for properties was added as part of the
hooks RFC, but it works also with non-hooked properties).
Yes, it seems like a good enough option, and use "final public
private(set)" to ensure only the current class will be able to set the
value.
If we all agree, I think this should be documented in the RFC.
There is another small problem, I think, with "final" modifier not being
allowed for constructor-promoted properties.
And maybe we can have this fixed in the current RFC if this ends up
being "the correct" way to define public-read private-write properties.
Regards,
Alex
Le 30 mai 2024 à 17:07, Derick Rethans derick@php.net a écrit :
Now, if I define the property as public private(set) with similar
intentions, to make sure that there is no way for external scope or
extending classes scope to write to the property, while allowing
reading from external scope (or extending classes scope).But the problem is that an extending class can define the property as
public protected(set), and that will easily allow the property that I
wanted to make sure it is private for writing to be changed by an
extending class to be protected.public private(set) properties aren't really private, so you don't get
the shadowing, but you do have a point wrt to the expectation that an
inherited class can't easily override the private(set) part (with
protected(set) or public(set)).Note that the issue already exists today with readonly properties: those are basically private(set); but if you redeclare a non-private readonly property in a subclass, you can in fact initialise it from the subclass bypassing the initial private(set) restriction of the superclass: https://3v4l.org/9AV4r
Yes, thank you; that's a good point.
Seems like another issue with readonly, but then again, the
private(set) part of readonly is not really explicitly designed, I
guess.If you want a property not to be overridable, end of story, you can mark it as
final
(the final marker for properties was added as part of the hooks RFC, but it works also with non-hooked properties).Yes, it seems like a good enough option, and use "final public
private(set)" to ensure only the current class will be able to set the
value.
If we all agree, I think this should be documented in the RFC.There is another small problem, I think, with "final" modifier not
being allowed for constructor-promoted properties.
And maybe we can have this fixed in the current RFC if this ends up
being "the correct" way to define public-read private-write properties.Regards,
Alex
Mm, yeah, this is an interesting corner case we'd not thought of. We spent a little time looking at how other languages handle this.
Kotlin just disallows using private-set on a non-final property. (Kotlin properties are final by default, unless otherwise specified.) cf: https://kotlinlang.org/spec/inheritance.html#overriding
C# disallows changing property visibility in child classes entirely. cf: https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/restricting-accessor-accessibility#access-modifiers-on-overriding-accessors
Swift is... weird. If you have a public private(set) property, you can override it by turning it into a virtual property with only a get hook. If you also define a set hook, though, it just never gets called. It doesn't appear that you can widen the set visibility, I think.
So since we now have final properties (yay, hooks RFC!), we could probably mostly solve this by just only allowing private(set) on a final property, like Kotlin. That's probably the easiest approach. We can also make private(set) properties implicitly final to avoid lots of boilerplate. (Having to always type final with private(set) every time is kinda silly.)
It would mean that proxy objects implemented using child classes with hooks would only work with a public protected(set) class, not with a private(set), but that's probably fine, and since no one is doing that quite yet, obviously, there's no existing code to be concerned about.
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
After some consideration, we believe the third option is best (least bad). The other options add still-more special cases or break existing code, neither of which are good. If a property is declared private(set), then child classes simply should not be allowed to change that, period. Even adding a get hook in a child is potentially a problem, as it would change the "round trip" behavior in ways the parent class doesn't expect. Disallowing readonly and making private(set) implicitly final avoids the loopholes in the language that would violate that expectation.
With aviz, all the use cases for readonly are essentially covered already. If you have private(set), you can control when it gets written anyway, so if you want it to be write-once, it’s write once. If you have protected(set), you’re allowing a child to set it, so they can do so whenever they want anyway, and could be doing it out of order from the parent class to begin with.
So if you wanted the equivalent of "public protected(set) readonly string $foo", in practice, the readonly is not giving you much to begin with. And "public public(set) readonly string $foo"... we just can't really think of a good use case for. Even if one exists, that could be emulated with a set hook now.
In terms of code length, thanks to the optional public-get visibility, the amount of typing is roughly the same:
readonly class ReadonlyPoint {
public function __construct(
public int $x,
public int $y,
) {}
}
class AvizPoint {
public function __construct(
private(set) int $x,
private(set) int $y,
) {}
}
(Or protected, if you prefer.) The latter provides a lot more flexibility, at the cost of "enforce write-once yourself in the class if you care", which seems an entirely reasonable tradeoff.
So we feel the best way forward is to make the following changes:
- private(set) implicitly means "final". (You can declare it explicitly if you want, but it isn't necessary.)
- Make readonly incompatible with aviz again.
Thoughts?
Also, Alexandru noted earlier that final properties don't seem to be supported in constructor promotion. That's an oversight from the hooks implementation, not a design choice, so Ilija will just fix that as part of the hooks PR before it gets fully merged.
--Larry Garfield
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
Another possible option is:
- Make readonly be
protected(set)
by default.
That would weaken the originally intended semantics of readonly, but in a compatible and acceptable way?
—Claude
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
Another possible option is:
- Make readonly be
protected(set)
by default.That would weaken the originally intended semantics of readonly, but in
a compatible and acceptable way?—Claude
Only sort of compatible. It would allow readonly props to be redefined, and wouldn't break any existing code, I think... but it would mean any time you use readonly, suddenly a child class can come along and mess with it out from under you.
In practice that's likely not a common concern, but it is a behavior change. I think it's possible (I need to confirm with Ilija), if we want that slight BC break, but I don't know if we do.
--Larry Garfield
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
Another possible option is:
- Make readonly be
protected(set)
by default.That would weaken the originally intended semantics of readonly, but in
a compatible and acceptable way?—Claude
Only sort of compatible. It would allow readonly props to be
redefined, and wouldn't break any existing code, I think... but it
would mean any time you use readonly, suddenly a child class can come
along and mess with it out from under you.In practice that's likely not a common concern, but it is a behavior
change. I think it's possible (I need to confirm with Ilija), if we
want that slight BC break, but I don't know if we do.--Larry Garfield
Ilija and I have been discussing this issue over the last few days. We agree that private(set)
should imply final
, as that eliminates a bunch of issues both implementation-wise and expectation-wise. However, that causes an issue for readonly
.
The root issue is that if we say "readonly int $x
is really just private(set) readonly int $x
", that runs into the issue of "whelp, you've just made readonly always final, which is a BC break." So that's no good.
We see a couple of ways to resolve this, presented below in our order of preference.
- Disallow readonly with aviz => No BC break, and we don't need to define readonly in terms of private(set). The only really useful combination anyway would be
readonly protected(set)
, in which case the protected(set) is doing 90% of the work already. There's few cases where the readonly is truly necessary at that point. Any other oddball edge cases could be handled by a custom hook. - Make
readonly
implicitlyprotected(set)
unless you explicitly specifyprivate(set)
=> Would have the most consistent result, and this is probably the cleanest in the engine, asreadonly private(set)
would mean exactly what it says on the tin, with no inconsistency of "well it's kinda sortaprivate(set)
" asreadonly
has now. However, this would be an expectation change, as suddenly all readonly properties could be written to from a child class. That may be good in some cases but it's possible some objects could have unexpected behavior if they didn't expect to be extended. (No existing code will break, but you could now do things to it in a child class that the author didn't anticipate.) - You can't mix
readonly
withprivate(set)
, but can use other visibilities => No BC break, and we don't need to define readonly in terms ofprivate(set)
. However, it means the implicitprivate(set)
ofreadonly
and an explicit private(set) behave differently (one is final, one is not). It also unclear if areadonly
property can be overridden withreadonly protected(set)
only, or alsoreadonly private(set)
. If the latter, does it become implicitlyfinal
at that point? -
readonly
behaves differently for an explicit (final) and implicit (not-final)private(set)
=> No BC break, but it's kinda weird and non-obvious to explain. It also has the same non-obvious inheritance questions as option 3.
We consider only the first two to be really viable. For simplicity, we'd favor doing option 1, and if desired option 2 could be presented in the future as its own RFC as that is technically a behavior change, not just addition, so deserves careful consideration. However, if there is a clear consensus to go with option 2 now, we're open to that.
--Larry Garfield
Le 5 juin 2024 à 16:28, Larry Garfield larry@garfieldtech.com a écrit :
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
Another possible option is:
- Make readonly be
protected(set)
by default.That would weaken the originally intended semantics of readonly, but in
a compatible and acceptable way?—Claude
Only sort of compatible. It would allow readonly props to be
redefined, and wouldn't break any existing code, I think... but it
would mean any time you use readonly, suddenly a child class can come
along and mess with it out from under you.In practice that's likely not a common concern, but it is a behavior
change. I think it's possible (I need to confirm with Ilija), if we
want that slight BC break, but I don't know if we do.--Larry Garfield
Ilija and I have been discussing this issue over the last few days. We agree that
private(set)
should implyfinal
, as that eliminates a bunch of issues both implementation-wise and expectation-wise. However, that causes an issue forreadonly
.The root issue is that if we say "
readonly int $x
is really justprivate(set) readonly int $x
", that runs into the issue of "whelp, you've just made readonly always final, which is a BC break." So that's no good.We see a couple of ways to resolve this, presented below in our order of preference.
- Disallow readonly with aviz => No BC break, and we don't need to define readonly in terms of private(set). The only really useful combination anyway would be
readonly protected(set)
, in which case the protected(set) is doing 90% of the work already. There's few cases where the readonly is truly necessary at that point. Any other oddball edge cases could be handled by a custom hook.- Make
readonly
implicitlyprotected(set)
unless you explicitly specifyprivate(set)
=> Would have the most consistent result, and this is probably the cleanest in the engine, asreadonly private(set)
would mean exactly what it says on the tin, with no inconsistency of "well it's kinda sortaprivate(set)
" asreadonly
has now. However, this would be an expectation change, as suddenly all readonly properties could be written to from a child class. That may be good in some cases but it's possible some objects could have unexpected behavior if they didn't expect to be extended. (No existing code will break, but you could now do things to it in a child class that the author didn't anticipate.)- You can't mix
readonly
withprivate(set)
, but can use other visibilities => No BC break, and we don't need to define readonly in terms ofprivate(set)
. However, it means the implicitprivate(set)
ofreadonly
and an explicit private(set) behave differently (one is final, one is not). It also unclear if areadonly
property can be overridden withreadonly protected(set)
only, or alsoreadonly private(set)
. If the latter, does it become implicitlyfinal
at that point?readonly
behaves differently for an explicit (final) and implicit (not-final)private(set)
=> No BC break, but it's kinda weird and non-obvious to explain. It also has the same non-obvious inheritance questions as option 3.We consider only the first two to be really viable. For simplicity, we'd favor doing option 1, and if desired option 2 could be presented in the future as its own RFC as that is technically a behavior change, not just addition, so deserves careful consideration. However, if there is a clear consensus to go with option 2 now, we're open to that.
--Larry Garfield
Hi Larry and Ilija,
Thanks for your work. Here is my opinion:
First, I do think that readonly
should integrate with aviz, unless that implies truly controversial changes on readonly
. As Theodore Brown commented in the previous version of the RFC: “Proposal feels unfinished since it can't be used in conjunction with readonly properties/classes. In my opinion the issues with this need to be resolved first, to avoid the language moving towards a messy hodgepodge of features that don't work well together.”
Second, I think that making readonly
implicitly protected(set)
by default (Option 2) is the way to go:
- At first glance it is an expectation change. But, in reality, all readonly properties can already be written to from a child class as of today: it suffices that the child class in question redeclare those properties: https://3v4l.org/9AV4r. From the point of view of the child class, the only thing that will change, is that it will no longer be required to explicitly opt into that possibility by redeclaring the readonly properties. From the point of view of the parent class, nothing will change, except false expectations—and it is a good thing that false expectations are eliminated.
- Relatively of Options 3 and 4, Option 2 leaves the language in a more simple and regular state.
—Claude
snip
Hi Larry and Ilija,Thanks for your work. Here is my opinion:
First, I do think that
readonly
should integrate with aviz, unless that
implies truly controversial changes onreadonly
. As Theodore Brown
commented in the previous version of the RFC: “Proposal feels unfinished
since it can't be used in conjunction with readonly properties/classes. In
my opinion the issues with this need to be resolved first, to avoid the
language moving towards a messy hodgepodge of features that don't work well
together.”Second, I think that making
readonly
implicitlyprotected(set)
by
default (Option 2) is the way to go:
- At first glance it is an expectation change. But, in reality, all
readonly properties can already be written to from a child class as of
today: it suffices that the child class in question redeclare those
properties: https://3v4l.org/9AV4r. From the point of view of the child
class, the only thing that will change, is that it will no longer be
required to explicitly opt into that possibility by redeclaring the
readonly properties. From the point of view of the parent class, nothing
will change, except false expectations—and it is a good thing that false
expectations are eliminated.- Relatively of Options 3 and 4, Option 2 leaves the language in a more
simple and regular state.—Claude
Hello everyone,
I've been seeing readonly bashed/blamed/being roadblock, etc, etc as in the
implementation ended up being sloppy and blocking other things or making
things hard...
While I know BC is king and stuff, why not just say "yes, this was designed
badly and we will redo it" and just do it? While there's not yet an
absolute boatload of that code out there when it would be absolutely
massive BC break? Don't repeat the mistakes of the old days :D
Cause the impression I'm getting any significant RFC now has to work around
the readonly's sloppy implementation and there's a bigger and bigger
section on that with each next RFC when there's more and more advanced
features for the OOP part of things.
--
Arvīds Godjuks
+371 26 851 664
arvids.godjuks@gmail.com
Telegram: @psihius https://t.me/psihius
snip
Hi Larry and Ilija,Thanks for your work. Here is my opinion:
First, I do think that
readonly
should integrate with aviz, unless that implies truly controversial changes onreadonly
. As Theodore Brown commented in the previous version of the RFC: “Proposal feels unfinished since it can't be used in conjunction with readonly properties/classes. In my opinion the issues with this need to be resolved first, to avoid the language moving towards a messy hodgepodge of features that don't work well together.”Second, I think that making
readonly
implicitlyprotected(set)
by default (Option 2) is the way to go:
- At first glance it is an expectation change. But, in reality, all readonly properties can already be written to from a child class as of today: it suffices that the child class in question redeclare those properties: https://3v4l.org/9AV4r. From the point of view of the child class, the only thing that will change, is that it will no longer be required to explicitly opt into that possibility by redeclaring the readonly properties. From the point of view of the parent class, nothing will change, except false expectations—and it is a good thing that false expectations are eliminated.
- Relatively of Options 3 and 4, Option 2 leaves the language in a more simple and regular state.
That's a valid point, actually. The implicit "private(set)" is already untrue because of readonly-specific workarounds to enable redeclaration. Switching it to implicit "protected(set)" would remove that special case, and still allow an explicit private(set) if desired (which would then imply final).
Since no one else has weighed in, we'll go with that route: readonly on its own changes to implicit protected(set), remove any special casing, and then readonly and aviz should be safely compatible. I'll update the RFC and Ilija will confirm that there's no implementation gotchas we're not aware of yet.
Hello everyone,
I've been seeing readonly bashed/blamed/being roadblock, etc, etc as in
the implementation ended up being sloppy and blocking other things or
making things hard...
While I know BC is king and stuff, why not just say "yes, this was
designed badly and we will redo it" and just do it? While there's not
yet an absolute boatload of that code out there when it would be
absolutely massive BC break? Don't repeat the mistakes of the old days
:D
Well, readonly has been out for 3 years. There is an absolute boatload of code out there that we do not want to break. :-)
Cause the impression I'm getting any significant RFC now has to work
around the readonly's sloppy implementation and there's a bigger and
bigger section on that with each next RFC when there's more and more
advanced features for the OOP part of things.
It's not a sloppy implementation per se. (I can't actually speak to the implementation myself.) It's the design of an implicit private(set) that works differently from any other private variable. The issue with "thou shalt not touch it outside of the constructor" isn't a language bug, it's a static-analyzer bug that those projects refuse to fix. Not something we can really do much about here. Uninitialized wasn't introduced by readonly but by property types in 7.4; readonly just inherited it. For hooks, the issue is that readonly needs a value to check to see if it's uninitialized, and with hooks, you don't always have that.
I think at this point, the change discussed above (making it implicit protected(set)) is the best we could do. In an ideal world, we would have never added readonly in the first place and just added aviz back in 8.1, which would cover nearly all the same use cases with fewer edge cases and oddities. Sadly, the world is not ideal.
--Larry Garfield
On Wed, 5 Jun 2024 at 19:59, Claude Pache claude.pache@gmail.com
wrote:
snip
Hello everyone,
I've been seeing readonly bashed/blamed/being roadblock, etc, etc as in
the implementation ended up being sloppy and blocking other things or
making things hard...
While I know BC is king and stuff, why not just say "yes, this was
designed badly and we will redo it" and just do it? While there's not
yet an absolute boatload of that code out there when it would be
absolutely massive BC break? Don't repeat the mistakes of the old days
:DWell, readonly has been out for 3 years. There is an absolute boatload of
code out there that we do not want to break. :-)Cause the impression I'm getting any significant RFC now has to work
around the readonly's sloppy implementation and there's a bigger and
bigger section on that with each next RFC when there's more and more
advanced features for the OOP part of things.It's not a sloppy implementation per se. (I can't actually speak to the
implementation myself.) It's the design of an implicit private(set) that
works differently from any other private variable. The issue with "thou
shalt not touch it outside of the constructor" isn't a language bug, it's a
static-analyzer bug that those projects refuse to fix. Not something we
can really do much about here. Uninitialized wasn't introduced by readonly
but by property types in 7.4; readonly just inherited it. For hooks, the
issue is that readonly needs a value to check to see if it's uninitialized,
and with hooks, you don't always have that.I think at this point, the change discussed above (making it implicit
protected(set)) is the best we could do. In an ideal world, we would have
never added readonly in the first place and just added aviz back in 8.1,
which would cover nearly all the same use cases with fewer edge cases and
oddities. Sadly, the world is not ideal.--Larry Garfield
It does depend on what the fix is - if we are talking removing readonly
keyword - that's a yikes and if we go that route, it needs to have an
official rector migration thing and it better be officially endorsed and
provided :)
If we are talking about tweaking how readonly works in some cases - while
not great, I hold the opinion that it's better to fix it now than 20 years
down the line.
I do have to say that I do not see a "==" between aviz and readonly. While
I can see how you can implement readonly with aviz, the boilerplate seems
not worth it, especially for bigger classes/structures where you just
designate the whole class with one "readonly class MyClass { ... }". And
constructor promotion with "private readonly Class $class" with DI is
basically all my services now - it's convenient and short and I really do
not need any more than that from the readonly :) Maybe simplifying readonly
is the answer and use aviz for more complicated cases?
Arvīds Godjuks
+371 26 851 664
arvids.godjuks@gmail.com
Telegram: @psihius https://t.me/psihius
snip
Hello everyone,
I've been seeing readonly bashed/blamed/being roadblock, etc, etc as in
the implementation ended up being sloppy and blocking other things or
making things hard...
While I know BC is king and stuff, why not just say "yes, this was
designed badly and we will redo it" and just do it? While there's not
yet an absolute boatload of that code out there when it would be
absolutely massive BC break? Don't repeat the mistakes of the old days
:DWell, readonly has been out for 3 years. There is an absolute boatload of code out there that we do not want to break. :-)
Cause the impression I'm getting any significant RFC now has to work
around the readonly's sloppy implementation and there's a bigger and
bigger section on that with each next RFC when there's more and more
advanced features for the OOP part of things.It's not a sloppy implementation per se. (I can't actually speak to the implementation myself.) It's the design of an implicit private(set) that works differently from any other private variable. The issue with "thou shalt not touch it outside of the constructor" isn't a language bug, it's a static-analyzer bug that those projects refuse to fix. Not something we can really do much about here. Uninitialized wasn't introduced by readonly but by property types in 7.4; readonly just inherited it. For hooks, the issue is that readonly needs a value to check to see if it's uninitialized, and with hooks, you don't always have that.
I think at this point, the change discussed above (making it implicit protected(set)) is the best we could do. In an ideal world, we would have never added readonly in the first place and just added aviz back in 8.1, which would cover nearly all the same use cases with fewer edge cases and oddities. Sadly, the world is not ideal.
--Larry Garfield
It does depend on what the fix is - if we are talking removing readonly
keyword - that's a yikes and if we go that route, it needs to have an
official rector migration thing and it better be officially endorsed
and provided :)
If we are talking about tweaking how readonly works in some cases -
while not great, I hold the opinion that it's better to fix it now than
20 years down the line.I do have to say that I do not see a "==" between aviz and readonly.
While I can see how you can implement readonly with aviz, the
boilerplate seems not worth it, especially for bigger
classes/structures where you just designate the whole class with one
"readonly class MyClass { ... }". And constructor promotion with
"private readonly Class $class" with DI is basically all my services
now - it's convenient and short and I really do not need any more than
that from the readonly :) Maybe simplifying readonly is the answer and
use aviz for more complicated cases?
That is essentially what we've ended up on. For services, aviz is probably not super useful; for that matter readonly is only marginally useful in services, but a lot of people (myself included) use it as a matter of course anyway. The only change we're discussing there is that "public readonly" or "protected readonly" will now be implicitly "protected(set)" instead of implicitly "private(set)". If you're already using "private readonly", then that will actually get a bit stricter as it removes the loophole that exists only for readonly. So this is a very small, focused change.
Aviz is much more useful for data classes: value objects, DTOs, domain models, ORM models, and so on. For that, readonly serves one narrow use case and does it well, but falls apart and often gets in the way for anything even slightly more complex/interesting. For that, aviz offers vastly more flexibility and robustness in about the same amount of syntax.
--Larry Garfield
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
However, this also brings up another interesting issue: readonly properties (in 8.3) DO allow redeclaration, essentially adjusting the property scope (the class that declares it) to make the visibility check pass. That is, the definition of the class it is private to changes, which is different from how inheritance works elsewhere. When the parent writes to the same property, a special check is needed to verify the two properties are related. All that special casing effectively means that readonly in 8.4 wouldn't really be "write once + private(set)", but "write once + private(set) - final", which is... just kinda screwy. That means our options are:
- A BC break on readonly (not allowing it to be overridden)
- Make readonly an exception to the implicit final.
- Just don't allow readonly with aviz after all.
Another possible option is:
- Make readonly be
protected(set)
by default.That would weaken the originally intended semantics of readonly, but in a compatible and acceptable way?
—Claude
I know this doesn't really contribute to the conversation ... but if I
could ever mash a +1 on a single email, this is the email I'd choose.
"Best elegant solution that happens to delete readonly without
deleting readonly" award.
Robert Landers
Software Engineer
Utrecht NL
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
So we feel the best way forward is to make the following changes:
- private(set) implicitly means "final". (You can declare it explicitly if you want, but it isn't necessary.)
- Make readonly incompatible with aviz again.
Thoughts?
After reflection, I don’t think that we need to make readonly incompatible with aviz, even with the current semantics of readonly (at least logically; no idea about implementationally):
- legacy-readonly properties could keep their own peculiar
private-overridable(set)
if they want; - aviz-readonly properties have, by definition, one of
public(set)
,protected(set)
orprivate(set)
marker; those will work regularly, including the implicitfinal
attached toprivate(set)
; - a non-aviz readonly property could not be redeclared in a subclass as aviz-readonly, and vice versa.
—Claude
So we feel the best way forward is to make the following changes:
- private(set) implicitly means "final". (You can declare it explicitly
if you want, but it isn't necessary.)- Make readonly incompatible with aviz again.
I'd make readonly incompatible with aviz. Readonly props have its
"peculiarities" that are being (found and) changed on each version.
If you don't want to change your value ever, use readonly.
If you want more flexibility, use aviz.
Making workarounds on the aviz RFC so it can be compatible with readonly is
not worth it, IMO.
Best regards,
Erick
Em sex., 31 de mai. de 2024 às 20:53, Claude Pache claude.pache@gmail.com
escreveu:
Le 31 mai 2024 à 18:08, Larry Garfield larry@garfieldtech.com a écrit :
So we feel the best way forward is to make the following changes:
- private(set) implicitly means "final". (You can declare it explicitly
if you want, but it isn't necessary.)- Make readonly incompatible with aviz again.
Thoughts?
After reflection, I don’t think that we need to make readonly incompatible
with aviz, even with the current semantics of readonly (at least logically;
no idea about implementationally):
- legacy-readonly properties could keep their own peculiar
private-overridable(set)
if they want;- aviz-readonly properties have, by definition, one of
public(set)
,
protected(set)
orprivate(set)
marker; those will work regularly,
including the implicitfinal
attached toprivate(set)
;- a non-aviz readonly property could not be redeclared in a subclass as
aviz-readonly, and vice versa.—Claude
On Fri, May 31, 2024 at 7:13 PM Larry Garfield larry@garfieldtech.com
wrote:
So we feel the best way forward is to make the following changes:
- private(set) implicitly means "final". (You can declare it explicitly
if you want, but it isn't necessary.)- Make readonly incompatible with aviz again.
Thoughts?
I think making properties final
when using private(set)
is a good
solution to this.
I don't think readonly
needs to be incompatible with aviz. We can have
readonly
act as private(set
) but not final
by default.
But you can define it as private(set) readonly
, and in this case it will
be final
, practically the same as final readonly
.
It would go like this:
- private(set) -> final, private(set)
- final private(set) -> final, private(set)
- readonly -> write-once, private(set)
- final readonly -> final, write-once, private(set)
- private(set) readonly -> final, write-once, private(set)
- protected(set) readonly -> write-once, protected(set)
- public(set) readonly -> write-once, public(set)
Also, Alexandru noted earlier that final properties don't seem to be
supported in constructor promotion. That's an oversight from the hooks
implementation, not a design choice, so Ilija will just fix that as part of
the hooks PR before it gets fully merged.
Maybe we need to have some discussion/considerations about allowing new
modifiers on constructor promoted properties parameters.
Right now there is no final
modifier for parameters, but this means we
might not be able to allow them in the future.
In other languages (Java), it makes the parameter variable a constant (not
reassignable).
So this decision might have implications so that when we decide to make
variables or parameters not reassignable, we will not be able to use
"final" as a keyword.
Not saying there is a problem, just that we need to be aware of it, as
probably final
is not a very good choice, and const
or readonly
are
probably better options.
Alex
Hey Larry, hey Ilija
Am 29.05.24 um 21:15 schrieb Larry Garfield:
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
- The section on magic methods has been greatly simplified. The implementation itself hasn't changed, but the explanation is a lot less confusing now.
- We've explained how aviz interacts with hooks (they don't, really) and with interface properties (in the obvious way), which didn't exist at the time of the last draft.
- We've added a section with examples of how aviz is a concrete improvement, even in a world with readonly and hooks.
- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
In general I do like the RFC. No need for a flame retardand suit IMO ;-)
There is only one thing that I stumbled upon which struck me as odd:
The set visibility, if specified explicitly, MUST be equal to or
lesser than the main (get) visibility. That is, protected public(set)
string $foo is not allowed.
Why?
Why can we not set a property as publicly writable but unreadable?
If that is a technical necessity, then so be it. But if that is a
logical limitation, then I'm asking myself: why do we want to explicitly
limit possible usecases? Not that I have one right now in my mind but I
do know that we used similar settings in Filesystems for letterbox
systems where users could copy files into other users letterbox (write
only) without being able to later see the files.
Similar possibilities are available here here a property could be
publicly writeable but only privately readable as read-access is only
granted via a method.
I'm not saying it's the usualy use-case, but why explicitly disallowing it?
Cheers
Andreas
--
,,,
(o o)
+---------------------------------------------------------ooO-(_)-Ooo-+
| Andreas Heigl |
| mailto:andreas@heigl.org N 50°22'59.5" E 08°23'58" |
| https://andreas.heigl.org |
+---------------------------------------------------------------------+
| https://hei.gl/appointmentwithandreas |
+---------------------------------------------------------------------+
| GPG-Key: https://hei.gl/keyandreasheiglorg |
+---------------------------------------------------------------------+
There is only one thing that I stumbled upon which struck me as odd:
The set visibility, if specified explicitly, MUST be equal to or
lesser than the main (get) visibility. That is, protected public(set)
string $foo is not allowed.Why?
Why can we not set a property as publicly writable but unreadable?
If that is a technical necessity, then so be it. But if that is a
logical limitation, then I'm asking myself: why do we want to explicitly
limit possible usecases? Not that I have one right now in my mind but I
do know that we used similar settings in Filesystems for letterbox
systems where users could copy files into other users letterbox (write
only) without being able to later see the files.Similar possibilities are available here here a property could be
publicly writeable but only privately readable as read-access is only
granted via a method.I'm not saying it's the usualy use-case, but why explicitly disallowing it?
It's not a technical requirement. Mainly, we just cannot think of a reason why you'd ever do that, so making some seemingly nonsensical cases illegal seems like a good guardrail.
If enough people felt strongly that it should be allowed, I don't think there's any technical reason it couldn't be allowed, other than it would allow some rather silly combinations. (Ilija can tell me if I'm wrong.) However, also note that it is, of course, much easier to allow more combinations in the future than to remove them, should we find they cause trouble.
--Larry Garfield
Hi
If enough people felt strongly that it should be allowed, I don't think there's any technical reason it couldn't be allowed, other than it would allow some rather silly combinations. (Ilija can tell me if I'm wrong.) However, also note that it is, of course, much easier to allow more combinations in the future than to remove them, should we find they cause trouble.
One thing that would get pretty wonky would be private-read properties:
Private property names are currently internally "mangled" to include the
class name. This allows to define the same private property in multiple
classes of an inheritance chain, without those classes needing to know
about the private properties of each other and making the addition and
removal of a private property not a BC break. For all intents and
purposes those private properties to not exist, unless you are the class
itself.
I have no idea what the semantics of a public-write, private-read
property should be - and this problem is pretty similar to the
sibling-discussion about making private-set properties implicitly final,
because otherwise the semantics get wonky.
I believe that the case of making a property public-write, private-read
is best left to a virtual set-only hook.
Best regards
Tim Düsterhus
Hi Tim
One thing that would get pretty wonky would be private-read properties:
Private property names are currently internally "mangled" to include the
class name. This allows to define the same private property in multiple
classes of an inheritance chain, without those classes needing to know
about the private properties of each other and making the addition and
removal of a private property not a BC break. For all intents and
purposes those private properties to not exist, unless you are the class
itself.I have no idea what the semantics of a public-write, private-read
property should be - and this problem is pretty similar to the
sibling-discussion about making private-set properties implicitly final,
because otherwise the semantics get wonky.I believe that the case of making a property public-write, private-read
is best left to a virtual set-only hook.
Indeed. A private property with a more permissible set operation is
the wrong approach. What we'd want here is a public property with a
restricted get operation. This is not quite expressible with the
current syntax. We'd need something like public private(get)
, or
public $prop { private get; }
with the C# equivalent. However, this
is quite an edge case, and since it requires additional syntax I don't
think it's something we should support without a specific use-case.
You can emulate it with a set-only virtual property, if you really
want to.
Ilija
As promised, Ilija and I offer this revised version of asymmetric
visibility.https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few
adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else
set is the most common use case.- The section on magic methods has been greatly simplified. The
implementation itself hasn't changed, but the explanation is a lot less
confusing now.- We've explained how aviz interacts with hooks (they don't, really) and
with interface properties (in the obvious way), which didn't exist at the
time of the last draft.- We've added a section with examples of how aviz is a concrete
improvement, even in a world with readonly and hooks.- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.com
Sending an email to quickly enable a new mailing list subscriber to engage
in the conversation.
I didn't like the Prefix-style
syntax. I prefer the Hook-embedded-style
syntax. First let's look at the counterpoints of the Prefix-style
syntax:
1) "Prefix-style
is more visually scannable"
The set visibility is 10 lines away from the get visibility!
Solution:
class HookEmbeddedStyle
{
public string $phone {
private set {
$this->phone = implode('', array_filter(fn($c) => is_numeric($c), explode($value)))
}
get {
if (!$this->phone) {
return '';
}
if ($this->phone[0] === 1) {
return 'US ' . $this->phone;
}
return 'Intl +' . $this->phone;
}
}
}
The set visibility is not 10 lines away from the get visibility if you put it at the top. For me this is about code convention, not about syntactic structure:
only put the hook with explicit visibility at the top, when some of the hooks do not have explicit visibility!
2) "Prefix-style
is shorter"
public private(set) string $name;
public string $name { private set; }
Irrelevant to consider 1-2 characters.
If you break the lines needed for the hook block, the line (the horizontal size) is smaller when using Hook-embedded-style
and in my opinion it is more readable because there is less information per line:
public private(set) string $name;
public string $name {
private set;
}
3) "Prefix-style
doesn't presume a connection with hooks"
As noted above in "Interaction with hooks", visibility controls exist independently of hooks.
I agree, but with Property Hooks, this should now define the overall visibility only. Like a bigger gate that you open, and there are other doors defining the visibility of the operations get, set (hooks).
In fact, as implemented, they don't interact at all. Using hook syntax for visibility controls is therefore surprising and confusing.
Why "surprising and confusing"? I see hooks as a different kind of function/method.
Property Hooks RFC shouldn't pass without requiring hook parentheses, for any hook when the property is not abstract. The $value
in the set
hook seems to come out of nowhere to some people (the exception is for hooks declared on an abstract property which you can define without parameters and thus you are free to define whatever parameters you want on the concrete property).
When you define a method in PHP, it should be possible to omit the "function", but the parameters should be required because that's the nature of a function/method: to have parameters.
4) "It's non-obvious in Hook-embedded-style
what hook behavior should be implied"
...So “arrays with hooks can do less” is already an established fact of the language. However, if the hook-style syntax is used for visibility:
class A
{
public array $arr { protected set; }
}
class B extends A
{
public function add(int $val)
{
// Should this be legal?
$this->arr[] = $val;
}
}
$b = new B();
$b->add(5);
First of all: non-abstract property hook must have a body.
Second: when asymmetric visibility is explicit, it means that symmetric visibility is implicit: a declared hook that does not have declared visibility has the same general visibility as the property, in this case: public.
Third:
There's another limitation on hooks here that makes things a bit confusing: there's a missing hook for a specific operation because you can clearly separate the set
from the push
operation...
Solution:
abstract class A
{
abstract public array $arr {
push; // Hook available only for "array" type properties only; public visibility
private set;
}
}
class B extends A
{
public array $arr {
push ($value) { // `public push ...` here is redundant
// Mandatory to implement logic here.
}
private set ($value) {
// Mandatory to Implement logic here.
}
}
public function __construct ()
{
// Legal ✅ (set hook is protected)
$this->arr = [1]; // call set hook
}
public function add(int $value)
{
// Legal ✅ (push hook is public)
$this->arr[] = $value; // call push hook
}
}
$b = new B();
$b->add(5);
$b->arr; // Legal ✅ (Inherited from the general visibility that is public)
$b->arr = [1, 2, 3]; // Fatal error ❌ - access to set hook is private only
$b->arr[] = 4; // Legal ✅ - call public push hook
My point: Prefix-style
is not future-proof
1) The Prefix-style
is not compatible with new hooks
If more hooks are added in the future, such as the push
hook for arrays or even a hook that is compatible with all types such as init
, Hook-embedded-style
will become compatible, but Prefix-style
not.
2) Hook overloading
If hook overloading is added in the future, Prefix-style
would not be supported to have this granular visibility into operations. For example, it might be possible to add a public hook and a protected hook for the same operation, where one would be triggered if the operation occurred from outside the class, and the other if the operation occurred from inside the class.
Em 10 de jun. de 2024 13:15 -0300, Tiffany tiffany.k.taylor@gmail.com escreveu:
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
- The section on magic methods has been greatly simplified. The implementation itself hasn't changed, but the explanation is a lot less confusing now.
- We've explained how aviz interacts with hooks (they don't, really) and with interface properties (in the obvious way), which didn't exist at the time of the last draft.
- We've added a section with examples of how aviz is a concrete improvement, even in a world with readonly and hooks.
- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.comSending an email to quickly enable a new mailing list subscriber to engage in the conversation.
You guys are killing PHP. There is a lot of work to be done on the engine
to modernize it and make it more robust and sturdy. Shit like this just
adds more complexity to PHP in the name of convenience. I think this is my
cue to explore other languages for handling requests
On Mon, Jun 10, 2024 at 1:18 PM Rodrigo Vieira rodrigo.vieira92@outlook.com
wrote:
I didn't like the
Prefix-style
syntax. I prefer the
Hook-embedded-style
syntax. First let's look at the counterpoints of the
Prefix-style
syntax:1) "
Prefix-style
is more visually scannable"The set visibility is 10 lines away from the get visibility!
Solution:
class HookEmbeddedStyle { public string $phone { private set { $this->phone = implode('', array_filter(fn($c) => is_numeric($c), explode($value))) } get { if (!$this->phone) { return ''; } if ($this->phone[0] === 1) { return 'US ' . $this->phone; } return 'Intl +' . $this->phone; } } }
The set visibility is not 10 lines away from the get visibility if you put
it at the top. For me this is about code convention, not about syntactic
structure:
only put the hook with explicit visibility at the top, when some of the
hooks do not have explicit visibility!2) "
Prefix-style
is shorter"public private(set) string $name; public string $name { private set; }
Irrelevant to consider 1-2 characters.
If you break the lines needed for the hook block, the line (the horizontal
size) is smaller when usingHook-embedded-style
and in my opinion it is
more readable because there is less information per line:public private(set) string $name; public string $name { private set; }
3) "
Prefix-style
doesn't presume a connection with hooks"As noted above in "Interaction with hooks", visibility controls exist
independently of hooks.I agree, but with Property Hooks, this should now define the overall
visibility only. Like a bigger gate that you open, and there are other
doors defining the visibility of the operations get, set (hooks).In fact, as implemented, they don't interact at all. Using hook syntax
for visibility controls is therefore surprising and confusing.Why "surprising and confusing"? I see hooks as a different kind of
function/method.
Property Hooks RFC shouldn't pass without requiring hook parentheses, for
any hook when the property is not abstract. The$value
in theset
hook
seems to come out of nowhere to some people (the exception is for hooks
declared on an abstract property which you can define without parameters
and thus you are free to define whatever parameters you want on the
concrete property).When you define a method in PHP, it should be possible to omit the
"function", but the parameters should be required because that's the nature
of a function/method: to have parameters.4) "It's non-obvious in
Hook-embedded-style
what hook behavior shouldbe implied"
...So “arrays with hooks can do less” is already an established fact of
the language. However, if the hook-style syntax is used for visibility:class A { public array $arr { protected set; } } class B extends A { public function add(int $val) { // Should this be legal? $this->arr[] = $val; } } $b = new B(); $b->add(5);
First of all: non-abstract property hook must have a body.
Second: when asymmetric visibility is explicit, it means that symmetric
visibility is implicit: a declared hook that does not have declared
visibility has the same general visibility as the property, in this case:
public.Third:
There's another limitation on hooks here that makes things a bit
confusing: there's a missing hook for a specific operation because you can
clearly separate theset
from thepush
operation...Solution:
abstract class A { abstract public array $arr { push; // Hook available only for "array" type properties only; public visibility private set; } } class B extends A { public array $arr { push ($value) { // `public push ...` here is redundant // Mandatory to implement logic here. } private set ($value) { // Mandatory to Implement logic here. } } public function __construct () { // Legal ✅ (set hook is protected) $this->arr = [1]; // call set hook } public function add(int $value) { // Legal ✅ (push hook is public) $this->arr[] = $value; // call push hook } } $b = new B(); $b->add(5); $b->arr; // Legal ✅ (Inherited from the general visibility that is public) $b->arr = [1, 2, 3]; // Fatal error ❌ - access to set hook is private only $b->arr[] = 4; // Legal ✅ - call public push hook
My point:
Prefix-style
is not future-proof1) The
Prefix-style
is not compatible with new hooksIf more hooks are added in the future, such as the
push
hook for arrays
or even a hook that is compatible with all types such asinit
,
Hook-embedded-style
will become compatible, butPrefix-style
not.2) Hook overloading
If hook overloading is added in the future,
Prefix-style
would not be
supported to have this granular visibility into operations. For example, it
might be possible to add a public hook and a protected hook for the same
operation, where one would be triggered if the operation occurred from
outside the class, and the other if the operation occurred from inside the
class.
Em 10 de jun. de 2024 13:15 -0300, Tiffany tiffany.k.taylor@gmail.com
escreveu:On Wed, May 29, 2024, 2:16 PM Larry Garfield larry@garfieldtech.com
wrote:As promised, Ilija and I offer this revised version of asymmetric
visibility.https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few
adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else
set is the most common use case.- The section on magic methods has been greatly simplified. The
implementation itself hasn't changed, but the explanation is a lot less
confusing now.- We've explained how aviz interacts with hooks (they don't, really) and
with interface properties (in the obvious way), which didn't exist at the
time of the last draft.- We've added a section with examples of how aviz is a concrete
improvement, even in a world with readonly and hooks.- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.comSending an email to quickly enable a new mailing list subscriber to engage
in the conversation.
You guys are killing PHP. There is a lot of work to be done on the engine
to modernize it and make it more robust and sturdy. Shit like this just
adds more complexity to PHP in the name of convenience. I think this is my
cue to explore other languages for handling requests
Please moderate your language.
Posts like this do nothing except a higher chance for people to ignore you, when you've actual points to make, such as credible suggestions as to how to make PHP "more rubust and sturdy".
Looking forwards to your RFCs and patches.
cheers
Derick
Why invest time in crafting yet another pull request or RFC when it's
glaringly obvious that you guys have no clue what you're doing? First,
there's the questionable decision to implement JIT in 8.0, followed by the
introduction of an entirely new library (IR) to a language that's
predominantly request-based, boasting nothing more than a rudimentary AST,
plagued with inconsistencies, clinging to CGI, and practically useless
without a framework. And now, after greenlighting a dubious property hooks
RFC, the same individuals are embroiled in a debate over Asymmetric
Visibility? If you can't think up ways to make the engine "more rubust and
sturdy" as a paid member of the foundation then maybe it is also time to
start reconsidering my donations.
You guys are killing PHP. There is a lot of work to be done on the engine
to modernize it and make it more robust and sturdy. Shit like this just
adds more complexity to PHP in the name of convenience. I think this is my
cue to explore other languages for handling requestsPlease moderate your language.
Posts like this do nothing except a higher chance for people to ignore
you, when you've actual points to make, such as credible suggestions as to
how to make PHP "more rubust and sturdy".Looking forwards to your RFCs and patches.
cheers
Derick
Why invest time in crafting yet another pull request or RFC when it's
glaringly obvious that you guys have no clue what you're doing? First,
there's the questionable decision to implement JIT in 8.0, followed by the
introduction of an entirely new library (IR) to a language that's
predominantly request-based, boasting nothing more than a rudimentary AST,
plagued with inconsistencies, clinging to CGI, and practically useless
without a framework. And now, after greenlighting a dubious property hooks
RFC, the same individuals are embroiled in a debate over Asymmetric
Visibility? If you can't think up ways to make the engine "more rubust and
sturdy" as a paid member of the foundation then maybe it is also time to
start reconsidering my donations.You guys are killing PHP. There is a lot of work to be done on the engine
to modernize it and make it more robust and sturdy. Shit like this just
adds more complexity to PHP in the name of convenience. I think this is
my
cue to explore other languages for handling requestsPlease moderate your language.
Posts like this do nothing except a higher chance for people to ignore
you, when you've actual points to make, such as credible suggestions as to
how to make PHP "more rubust and sturdy".Looking forwards to your RFCs and patches.
cheers
Derick
Sometimes people grow in a different direction than a project and that's
okay, you should do what you feel is best for you. Even if that means you
will no longer be enriching us with your presence, I think the PHP project
will eventually recover from such great loss.
Yeah i know im just one irrelevant person with crazy ideas (stable engine
etc). I never said PHP needs me, I simply decided to no longer be a part of
this shithole. I mean i brought up valid points about the engine, yet here
you are with a retarded cheeky response when you could've just ignored me.
Cheers,
Lanre
Why invest time in crafting yet another pull request or RFC when it's
glaringly obvious that you guys have no clue what you're doing? First,
there's the questionable decision to implement JIT in 8.0, followed by the
introduction of an entirely new library (IR) to a language that's
predominantly request-based, boasting nothing more than a rudimentary AST,
plagued with inconsistencies, clinging to CGI, and practically useless
without a framework. And now, after greenlighting a dubious property hooks
RFC, the same individuals are embroiled in a debate over Asymmetric
Visibility? If you can't think up ways to make the engine "more rubust and
sturdy" as a paid member of the foundation then maybe it is also time to
start reconsidering my donations.You guys are killing PHP. There is a lot of work to be done on the
engine
to modernize it and make it more robust and sturdy. Shit like this just
adds more complexity to PHP in the name of convenience. I think this is
my
cue to explore other languages for handling requestsPlease moderate your language.
Posts like this do nothing except a higher chance for people to ignore
you, when you've actual points to make, such as credible suggestions as to
how to make PHP "more rubust and sturdy".Looking forwards to your RFCs and patches.
cheers
DerickSometimes people grow in a different direction than a project and that's
okay, you should do what you feel is best for you. Even if that means you
will no longer be enriching us with your presence, I think the PHP project
will eventually recover from such great loss.
I didn't like the
Prefix-style
syntax. I prefer theHook-embedded-style
syntax. First let's look at the counterpoints of thePrefix-style
syntax:1) "
Prefix-style
is more visually scannable"The set visibility is 10 lines away from the get visibility!
Solution:
class HookEmbeddedStyle { public string $phone { private set { $this->phone = implode('', array_filter(fn($c) => is_numeric($c), explode($value))) } get { if (!$this->phone) { return ''; } if ($this->phone[0] === 1) { return 'US ' . $this->phone; } return 'Intl +' . $this->phone; } } }
The set visibility is not 10 lines away from the get visibility if you put it at the top. For me this is about code convention, not about syntactic structure:
only put the hook with explicit visibility at the top, when some of the hooks do not have explicit visibility!2) "
Prefix-style
is shorter"public private(set) string $name; public string $name { private set; }
Irrelevant to consider 1-2 characters.
If you break the lines needed for the hook block, the line (the horizontal size) is smaller when usingHook-embedded-style
and in my opinion it is more readable because there is less information per line:public private(set) string $name; public string $name { private set; }
3) "
Prefix-style
doesn't presume a connection with hooks"As noted above in "Interaction with hooks", visibility controls exist independently of hooks.
I agree, but with Property Hooks, this should now define the overall visibility only. Like a bigger gate that you open, and there are other doors defining the visibility of the operations get, set (hooks).
In fact, as implemented, they don't interact at all. Using hook syntax for visibility controls is therefore surprising and confusing.
Why "surprising and confusing"? I see hooks as a different kind of function/method.
Property Hooks RFC shouldn't pass without requiring hook parentheses, for any hook when the property is not abstract. The$value
in theset
hook seems to come out of nowhere to some people (the exception is for hooks declared on an abstract property which you can define without parameters and thus you are free to define whatever parameters you want on the concrete property).When you define a method in PHP, it should be possible to omit the "function", but the parameters should be required because that's the nature of a function/method: to have parameters.
4) "It's non-obvious in
Hook-embedded-style
what hook behavior should be implied"...So “arrays with hooks can do less” is already an established fact of the language. However, if the hook-style syntax is used for visibility:
class A { public array $arr { protected set; } } class B extends A { public function add(int $val) { // Should this be legal? $this->arr[] = $val; } } $b = new B(); $b->add(5);
First of all: non-abstract property hook must have a body.
Second: when asymmetric visibility is explicit, it means that symmetric visibility is implicit: a declared hook that does not have declared visibility has the same general visibility as the property, in this case: public.Third:
There's another limitation on hooks here that makes things a bit confusing: there's a missing hook for a specific operation because you can clearly separate theset
from thepush
operation...Solution:
abstract class A { abstract public array $arr { push; // Hook available only for "array" type properties only; public visibility private set; } } class B extends A { public array $arr { push ($value) { // `public push ...` here is redundant // Mandatory to implement logic here. } private set ($value) { // Mandatory to Implement logic here. } } public function __construct () { // Legal ✅ (set hook is protected) $this->arr = [1]; // call set hook } public function add(int $value) { // Legal ✅ (push hook is public) $this->arr[] = $value; // call push hook } } $b = new B(); $b->add(5); $b->arr; // Legal ✅ (Inherited from the general visibility that is public) $b->arr = [1, 2, 3]; // Fatal error ❌ - access to set hook is private only $b->arr[] = 4; // Legal ✅ - call public push hook
My point:
Prefix-style
is not future-proof1) The
Prefix-style
is not compatible with new hooksIf more hooks are added in the future, such as the
push
hook for arrays or even a hook that is compatible with all types such asinit
,Hook-embedded-style
will become compatible, butPrefix-style
not.2) Hook overloading
If hook overloading is added in the future,
Prefix-style
would not be supported to have this granular visibility into operations. For example, it might be possible to add a public hook and a protected hook for the same operation, where one would be triggered if the operation occurred from outside the class, and the other if the operation occurred from inside the class.
Em 10 de jun. de 2024 13:15 -0300, Tiffany tiffany.k.taylor@gmail.com escreveu:As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something else set is the most common use case.
- The section on magic methods has been greatly simplified. The implementation itself hasn't changed, but the explanation is a lot less confusing now.
- We've explained how aviz interacts with hooks (they don't, really) and with interface properties (in the obvious way), which didn't exist at the time of the last draft.
- We've added a section with examples of how aviz is a concrete improvement, even in a world with readonly and hooks.
- We've added a section discussing why the prefix-style syntax was chosen.
dons flame retardant suit
--
Larry Garfield
larry@garfieldtech.comSending an email to quickly enable a new mailing list subscriber to engage in the conversation.
I’m also not a fan of the prefix style, but for different reasons. My main reason is that it increases the minimum line-length, potentially forcing you to chop things down into awkward looking lines:
public
private(set)
string $longvarname {
get;
set;
}
I find that extremely hard to scan, but maybe others do not. The more natural looking syntax is easier to scan and reason about (IMHO):
public
string $longvarname {
get;
private set;
}
If I’m having to read the code, I prefer to have everything near where it is used so I don’t have to scroll up to the top and see its visibility. Granted, I haven’t used property hooks and I have no idea how IDEs will help here; maybe it is a non-issue — but I guess people still have to do code reviews which very rarely comes with IDE powers.
— Rob
I’m also not a fan of the prefix style, but for different reasons. My
main reason is that it increases the minimum line-length, potentially
forcing you to chop things down into awkward looking lines:public
private(set)
string $longvarname {
get;
set;
}I find that extremely hard to scan, but maybe others do not. The more
natural looking syntax is easier to scan and reason about (IMHO):public
string $longvarname {
get;
private set;
}If I’m having to read the code, I prefer to have everything near where
it is used so I don’t have to scroll up to the top and see its
visibility. Granted, I haven’t used property hooks and I have no idea
how IDEs will help here; maybe it is a non-issue — but I guess people
still have to do code reviews which very rarely comes with IDE powers.— Rob
I have never in my life seen someone split the visibility to a separate line from the type and variable name in PHP. I don't know why anyone would start now, especially not because of hooks or aviz. I just checked and PER-CS very directly states "All keywords MUST be on a single line, and MUST be separated by a single space." So splitting it like shown above would be against standard coding conventions anyway.
This is really a strawman argument.
--Larry Garfield
I’m also not a fan of the prefix style, but for different reasons. My
main reason is that it increases the minimum line-length, potentially
forcing you to chop things down into awkward looking lines:public
private(set)
string $longvarname {
get;
set;
}I find that extremely hard to scan, but maybe others do not. The more
natural looking syntax is easier to scan and reason about (IMHO):public
string $longvarname {
get;
private set;
}If I’m having to read the code, I prefer to have everything near where
it is used so I don’t have to scroll up to the top and see its
visibility. Granted, I haven’t used property hooks and I have no idea
how IDEs will help here; maybe it is a non-issue — but I guess people
still have to do code reviews which very rarely comes with IDE powers.— Rob
I have never in my life seen someone split the visibility to a separate line from the type and variable name in PHP. I don't know why anyone would start now, especially not because of hooks or aviz. I just checked and PER-CS very directly states "All keywords MUST be on a single line, and MUST be separated by a single space." So splitting it like shown above would be against standard coding conventions anyway.
This is really a strawman argument.
--Larry Garfield
I’m willing to concede that it might be a straw man, though I did not intend it as such. I was being serious in my pointing out of it increasing the minimum line length and PER isn’t the only coding standard. It may result in some ugly code, as in my example.
— Rob
I’m also not a fan of the prefix style, but for different reasons. My
main reason is that it increases the minimum line-length, potentially
forcing you to chop things down into awkward looking lines:public
private(set)
string $longvarname {
get;
set;
}I find that extremely hard to scan, but maybe others do not. The more
natural looking syntax is easier to scan and reason about (IMHO):public
string $longvarname {
get;
private set;
}If I’m having to read the code, I prefer to have everything near where
it is used so I don’t have to scroll up to the top and see its
visibility. Granted, I haven’t used property hooks and I have no idea
how IDEs will help here; maybe it is a non-issue — but I guess people
still have to do code reviews which very rarely comes with IDE powers.— Rob
I have never in my life seen someone split the visibility to a separate line from the type and variable name in PHP. I don't know why anyone would start now, especially not because of hooks or aviz. I just checked and PER-CS very directly states "All keywords MUST be on a single line, and MUST be separated by a single space." So splitting it like shown above would be against standard coding conventions anyway.
This is really a strawman argument.
--Larry Garfield
I’m willing to concede that it might be a straw man, though I did not
intend it as such. I was being serious in my pointing out of it
increasing the minimum line length and PER isn’t the only coding
standard. It may result in some ugly code, as in my example.— Rob
I didn't think it was deliberate. But in practice:
public string $foo; // 13 chars, excluding variable.
public readonly string $foo; // 21 chars, excluding variable.
private(set) string $foo; // 18 chars, excluding variable.
protected(set) string $foo; // 19 chars, excluding variable.
public private(set) string $foo // 24 chars, excluding variable.
public protected(set) readonly string $foo; // 29 chars, excluding variable.
Even in the most pathologically long case, where you include every possible modifier, we're up to 29 characters. Typical coding guidelines say to use either 80 or 120 character lines. So even factoring in indentation, you're still going to need a 50+ character variable name before line length becomes an issue. At that point, the issue is your silly variable name, not the modifiers.
The far more common cases would be private(set) string $foo
and protected(set) string $foo
, both of which are... shorter than public readonly string $foo
, which we already have.
In comparison, using hook-embedded style without hook implementations, we get:
public string $foo { private set; } // 29 characters excluding variable
So the base case with hook-embedded style is about 50% more characters than the prefix-style.
The only way that it wouldn't be is to structure it like so:
public string $foo {
private set;
}
At which point I'd argue that's worse in every regard, especially with how many properties are now defined in constructor promotion. That would be vastly uglier in constructor promotion than the inline style, with either syntax.
To whatever extent we care about line length and conciseness, prefix-style wins over hook-embedded, unquestionably.
--Larry Garfield
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few
adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something
else set is the most common use case.- The section on magic methods has been greatly simplified. The
implementation itself hasn't changed, but the explanation is a lot less
confusing now.- We've explained how aviz interacts with hooks (they don't, really)
and with interface properties (in the obvious way), which didn't exist
at the time of the last draft.- We've added a section with examples of how aviz is a concrete
improvement, even in a world with readonly and hooks.- We've added a section discussing why the prefix-style syntax was
chosen.
Hi folks. After a side quest to polish off hooks, we're nearly ready to bring aviz to a vote.
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Baring any new developments, we plan to start the vote early next week.
Cheers.
--Larry Garfield
Will the alternative syntax on hook not even be put to a vote?
I think the "prefix-style" syntax breaks the standard property signature template that exists since PHP 5!
Natural syntax:
<visibility> <types> $<name>;
With prefix-style:
<visibility> <visibility>(set) <types> $<name>;
This introduces a syntax that is totally unexpected to natural reading. In my opinion, pretty ugly.
What about some static code analysis tools? What about using regex to parse code and syntax in PHP (which takes a lot of work to build) for extensions used in IDEs like VSCode, IntelliJ, etc.?
To parse Property Hooks, a lot of time and work will already be spent adapting to the new syntax, and now this as well? Why not take advantage of the new Property Hooks syntax newly implemented?
Implementing visibility in the hook, which can be now the default API of the properties, would further enhance the syntax of Property Hooks, which in my opinion is one of the best features of PHP in the past five years.
If more operations on Hooks are implemented in the future, what is the use of this syntax? How could it be used? For example, an operation that is called only when a property is first set: init
hook.
public $prop {
private init => //...set a default initial value here
}
A hook called 'push' only for arrays, would solve some semantic problems and make it clear to separate the set operation (set values of an array) vs push only one item to the array as I showed in the example above.
What about other operations that are already common when we use magical methods in the future: unset, isset, etc.?
In the Property Hooks RFC, it was mentioned that more hooks may be added in the future.
It may be a default behavior for developers to want to override all magic methods for properties using Property Hooks. For now there is only get and set, but what about unset, isset, etc? How is it to define the visibility for this in the future?
public private(set, unset)....
??
Where would new operations beyond the set
will enter this syntax?
Cheers,
Rodrigo Vieira
Em Jul 19, 2024, 10:18 PM -0300, Larry Garfield larry@garfieldtech.com escreveu:
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few
adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something
else set is the most common use case.- The section on magic methods has been greatly simplified. The
implementation itself hasn't changed, but the explanation is a lot less
confusing now.- We've explained how aviz interacts with hooks (they don't, really)
and with interface properties (in the obvious way), which didn't exist
at the time of the last draft.- We've added a section with examples of how aviz is a concrete
improvement, even in a world with readonly and hooks.- We've added a section discussing why the prefix-style syntax was
chosen.Hi folks. After a side quest to polish off hooks, we're nearly ready to bring aviz to a vote.
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Baring any new developments, we plan to start the vote early next week.
Cheers.
--Larry Garfield
Will the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The preference was split, but leaned toward the prefix-style syntax. So we went with that. I don't think we'll ever get everyone to want the same syntax, but we're using the one that was both somewhat more popular, and (as discussed in the RFC) arguably superior.
As the "comments in yield from" thread has shown, any even slight change to PHP's syntax will require work from static analysis tools. That's the nature of the problem space, regardless of the syntax specifics.
--Larry Garfield
Will the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The preference was split, but leaned toward the prefix-style syntax. So we went with that. I don't think we'll ever get everyone to want the same syntax, but we're using the one that was both somewhat more popular, and (as discussed in the RFC) arguably superior.
As the "comments in yield from" thread has shown, any even slight change to PHP's syntax will require work from static analysis tools. That's the nature of the problem space, regardless of the syntax specifics.
--Larry Garfield
Just to play devil’s advocate, it was also before we had property hooks who advertised itself as a way to “wrap and guard access to object properties” but we are simply ignoring their existence here.
— Rob
Will the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The
preference was split, but leaned toward the prefix-style syntax. So
we went with that. I don't think we'll ever get everyone to want the
same syntax, but we're using the one that was both somewhat more
popular, and (as discussed in the RFC) arguably superior.As the "comments in yield from" thread has shown, any even slight
change to PHP's syntax will require work from static analysis tools.
That's the nature of the problem space, regardless of the syntax
specifics.--Larry Garfield
Just to play devil’s advocate, it was also before we had property
hooks who advertised itself as a way to “wrap and guard access to
object properties” but we are simply ignoring their existence here.
Just to compare them, because my initial gut feel was to say "yes please
just put this together with hooks"..
As far as I understand these would be the two options?
class C {
public protected(set) $answer { get => 42; set => { $this->answer = $value * 2; }
public private(set) $publicReadOnly;
}
class C {
public $answer { get => 42; protected set => { $this->answer = $value * 2; }
public $publicReadOnly { private set; }
}
And now that I see it spelled out more, I do agree that while it appears a bit more verbose, and this "(set)" looks odd at first, having all the visibility upfront is a lot clearer than having to read through the hooks to see what visibility applies.
Best,
Jordi
--
Jordi Boggiano
@seldaek -https://seld.be
Will the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The preference was split, but leaned toward the prefix-style syntax. So we went with that. I don't think we'll ever get everyone to want the same syntax, but we're using the one that was both somewhat more popular, and (as discussed in the RFC) arguably superior.
As the "comments in yield from" thread has shown, any even slight change to PHP's syntax will require work from static analysis tools. That's the nature of the problem space, regardless of the syntax specifics.
--Larry Garfield
Just to play devil’s advocate, it was also before we had property hooks who advertised itself as a way to “wrap and guard access to object properties” but we are simply ignoring their existence here.
Just to compare them, because my initial gut feel was to say "yes please just put this together with hooks"..
As far as I understand these would be the two options?
class C { public protected(set) $answer { get => 42; set => { $this->answer = $value * 2; } public private(set) $publicReadOnly; } class C { public $answer { get => 42; protected set => { $this->answer = $value * 2; } public $publicReadOnly { private set; } }
And now that I see it spelled out more, I do agree that while it appears a bit more verbose, and this "(set)" looks odd at first, having all the visibility upfront is a lot clearer than having to read through the hooks to see what visibility applies.
On a large property hook that potentially span hundreds of lines, I'd rather only need to scroll up to the "set =>" to see how it is set vs. going all the way back up to the property itself.
And now that I see it spelled out more, I do agree that while it appears a bit more verbose, and this "(set)" looks odd at first, having all the visibility upfront is a lot clearer than having to read through the hooks to see what visibility applies.
On a large property hook that potentially span hundreds of lines, I'd
rather only need to scroll up to the "set =>" to see how it is set vs.
going all the way back up to the property itself.
If someone has a property hook that is hundreds of lines long, their code is already crap and there's no hope for them.
--Larry Garfield
And now that I see it spelled out more, I do agree that while it appears a bit more verbose, and this "(set)" looks odd at first, having all the visibility upfront is a lot clearer than having to read through the hooks to see what visibility applies.
On a large property hook that potentially span hundreds of lines, I'd
rather only need to scroll up to the "set =>" to see how it is set vs.
going all the way back up to the property itself.If someone has a property hook that is hundreds of lines long, their code is already crap and there's no hope for them.
--Larry Garfield
True, but we don't always get to choose what code we work on.
— Rob
From: Rob Landers rob@bottled.codes
Sent: Sunday, July 21, 2024 11:21 AM
Will the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The preference was split, but leaned toward the prefix-style syntax. So we went with that. I don't think we'll ever get everyone to want the same syntax, but we're using the one that was both somewhat more popular, and (as discussed in the RFC) arguably superior.
As the "comments in yield from" thread has shown, any even slight change to PHP's syntax will require work from static analysis tools. That's the nature of the problem space, regardless of the syntax specifics.
Just to play devil’s advocate, it was also before we had property hooks who advertised itself as a way to “wrap and guard access to object properties” but we are simply ignoring their existence here.
I'm very disappointed that this discussion was not concluded before the vote was started. One of the main arguments for picking this syntax is the research from two years ago, when hooks where not a thing. In my opinion that makes that whole research obsolete in this new context. I've asked to redo the research, but that was not acknowledged
For the 'split visibility' concern, there has been some mentioning of reviving the var
keyword, allowing you to place all visibility in the hook block.
While I don't have the 'perfect' syntax in mind, I strongly believe that this subject required a bit more investigation and discussion. My only hope now is that the people voting take this into consideration, especially as this is now being rushed into 8.4.
--
Vincent de Lau
From: Rob Landers rob@bottled.codes
Sent: Sunday, July 21, 2024 11:21 AMWill the alternative syntax on hook not even be put to a vote?
It was, a year and a half ago when Aviz was first proposed. The preference was split, but leaned toward the prefix-style syntax. So we went with that. I don't think we'll ever get everyone to want the same syntax, but we're using the one that was both somewhat more popular, and (as discussed in the RFC) arguably superior.
As the "comments in yield from" thread has shown, any even slight change to PHP's syntax will require work from static analysis tools. That's the nature of the problem space, regardless of the syntax specifics.
Just to play devil’s advocate, it was also before we had property hooks who advertised itself as a way to “wrap and guard access to object properties” but we are simply ignoring their existence here.
I'm very disappointed that this discussion was not concluded before the
vote was started. One of the main arguments for picking this syntax is
the research from two years ago, when hooks where not a thing. In my
opinion that makes that whole research obsolete in this new context.
I've asked to redo the research, but that was not acknowledgedFor the 'split visibility' concern, there has been some mentioning of
reviving thevar
keyword, allowing you to place all visibility in the
hook block.While I don't have the 'perfect' syntax in mind, I strongly believe
that this subject required a bit more investigation and discussion. My
only hope now is that the people voting take this into consideration,
especially as this is now being rushed into 8.4.
While hooks were not a feature when Aviz was first proposed, it was very clear at the time that they were coming, and the syntax was mostly already figured out, at least in broad strokes. It's not like no one knew we'd be having {} after the property if hooks pass. I would not call the results obsolete. Rather, I think what it shows is that there's no syntax that will satisfy everyone, so trying to find a syntax favored by everyone would just waste time and end in failure anyway.
I would hardly call this rushed; it was open for multiple months, and built on the previous discussion in 2023. Lazy Objects, for instance, was first proposed a month after aviz, and started its vote on the same day. (No shade on Lazy Objects; I'm happy to see that passing.)
--Larry Garfield
As promised, Ilija and I offer this revised version of asymmetric visibility.
https://wiki.php.net/rfc/asymmetric-visibility-v2
It's still essentially the same as last year's version, but with a few
adjustments and changes:
- readonly properties are now supported in a logical fashion.
- We've brought back the abbreviated form, as public-read, something
else set is the most common use case.- The section on magic methods has been greatly simplified. The
implementation itself hasn't changed, but the explanation is a lot less
confusing now.- We've explained how aviz interacts with hooks (they don't, really)
and with interface properties (in the obvious way), which didn't exist
at the time of the last draft.- We've added a section with examples of how aviz is a concrete
improvement, even in a world with readonly and hooks.- We've added a section discussing why the prefix-style syntax was
chosen.Hi folks. After a side quest to polish off hooks, we're nearly ready to bring aviz to a vote.
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Baring any new developments, we plan to start the vote early next week.
Cheers.
--Larry Garfield
Hmm,
There’s a lot of existing code that relies on this behavior, it’s pretty much the best way to create proxies without requiring code generation.
This is a pretty massive breaking change.
— Rob
Hi Rob
Hi folks. After a side quest to polish off hooks, we're nearly ready to bring aviz to a vote.
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Hmm,
There’s a lot of existing code that relies on this behavior, it’s pretty much the best way to create proxies without requiring code generation.
This is a pretty massive breaking change.
There was a miscommunication between Larry and me. The change is not
to any existing behavior. __get/__set are currently called under two
circumstances:
- The properties full visibility is not met.
- The property was explicitly unset.
We're not changing this behavior. Instead, we decided not calling
__set for asymmetric visibility, when only the set visibility isn't
met. Before making this decision, implicitly implying protected(set)
for a readonly property would have led to __set being called (because
the scope protection now comes from asymmetric visibility, rather than
readonly itself), which would have been a change to the current
behavior.
So, in short: If only the set visibility isn't met, we're now throwing
an error. This is consistent with readonly today, and with get-only
hooks. If somebody wants to change this in the future, they can do so
without any BC breaks, and most likely should make the behavior
consistent across all of these three cases.
Ilija
Hi Rob
Hi folks. After a side quest to polish off hooks, we're nearly ready to bring aviz to a vote.
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
This is a pretty massive breaking change.
There was a miscommunication between Larry and me. The change is not
to any existing behavior. __get/__set are currently called under two
circumstances:
- The properties full visibility is not met.
- The property was explicitly unset.
We're not changing this behavior. Instead, we decided not calling
__set for asymmetric visibility, when only the set visibility isn't
met. Before making this decision, implicitly implying protected(set)
for a readonly property would have led to __set being called (because
the scope protection now comes from asymmetric visibility, rather than
readonly itself), which would have been a change to the current
behavior.So, in short: If only the set visibility isn't met, we're now throwing
an error. This is consistent with readonly today, and with get-only
hooks. If somebody wants to change this in the future, they can do so
without any BC breaks, and most likely should make the behavior
consistent across all of these three cases.Ilija
After discussing with Ilija further, I've rewritten the __set section (again). As Ilija said, the long and short of it is "we change nothing", but leave the door open to clean up __set's behavior in the future, once we decide what cleanup is warranted.
--Larry Garfield
Hi
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Baring any new developments, we plan to start the vote early next week.
I find it unfortunate that this RFC received a non-trivial change on a
Saturday after the discussion has been idle for more than a month
together with the announcement that the vote will begin shortly after
the weekend. Folks should be given sufficient time to react to the
newest developments. Personally I plan to give this RFC another read,
because I already forgot the details after a month of not looking at it,
but I'm not sure if I'll manage today.
For now:
Is there any implementation of this RFC? The RFC text only references
the https://github.com/php/php-src/pull/9257 PR hasn't been updated
since December 2022.
Best regards
Tim Düsterhus
Hi
We've made one change since we last discussed it: Specifically, Ilija realized that __set's behavior is already inconsistent, so supporting it for aviz properties with invisible set would make it even more inconsistent, not less. For that reason, we've changed the __set behavior such that a non-readonly aviz property will not trigger __set. Further details are in the RFC, but in short, all of the use cases for that behavior now have better alternatives, such as property types, hooks, and aviz itself. So there's really no point to falling back to __set in edge cases.
https://wiki.php.net/rfc/asymmetric-visibility-v2#interaction_with_set_and_unset
Baring any new developments, we plan to start the vote early next week.
I find it unfortunate that this RFC received a non-trivial change on a
Saturday after the discussion has been idle for more than a month
together with the announcement that the vote will begin shortly after
the weekend. Folks should be given sufficient time to react to the
newest developments. Personally I plan to give this RFC another read,
because I already forgot the details after a month of not looking at it,
but I'm not sure if I'll manage today.For now:
Is there any implementation of this RFC? The RFC text only references
the https://github.com/php/php-src/pull/9257 PR hasn't been updated
since December 2022.Best regards
Tim Düsterhus
Oof, sorry, I thought Ilija had updated the link already. It is now updated, and the PR is here:
https://github.com/php/php-src/pull/15063
Since the PR wasn't easily available until now, we're going to push the vote start back to this coming Friday.
--Larry Garfield
Hi
Baring any new developments, we plan to start the vote early next week.
I've went through the RFC once more. I have the following remarks:
For that reason, a private(set) property is automatically final and may not be redeclared at all.
I assume that explicitly marking it as final is still allowed (just
redundant)?
There is one caveat regarding virtual properties that have no set operation. If there is no set operation defined on a property, then it is nonsensical to specify a visibility for it. That case will trigger a compile error. For example:
How does this interact with inheritance? Consider the following example:
class P {
public $answer { get => 42; }
}
class C extends P {
public protected(set) $answer { get => 42; set => 'dummy'; }
}
This could be considered to be both narrowing the set
visibility from
public
to protected
(which is unsound), but also widening it from
“never” to protected
(which would be sound).
as it's possible now for a property to be visible but not writeable. For the time being,
I dislike the “for the time being” phrasing, because changing that would
effectively result in a breaking change, because the __set() may be
called in situations that were not anticipated. I would have preferred a
stronger phrasing that makes it clear that the RFC authors know what
they are talking about.
Best regards
Tim Düsterhus
Hi
Baring any new developments, we plan to start the vote early next week.
I've went through the RFC once more. I have the following remarks:
For that reason, a private(set) property is automatically final and may not be redeclared at all.
I assume that explicitly marking it as final is still allowed (just
redundant)?
Correct. I have added a comment to that effect.
There is one caveat regarding virtual properties that have no set operation. If there is no set operation defined on a property, then it is nonsensical to specify a visibility for it. That case will trigger a compile error. For example:
How does this interact with inheritance? Consider the following example:
class P { public $answer { get => 42; } } class C extends P { public protected(set) $answer { get => 42; set => 'dummy'; } }
This could be considered to be both narrowing the
set
visibility from
public
toprotected
(which is unsound), but also widening it from
“never” toprotected
(which would be sound).
I checked with Ilija, and confirmed that it's the second. I have updated the RFC accordingly to make that explicit.
as it's possible now for a property to be visible but not writeable. For the time being,
I dislike the “for the time being” phrasing, because changing that would
effectively result in a breaking change, because the __set() may be
called in situations that were not anticipated. I would have preferred a
stronger phrasing that makes it clear that the RFC authors know what
they are talking about.Best regards
Tim Düsterhus
I wordsmithed this a bit further. We're not trying to be wishy-washy. Rather, there are a couple of knock-on implications that are worth discussing that we consider off topic, but the approach we're taking does not preclude them. We're deliberately taking the conservative approach. Adding a __set fallback in the future would not be a BC break, as it would be changing a hard error condition to a legal condition, which is something nearly all new features do. We don't think it's wise to do that, but if someone wanted to advocate for that in the future we're not precluding it. If none of those further discussions happen, we still consider this behavior complete and acceptable.
--Larry Garfield