Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:128038
Feedback-ID: i8414410d:Fastmail
Precedence: bulk
MIME-Version: 1.0
Date: Mon, 14 Jul 2025 08:38:32 -0500
To: "php internals" <internals@lists.php.net>
Message-ID: <0856c89f-2000-448a-bbbf-c145a8699f6a@app.fastmail.com>
In-Reply-To: 
 <CAPyj-LDVi3ZYQuzzJz7d+cHdU6C-cxYJUSkuzdyvWpw+sWuCsg@mail.gmail.com>
References: <1e8634d7-ac1a-4025-b4e2-1948aabf5251@app.fastmail.com>
 <9D5043B2-1589-4FD5-B289-6E98FB1177BE@nicksdot.dev>
 <CAPyj-LDVi3ZYQuzzJz7d+cHdU6C-cxYJUSkuzdyvWpw+sWuCsg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Readonly property hooks
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Sun, Jul 13, 2025, at 6:28 PM, Ilija Tovilo wrote:
> Hi Nick
>
> On Fri, Jul 11, 2025 at 6:31=E2=80=AFAM Nick <php@nicksdot.dev> wrote:
>>
>>> On 8. Jun 2025, at 11:16, Larry Garfield <larry@garfieldtech.com> wr=
ote:
>>>
>>> https://wiki.php.net/rfc/readonly_hooks
>>>
>>> To not get this buried in individual answers to others:
>>
>> I came up with two alternative implementations which cache the comput=
ed `get` hook value.
>> One leverages separate cache properties, the other writes directly to=
 the backing store.
>>
>> Links to the alternative branches can be found in the description of =
the original PR.
>> https://github.com/php/php-src/pull/18757
>
> I am not a fan of the caching approach. The implementation draft for
> this approach [^1] works by storing the assigned value in the property
> slot, and replacing it with the value returned from get one called for
> the first time. One of the issues here is that the backing value is
> observable without calling get. For example:
>
> ```
> class C {
>     public public(set) readonly string $prop {
>         get =3D> strtoupper($this->prop);
>     }
> }
> $c =3D new C();
> $c->prop =3D 'foo';
> var_dump(((array)$c)['prop']); // foo
> $c->prop;
> var_dump(((array)$c)['prop']); // FOO
> ```
>
> Here we can see that the underlying value changes, despite the
> readonly declaration. This is especially problematic for things like
> [un]serialize(), where calling serialize() before or after accessing
> the property will change which underlying value is serialized. Even
> worse, we don't actually know whether an unserialized property has
> already called the get hook.
>
> ```
> class C {
>     public public(set) readonly int $prop {
>         get =3D> $this->prop + 1;
>     }
> }
> $c =3D new C();
> $c->prop =3D 1;
> $s1 =3D serialize($c);
> $c->prop;
> $s2 =3D serialize($c);
> var_dump(unserialize($s1)->prop); // int(2)
> var_dump(unserialize($s2)->prop); // int(3)
> ```
>
> Currently, get is always called after unserialize(). There may be
> similar issues for __clone().
>
> For readable and writable properties, the straight-forward solution is
> to move the logic to set.
>
> ```
> class C {
>     public public(set) readonly int $prop {
>         set =3D> $value + 1;
>     }
> }
> ```
>
> This is slightly differently, semantically, in that it executes any
> potential side-effects on write rather than read, which seems
> reasonable. This also avoids the implicit mutation mentioned
> previously. At least in these cases, disallowing readonly + get seems
> reasonable to me. I will say that this doesn't solve all get+set
> cases. For example, proxies. Hopefully, lazy objects can mostly bridge
> this gap.
>
> Another case is lazy getters.
>
> ```
> class C {
>     public readonly int $magicNumber {
>         get =3D> expensiveComputation();
>     }
> }
> ```
>
> This does not seem to work in the current implementation:
>
>> Fatal error: Hooked virtual properties cannot be declared readonly
>
> I presume it would be possible to fix this, e.g. by using readonly as
> a marker to add a backing value to the property. I'm personally not
> too fond of making the rules on which properties are backed more
> complicated, as this is already a common cause for confusion. I also
> fundamentally don't like that readonly changes whether get is called.
> Currently, if hooks are present, they are called. This adds more
> special cases to an already complex feature.
>
> To me it seems the primary motivator for this RFC are readonly
> classes, i.e. to prevent the addition of hooks from breaking readonly
> classes. However, as lazy-getters are de-facto read-only, given they
> are only writable from the extremely narrow scope of the hook itself,
> the modifier doesn't do much. Maybe an easier solution would be to
> provide an opt-out of readonly.

Thanks, Ilija.  You expressed my concerns as well.  And yes, in practice=
, readonly classes over-reaching is the main use case; if you're marking=
 individual properties readonly, then just don't mark the one that has a=
 hook on it (use aviz if needed) and there's no issue. =20

Perhaps we're thinking about this the wrong way, though?  So far we've t=
alked as though readonly makes the property write-once.  But... what if =
we think of it as applying to the field, aka the backing value?

So readonly doesn't limit calling the get hook, or even the set hook, mu=
ltiple times.  Only writing to the actual value in the object table.  Th=
at gives the exact same set of guarantees that a getX()/setX() method wo=
uld give.  The methods can be called any number of times, but the stored=
 value can only be written once.

That would allow conditional set hooks, conditional gets, caching gets (=
like we already have with ??=3D), and so on.  The mental model is simple=
 and easy to explain/document.  The behavior is the same as with methods=
.  But the identity of the stored value would be consistent.

It would not guarantee $foo->bar =3D=3D=3D $foo->bar in all cases (thoug=
h that would likely hold in the 99% case in practice), but then, $foo->g=
etBar() =3D=3D=3D $foo->getBar() has never been guaranteed either.

Would that way of looking at it be acceptable to folks?

--Larry Garfield