Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:128025
Precedence: bulk
MIME-Version: 1.0
References: <1e8634d7-ac1a-4025-b4e2-1948aabf5251@app.fastmail.com> <9D5043B2-1589-4FD5-B289-6E98FB1177BE@nicksdot.dev>
In-Reply-To: <9D5043B2-1589-4FD5-B289-6E98FB1177BE@nicksdot.dev>
Date: Mon, 14 Jul 2025 01:28:27 +0200
Message-ID: <CAPyj-LDVi3ZYQuzzJz7d+cHdU6C-cxYJUSkuzdyvWpw+sWuCsg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Readonly property hooks
To: Nick <php@nicksdot.dev>
Cc: 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 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> wrote:
>>
>> 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 computed `=
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.

Side note: Your implementation has a bug:

```
class C {
    public public(set) readonly int $prop {
        get =3D> $this->prop + 1;
    }
}

function test($c) {
    var_dump($c->prop);
}

$c =3D new C();
$c->prop =3D 1;
test($c); // int(2)
test($c); // int(3)
```

You likely need to dodge the fast path in the VM by not marking the
property as "SIMPLE_GET" [^2].

Sorry if this e-mail is a bit all over the place, I had trouble
structuring it in a more sensible way.

Ilija

[^1]: https://github.com/php/php-src/compare/master...NickSdot:php-php-src:=
readonly-hooks-once
[^2]: https://github.com/php/php-src/blob/4d9fc506df1131c630c530a0bfa6d0338=
cffa03c/Zend/zend_object_handlers.c#L864