Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127400
Precedence: bulk
MIME-Version: 1.0
References: <CAMzRQTdePFXUMWO38g=gQA2vbqL5zp=o1cPt00ec41g95xys=g@mail.gmail.com>
 <CAJp_myXREmnyufwxj4tDCsopouUFEWenap5EJaH+sXKTyg4K1Q@mail.gmail.com>
 <CAMzRQTfWj=CFsu=OyOiK_HiwCM5UkyPhfY3DiwSBF4N3FtzObA@mail.gmail.com>
 <F38892E6-4219-4B1A-A2C5-4F06AE70056F@koalephant.com> <bca6d562-6e31-4908-a01b-fb524f226029@app.fastmail.com>
 <CAOWwgpkvqwf-CXiQd_t1MikpDO31n2xbHrrc1WxR78HtrhOMzQ@mail.gmail.com>
In-Reply-To: <CAOWwgpkvqwf-CXiQd_t1MikpDO31n2xbHrrc1WxR78HtrhOMzQ@mail.gmail.com>
Date: Mon, 19 May 2025 16:30:13 +0200
Message-ID: <CAB-BujbofnxBGB1OMz_A4erA6P0K2JfrEz8Nrcyr+nCQcPN-Dg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Clone with v2
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: Larry Garfield <larry@garfieldtech.com>, php internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: andreas@dqxtech.net (Andreas Hennings)

On Fri, 16 May 2025 at 21:59, Nicolas Grekas
<nicolas.grekas+php@gmail.com> wrote:
>
>
>
> Le jeu. 15 mai 2025 =C3=A0 16:06, Larry Garfield <larry@garfieldtech.com>=
 a =C3=A9crit :
>>
>> 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 propertie=
s
>> > 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 ca=
n
>> > 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 an=
y
>> > (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 a=
rguments 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 figur=
e it out.  That just makes a mess.
>>
>> Rob makes a good point elsewhere in thread that running __clone() afterw=
ard 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 r=
eject 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 h=
ook 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 prope=
rties were modified by the clone call.  It can then recheck just those prop=
erties or ignore it entirely, as it prefers.  If that handles only complex =
cases (eg, firstName was updated so the computed fullName needs to be updat=
ed) and set hooks handle the single-property ones, that would probably cove=
r all bases reasonably well.
>
>
> I like where this is going but here is a variant that'd be even more capa=
ble:
>
> 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 =3D $values['x'] * 10;
      // Prevent that the same property is assigned again.
      unset($values['x']);
    }
  }
}

$obj =3D new C(5, 7, 9);
$clone =3D clone($obj, x: 2, y: 3);
assert($clone->x =3D=3D=3D 20);  // x was update in __clone().
assert($clone->y =3D=3D=3D 3);  // y was auto-updated after __clone().
assert($clone->z =3D=3D=3D 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 sign=
ificant 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 ob=
jects are cloned.
>
> On this last aspect, I think it's new to the discussion but it's somethin=
g I've always found very limiting when using weak-map: let's say some metad=
ata 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: i=
t'dbe a BC break to mandate this argument at the declaration level, and add=
ing one right now generates an error with current versions of PHP.
> However, I think we could (and should if confirmed) provide some FC/BC la=
yer by allowing one to use func_get_args() in __clone. The engine could the=
n verify at compile time that __clone has either one non-nullable "object" =
argument, or zero.

This seems reasonable.

>
> Nicolas
>
>
>
>