On Thu, Jul 17, 2025, at 15:10, Eric Norris wrote:
> 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`.
>
It depends... for example, you might have an isValid property which computes
whether or not the object is valid, or in the case of lazy properties,
detecting an invalid state there. You also might put validation in getters
because you're building up the object over several lines, thus it might only be
in a partially valid state during construction:
readonly class User {
public public(set) $first_name;
public public(set) $last_name;
public $name { get => implode(' ', [$this->first_name, $this->last_name]) }
}
$user->first_name = "Rob"
echo $user->name; // oops
Or something. In this case, we can rely on the "uninitialized property"
exception to be raised if it isn't yet fully valid, but you might want to throw
your own exception.
— Rob
