Re: [RFC] Clone with v2

On Fri, 16 May 2025 at 21:59, Nicolas Grekas <nicolas.grekas+php@gmail.com> wrote: > > > > Le jeu. 15 mai 2025 à 16:06, Larry Garfield <larry@garfieldtech.com> 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. >> >> –ûÓ >> M²·`¶ÄÏY¯¸…¢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 > > > >