On Thu, Jul 17, 2025 at 3:31 AM Rob Landers <rob@bottled.codes> wrote: > > On Tue, Jul 15, 2025, at 19:27, Nicolas Grekas wrote: > > > > Le lun. 14 juil. 2025 à 15:41, Larry Garfield <la...@garfieldtech.com> a > écrit : > > 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? > > > It does to me: readonly applies to the backed property, then hooks add > behavior as see fit. This is especially useful to intercept accesses to said > properties. Without readonly hooks, designing an abstract API that uses > readonly properties is a risky decision since it blocks any (future) > implementation that needs this interception capability. As a forward-thinking > author, one currently has two choices: not using readonly properties in > abstract APIs, or falling back to using getter/setters. That's a design > failure for hooks IMHO. I'm glad this RFC exists to fill this gap. > > Nicolas > > > To add to this, as I just mentioned on the Records thread, it would be good > to get hooks on readonly objects. With the new clone(), there is no way to > rely on validation in constructors. The most robust validation in 8.5 can > only be done via set/get hooks, but these hooks are not available on readonly > classes. This means that it is remarkably easy to "break" objects that do > constructor validation + use public(set) -- or use clone() in inherited > objects instead of the parent constructor. In my experience, readonly objects > typically only do constructor validation (DRY).
(shoot, double post, sorry Rob) I'm not sure I follow - do you actually need both `set` and `get` hooks for validation? I would think only `set` hooks would be necessary, and I don't yet think I've seen an objection to `set` hooks for `readonly`.
