On Fri, 16 May 2025 at 21:59, Nicolas Grekas
<[email protected]> wrote:
>
>
>
> Le jeu. 15 mai 2025 à 16:06, Larry Garfield <[email protected]> a écrit :
>>
>> On Thu, May 15, 2025, at 1:22 AM, Stephen Reay wrote:
>>
>> > I may be missing something here..
>> >
>> > So far the issues are "how do we deal with a parameter for the actual
>> > object, vs new properties to apply", "should __clone be called before
>> > or after the changes" and "this won't allow regular readonly properties
>> > to be modified".
>> >
>> > Isn't the previous suggestion of passing the new property arguments
>> > directly to the __clone method the obvious solution to all three
>> > problems?
>> >
>> > There's no potential for a conflicting property name, the developer can
>> > use the new property values in the order they see fit relative to the
>> > logic in the __clone call, and it's inherently in scope to write to any
>> > (unlocked during __clone) readonly properties.
>>
>> I did some exploratory design a few years ago on this front, looking at the
>> implications of different possible syntaxes.
>>
>> https://peakd.com/hive-168588/@crell/object-properties-part-2-examples
>>
>> What that article calls "initonly" is essentially what became readonly. The
>> second example is roughly what this RFC would look like if the extra
>> arguments were passed to __clone(). As noted in the article, the result is
>> absolutely awful.
>>
>> Auto-setting the values while using the clone($object, ...$args) syntax is
>> the cleanest solution. Given that experimentation, I would not support an
>> implementation that passes args to __clone and makes the developer figure it
>> out. That just makes a mess.
>>
>> Rob makes a good point elsewhere in thread that running __clone() afterward
>> is a way to allow the object to re-inforce validation if necessary. My
>> concern is whether the method knows it needs to do the extra validation or
>> not, since it may be arbitrarily complex. It would also leave no way to
>> reject the changes other than throwing an exception, though in fairness the
>> same is true of set hooks. Which also begs the question of whether a set
>> hook would be sufficient that __clone() doesn't need to do extra validation?
>> At least in the typical case?
>>
>> One possibility (just brainstorming) would be to update first, then call
>> __clone(), but give clone a new optional arg that just tells it what
>> properties were modified by the clone call. It can then recheck just those
>> properties or ignore it entirely, as it prefers. If that handles only
>> complex cases (eg, firstName was updated so the computed fullName needs to
>> be updated) and set hooks handle the single-property ones, that would
>> probably cover all bases reasonably well.
>
>
> I like where this is going but here is a variant that'd be even more capable:
>
> we could pass the original object to __clone.
My proposal earlier was to pass the original object _and_ the values
that were passed to the clone call, by reference.
And this would happen before those values are assigned to the object.
class MyClass {
public function __construct(
public readonly int $x,
public readonly int $y,
public readonly int $z,
) {}
public function __clone(object $original, array &$values): void {
// Set a value directly, and modify it.
if (isset($values['x'])) {
$this->x = $values['x'] * 10;
// Prevent that the same property is assigned again.
unset($values['x']);
}
}
}
$obj = new C(5, 7, 9);
$clone = clone($obj, x: 2, y: 3);
assert($clone->x === 20); // x was update in __clone().
assert($clone->y === 3); // y was auto-updated after __clone().
assert($clone->z === 9); // z was not touched at all.
>
> The benefits I see:
> - Allow implementing this validation logic you're talking about.
> - Allow to skip deep-cloning of already updated properties (that's a
> significant drawback of the current proposal - deep cloning before setting is
> a potential perf/mem hog built into the engine) : guarding deep-cloning with
> a strict comparison would be enough.
> - Allow implementing WeakMap that are able to clone weak-properties as
> objects are cloned.
>
> On this last aspect, I think it's new to the discussion but it's something
> I've always found very limiting when using weak-map: let's say some metadata
> are stored about object $foo in a weakmap, it's currently not possible to
> track those metadata across clones without using some nasty tricks. If
> __clone were given the original object, it's be easy to duplicate meta-data
> from $foo to $clone.
>
> I have just one concern significant with adding an argument to __clone:
> it'dbe a BC break to mandate this argument at the declaration level, and
> adding one right now generates an error with current versions of PHP.
> However, I think we could (and should if confirmed) provide some FC/BC layer
> by allowing one to use func_get_args() in __clone. The engine could then
> verify at compile time that __clone has either one non-nullable "object"
> argument, or zero.
This seems reasonable.
>
> Nicolas
>
>
>
>
