Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123593
Precedence: bulk
MIME-Version: 1.0
References: <CAPyj-LCW6ehN4j5p8Wf3ubftEaFz21R4x0ALkK+UtJHNoB9jKw@mail.gmail.com>
 <I1fGmGC3KmELG1T4T1BPyTLXv9UCqRqfV28qcx031srr0OcNS1ufKVVAuvh5uvUzKFttk7IigWISaEf7Bmhjua_X7FKYMxxiwuAy3SHpMp8=@gpb.moe>
In-Reply-To: <I1fGmGC3KmELG1T4T1BPyTLXv9UCqRqfV28qcx031srr0OcNS1ufKVVAuvh5uvUzKFttk7IigWISaEf7Bmhjua_X7FKYMxxiwuAy3SHpMp8=@gpb.moe>
Date: Thu, 13 Jun 2024 10:33:54 +0200
Message-ID: <CAPyj-LD63+r+yuyXXkNUt5Jt5NA14M_+AxY2ejov+iFKu85rtg@mail.gmail.com>
Subject: Re: [PHP-DEV] Property hooks, and &get-only write behavior
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: tovilo.ilija@gmail.com (Ilija Tovilo)

Hi Gina

On Thu, Jun 13, 2024 at 3:24=E2=80=AFAM Gina P. Banyard <internals@gpb.moe>=
 wrote:
>
> On Wednesday, 12 June 2024 at 16:06, Ilija Tovilo <tovilo.ilija@gmail.com=
> wrote:
>
> > Hi everyone
> >
> > [...]
> >
> > I hope that these semantics are acceptable for everyone.
> >
> > Ilija
>
> Hello Ilija,
>
> I might know what sort of technical issues you've been running into, as I=
 have some similar issues correctly handling references with my Container/O=
ffset RFC implementation.
> The way I'm handling this is by splitting the "get"/read handler/hook to =
*just* be a by-value return, and for fetching creating a new "fetch" handle=
r that is called in the respective situations.
> Therefore, I am wondering if it wouldn't possibly make more sense to add =
a "fetch" hook, and ban returning by-ref for the get hook.

I don't know exactly what issue you encountered, but it sounds
unrelated to mine. The issue was not related to the design of &get,
but the way object handlers work for properties. In particular,
get_property_ptr_ptr does not provide zval space for temporary values.

There are three object handlers relevant:

* get_property_ptr_ptr, which returns a direct pointer to the property
zval. This is usually used for read+write operations such as ++, or
when assigning by-ref, like =3D&.
* read_property
* write_property

read_property and write_property are also used as fallbacks when
get_property_ptr_ptr returns NULL. This is the case for hooks.
Essentially, for hooks, we want to make sure the engine does not
bypass the set hook by writing to the zval directly.

Pure assignments aren't really an issue, I was able to implement them
using write_property. When a value is assigned to a &get-only
property, we call &get and assign the value to the resulting
reference. However, for read+write operations, the flow works like
this:

* get_property_ptr_ptr returns NULL, indicating that a direct
modification of the zval is not possible.
* The engine calls read_property instead.
* It modifies the resulting value.
* It calls write_property with the new value.

With this workflow, &get gets called twice, once for read_property and
once for write_property, which is the unexpected part. I would expect
&get only to be called once, and for this value to be modified
in-place. To implement this properly, get_property_ptr_ptr should
return the reference instead.

However, the way get_property_ptr_ptr is built doesn't allow for this,
because unlike read_property, it doesn't have a `zval *rv` parameter
the caller must supply that the callee may use in the case of
temporary values (such as &__get, or here, &get).

So, rather than causing a big internal API change to add the `zend
*rv` parameter (this API is very old), I opted to go with the original
plan.

I hope that helps.

Ilija