Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123069
Precedence: bulk
MIME-Version: 1.0
References: <CAPyj-LBMQEJ=2BF=xk_on4O2xTU+MM2XRAZ5g=uPwVOf=bh+sw@mail.gmail.com>
 <CAAKU0u=amT01bwyFdhhwvDR-X-_L39qFsOo2+4HoVZ4hBoV2NQ@mail.gmail.com> <9ff3864e-7fb8-4ad1-bbd7-f8c69567a34d@app.fastmail.com>
In-Reply-To: <9ff3864e-7fb8-4ad1-bbd7-f8c69567a34d@app.fastmail.com>
Date: Tue, 9 Apr 2024 21:32:42 +0200
Message-ID: <CAPzBOBPaGtUMJrEv60BpGBnka2KoZh22KhafkrOHP4G=QM6nGg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC][Vote announcement] Property hooks
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: landers.robert@gmail.com (Robert Landers)

On Tue, Apr 9, 2024 at 8:56=E2=80=AFPM Larry Garfield <larry@garfieldtech.c=
om> wrote:
>
> On Tue, Apr 9, 2024, at 4:07 PM, Levi Morrison wrote:
>
> > I was playing around with 3v4l.org. Is the implementation up-to-date th=
ere?
> >
> > I don't have any hard objections at the moment, but after playing with
> > it for a while, I do kind of wonder if it's a lot of complexity for
> > what is effectively a niche feature because:
> >
> >  1. It does not support asymmetric visibility for get/set. Having a
> > public getter and private setter seems really natural.
>
> Aviz is a separate RFC, for reasons we've covered before: They're actuall=
y two separate features that do not depend on each other, so we split them =
up to help reduce RFC size (something RFC authors are often encouraged to d=
o).  The Aviz RFC was put to a vote last year but didn't pass.  If hooks pa=
ss, we do want to revisit aviz and bring it to another vote, as the argumen=
t may be stronger now with hooks in place.
>
> >  2. You can't access accessors for "siblings".
>
> If a use case for this is found, that should be addable in a future RFC w=
ith no notable BC break.  (Assuming a syntax of self::$otherProp::get() or =
similar, it would only have the same slight edge case as the parent::$thisP=
rop::get() has, as documented in the RFC.  As we've decided that is too edg=
e-casey to care about here, presumably the same would hold true for such an=
 follow-up.)
>
> >  3. You can't do by-reference set (important for arrays).
>
> This is due to logical limitations in the context.  By-ref set means set =
hooks don't get called.  So we can either block by-ref manipulation when a =
set hook is defined, or we can make set hooks easily-bypassable suggestions=
 at best.  We have not found any way around that decision.  (And as noted i=
n the RFC, this problem is already present if just using methods.  It's not=
 a new issue.)
>
> Please note that *the scope of what is blocked has been reduced considera=
bly from earlier versions of the RFC!*  The only invalid combination is &ge=
t/set on a backed property, because of the issue above.  Writing to an arra=
y returned from a hook is allowed in certain circumstances, namely, if it w=
as returned by &get and there is no set.
>
> So we're only preventing the narrowest set of operations we reasonably ca=
n, without hamstringing the concept of a set hook in the first place, and w=
e don't believe there is any logical approach that would allow more.  If in=
 the future someone can come up with an approach that would work, it would =
likely be addable in a BC way.
>
> >  4. You can't satisfy a parent's readonly property with a getter in a c=
hild.
>
> In the last week or two we've figured out a situation where we may be abl=
e to allow this; iff the child getter works by reading the parent's value, =
then it would *probably* still be able to satisfy readonly's guarantees.  W=
e haven't tried implementing it yet as the RFC is large enough as is, but i=
t's possible that could be added in the future.  The main issue is that the=
re's no way to fully guarantee that a get hook on a readonly property is id=
emopotent, the way a bare readonly property is.  (It's not a lack of desire=
 to support it, just the logical issues in ensuring different features' inv=
ariants are maintained.)
>
> Note that it's still unclear what set from a child set hook should do, gi=
ven that readonly properties are also silently private-set, even if the pro=
perty itself is protected or public.  Potentially we could mandate that a c=
hild set MUST use parent::$prop::set(), so the actual write is technically =
in the parent class.  I've noted this before as a design flaw in readonly t=
hat really needs to be corrected, but that's not a job for this RFC.  (We a=
ctually had a solution in the aviz RFC, but people pushed back on it so we =
had to remove it.)
>
> > Now, points 2 through 4 are fairly minor and niche by themselves, but
> > if we take all these restrictions as a whole... I'm a bit worried.
>
> Hopefully the above comments make you less worried. :-)
>
> --Larry Garfield

Off-topic random thought:

> The Aviz RFC was put to a vote last year but didn't pass.

It would be really nice if votes weren't just a yes/no vote, but
yes/needs-more-work/no vote, where needs-more-work and no are
effectively the same in terms of passing the RFC, but needs-more-work
just means there is more to do (either addressing ugly syntax or the
idea is sound, but as it says, it needs more work), and can thus be
simply revoted on after concerns are addressed -- instead of creating
a whole new RFC that needs to pass the "not too similar to other RFCs
rule."

I got the impression from the Aviz discussions that most people were
against Aviz due to the syntax, not the feature itself. It would be
absolutely tragic if this failed to pass simply because people
expected Aviz here.

Robert Landers
Software Engineer
Utrecht NL