On Sun, Jul 13, 2025, at 6:28 PM, Ilija Tovilo wrote: > Hi Nick > > On Fri, Jul 11, 2025 at 6:31 AM Nick <p...@nicksdot.dev> wrote: >> >>> On 8. Jun 2025, at 11:16, Larry Garfield <la...@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 => strtoupper($this->prop); > } > } > $c = new C(); > $c->prop = '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 => $this->prop + 1; > } > } > $c = new C(); > $c->prop = 1; > $s1 = serialize($c); > $c->prop; > $s2 = 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 => $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 => 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. Perhaps we're thinking about this the wrong way, though? So far we've talked 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, multiple times. Only writing to the actual value in the object table. That gives the exact same set of guarantees that a getX()/setX() method would 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 ??=), 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 === $foo->bar in all cases (though that would likely hold in the 99% case in practice), but then, $foo->getBar() === $foo->getBar() has never been guaranteed either. Would that way of looking at it be acceptable to folks? --Larry Garfield
