Le lun. 19 mai 2025 à 16:30, Andreas Hennings <andr...@dqxtech.net> a écrit :
> On Fri, 16 May 2025 at 21:59, Nicolas Grekas > <nicolas.grekas+...@gmail.com> wrote: > > > > > > > > Le jeu. 15 mai 2025 à 16:06, Larry Garfield <la...@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. > >> > >> 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. > I'm not sure I understand, there might be missing bits to your idea, eg where is visibility enforced? why is pass-by-ref needed at all? Pass-by-ref makes me think this is a bad idea already :) Also, WDYT of my simpler proposal itself? Wouldn't it cover all use cases? Note that I don't see the need for operations like in your example (transforming a value while cloning). This looks like the job of a setter or a hook instead, not a cloner. > > > > 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. > > I'd be happy to know what Volker and Tim think about this? I read they excluded any change to __clone in the RFC, but I think it should still be possible to discuss this, especially if it provides the path to the desired solution. Nicolas
