On Mon, Jan 9, 2023, at 10:00 AM, Nicolas Grekas wrote:
>> I am hereby opening the vote on the Asymmetric Visibility RFC:
>>
>> https://wiki.php.net/rfc/asymmetric-visibility
>>
>> Voting will be open for 2 weeks.
>>
>> While we would certainly prefer if everyone voted in favor, for those who
>> vote against the authors kindly request that you indicate why, using the
>> second poll question or comment section afterward. Thank you.
>>
>
> Hi Larry, Ilija,
>
> Thanks for opening the vote. I played a bit with the code and I didn't spot
> any unexpected behavior, nice work.
>
> I'm undecided for now, on two points:
> 1. the syntax: looking at https://wiki.php.net/rfc/property-hooks, I wonder
> if using public string $foo {private set;} wouldn't be more appropriate.
> Otherwise, it might be strange to have public private(set) string $foo {set
> ($value) { /* the setter */};} if hooks are the way forward. Sure we could
> compact that but that would introduce two very different syntax whether a
> setter is defined or not. WDYT?
As discussed, putting the visibility on the right has a number of problems.
1. If we assume that hooks in something similar to the current unreleased RFC
happen (which I hope they do), then putting the visibility in the hook area
creates additional problems.
For one, hooks will require disabling getting a reference to a property. That
is not negotiable, as getting a reference to a property would let you bypass
the hooks. Aviz doesn't require that, though. If the only thing you're doing
is private-set, then using the hooks syntax for aviz means either introducing
yet another access marker, like "private raw" to indicate that you don't want
to disable references (I don't want to have to explain that to people), or just
sucking it up and saying that aviz disables references.
But disabling references also makes `$obj->arrayProp[] = 4` break entirely. So
that would make arrays completely incompatible with asymmetric visibility, when
there's no actual reason that has to be the case.
So in short, we end up with three options:
A) The current proposed syntax.
B) Having both "{ private set }" and "{private raw}", and having to explain WTH
that even means.
C) Arrays are incompatible with asymmetric visibility entirely, period.
Of those, A is, I would argue, the clearly better option.
2. Some nerdy engine details, courtesy of Ilija. Basically, given the way
we're looking at doing hooks, mixing the hook and a-viz syntax would make it
*more* complicated, not less.
We're hoping to completely avoid generated accessors ({ get; set; } with no
additional behavior) completely. Essentially, all they do is disable indirect
modification (i.e. modifying arrays with []) and references (assigning a
reference or extracting a reference). The reason they have to do that is that
both of those things allow mutating the value of the property in a way where we
cannot call the accessors, either because the engine isn't aware of it or
because there's no simple way to "feed" the modified value into the setter.
There are two primary reasons generated modifiers existed in the previous
draft: 1. To avoid a BC break when adding a hook in the future and 2. to allow
sub-classes to override the property while staying compatible with the parent.
As described here
(https://gist.github.com/iluuu1994/4ca8cc52186f6039f4b90a0e27e59180#use-case-5-arrays-properties)
we've come to the conclusion that this issue of incompatibility is almost
exclusively restricted to array properties. However, there are other reasons
why publicly exposing an array property with write access isn't a good idea, as
outlined in the gist. In other words, this compatibility layer provided by {
get; set; } was provided for something that shouldn't be a publicly writable
property in the first place. We're now hoping to completely avoid the { get;
set; } syntax which also makes public private(set) a better fit, instead of
offering a fake { get; private set; } syntax that is either limiting (i.e. not
working properly for arrays) or semantically inconsistent (not preventing
indirect modification and references).
3. More stylistically, it means visibility may end up split in two different
locations. That's harder for people to read.
Especially if we add hooks later, it means they could be several lines apart!
Consider:
public string $foo {
beforeSet {
return strtolower($value);
}
afterSet {
$this->modified['foo'] = true;
}
get => $this->_foo;
protected set => $this->_foo = $value;
}
This is a worst-case example, but worth considering as it would be valid, legal
syntax and almost certainly appear in the wild. The "set visibility" is 8
lines away from the "get visibility." Too, the hooks are more confusing
because only one of them supports an extra keyword; the other three have none.
Even in a more reasonable ordering:
public string $foo {
get => $this->_foo;
protected set => $this->_foo = $value;
beforeSet {
return strtolower($value);
}
afterSet {
$this->modified['foo'] = true;
}
}
Visibility is still spread across separate non-contiguous lines.
Compare the same example with the syntax in the RFC now:
public private(set) string $foo {
beforeSet {
return strtolower($value);
}
afterSet {
$this->modified['foo'] = true;
}
get => $this->_foo;
set => $this->_foo = $value;
}
It's now completely obvious that there's aviz in play. I don't even have to
look at the hooks to know that.
Aviz and hooks simply do two different things in different parts of the engine.
The poll we ran was split on this question, but did show a clear preference for
what is in the RFC now.
I think one of the points some people are hung up on is thinking of the hooks
like methods, and thus they should have method-esque syntax. That's not an
unreasonable first attempt at a mental model, but in practice I don't think
it's accurate. That's one reason the follow-up RFC renames accessors to
"hooks", because they really are "hooks" into the existing property access
workflow. They're not methods unto themselves. That means "well methods have
a visibility modifier so shouldn't these methods have one?" doesn't make sense,
as the hooks are not methods.
> 2. the feature itself: I feel like this new syntax doesn't open new
> capabilities over using methods, so this might "just" add complexity in the
> end, adding alternative ways to do something already doable (I'm making the
> argument for hooks also BTW). Would you be able to clarify why we need
> this? (The only new capability that feels appealing to me is the "init"
> modifier, but this is not what this RFC is about.)
It's true that *technically* anything that you can do with aviz can be done
with methods. However, the same is true of readonly. Technically we could
also enforce types on properties through methods too, without needing property
types. And constructor promotion didn't enable any new functionality, just
less typing/reading. But all of these things improve the ergonomics of the
language.
In particular, aviz mainly lets us get rid of a whole host of pointless getter
methods. Consider a typical Doctrine model.
class Point
{
private string $id;
private int $x;
private int $y;
private int $z;
public function getId(): string
{
return $this->id;
}
public function getX(): int
{
return $this->x;
}
public function getY(): int
{
return $this->y;
}
public function getZ(): int
{
return $this->z;
}
public function setX(int $x): void
{
$this->x = $x;
}
public function setY(int $y): void
{
$this->y = $y;
}
public function setZ(int $z): void
{
$this->z = $z;
}
}
Aviz lets us remove 4 of the 7 methods in that class:
class Point
{
public private(set) string $id;
public private(set) int $x;
public private(set) int $y;
public private(set) int $z;
public function setX(int $x): void
{
$this->x = $x;
}
public function setY(int $y): void
{
$this->y = $y;
}
public function setZ(int $z): void
{
$this->z = $z;
}
}
Assuming we still want some kind of control over the set actions, say enforcing
positive values. That's not necessary for the get side, only the set side.
As for the next obvious question of "well what if I do want to do something
special on get, too?" In that rare instance, that's what the hooks RFC is for.
Another use case is in my Serde library (https://github.com/Crell/Serde). I
won't go too far into details here (as me in chat somewhere if you want the
dirty details), but in short I have an object (an attribute class, actually)
that does a lot of multi-stage stateful setup via reflection and other
mechanisms, and then is "frozen" with a bunch of public readonly properties.
Being readonly is a very nice API and very convenient, but I have to do some
tricky dancing in the setup code to make sure I only write to the properties
once and in the right order. There's some default-handling functionality that
I want to add but don't know how to do safely while using a readonly property.
Right now, that's the only option if I want to have public-read-only
properties. With a-viz, I think I could simplify some of the code by switching
to public private(set), eliminate some of extra handling, and easily add
another feature or two, all without an API change.
Aviz also lets us side-step readonly's insistence on private-set. Right now,
there is no option for "public read, protected write", In most cases, `public
protected(set)` would be completely sufficient in practice, but is not
currently possible.
> About my point 2., did you consider a syntax to map a getter+setter to
> properties? That might fill the need for asym visibility + hooks in one go?
> I'm seeing quite often libraries that do this sort of mapping based on
> conventions. This would bake the practice into the language:
>
> public string $foo {get: getFoo, set: setFoo} or something like that, with
> method visibility being added on top of the property visibility.
The currently in-progress proposal for hooks supports almost that as a trivial
option, thanks to the shorthand support.
public string $foo {
get => $this->someGetter();
set => $this->someSetter($value);
beforeSet => $this->someCallback($value);
}
But you can also inline it if it's simple. It gives developers both options.
We did consider using methods directly for hooks, more akin to how JS and
Python do it. We eventually rejected that entirely as trying to keep the
visibility and the type in sync between the property and the methods would
be... really really hard, both for the implementation and for developers. With
the current setup, there is no opportunity for confusion in the engine and
extremely little for the developer.
> Sorry to chime in that late, I'm really undecided and discussing those
> topics would help.
>
> Thanks,
> Nicolas
I'm honestly a bit surprised to see this comment from you, as you coined the
term "asymmetric visibility" in the first place, and IIRC objected to
`readonly` on the grounds it would make aviz harder. (Turns out, you were
right. :-) ) Still, I hope I was able to clarify things.
--Larry Garfield
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
