Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:127401 X-Original-To: internals@lists.php.net Delivered-To: internals@lists.php.net Received: from php-smtp4.php.net (php-smtp4.php.net [45.112.84.5]) by lists.php.net (Postfix) with ESMTPS id 10A1F1A00BC for ; Mon, 19 May 2025 15:13:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=php.net; s=mail; t=1747667479; bh=hhglusDI6bgHmejqfmCnKbsNHeQkluZk3Tgxt8Qll9w=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=i4HGEuY7QV6LIeLQiRpuW19FM2Zbh4KBKyKiGGnd9SOBlxk0lj2sMLPIoy84Hqtd1 +/77aMibA2eEfSNr2qOzldIFdndcvi+RA5uttxjwcHmjo9u9JalrjLSJQhybP9KPkM SMOcInq/NuWAmelWk9rBxUNtQbFcxmO0BHXF2hdW9jjBMiV5Nqdc2wORNO62Q+MlwN Z5QhCuO0Xbp6im8tJ8XRLF/ytqqnPcv7wRwsjOs4JzblylPM9bdq4RiMdRVnj2L0oE +qDMZxUvZpvwEsaWByH+eo2SA/1q1g9wF0QecAsGOetn7h0zrG+RpXYlNVgytSugEn QZBNastUUxSHw== Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 6BD9118007C for ; Mon, 19 May 2025 15:11:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on php-smtp4.php.net X-Spam-Level: * X-Spam-Status: No, score=1.6 required=5.0 tests=BAYES_50,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,DMARC_PASS,FREEMAIL_FROM, FREEMAIL_REPLY,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=4.0.0 X-Spam-Virus: Error (Cannot connect to unix socket '/var/run/clamav/clamd.ctl': connect: Connection refused) X-Envelope-From: Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Mon, 19 May 2025 15:11:18 +0000 (UTC) Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-551f14dc30dso1090854e87.1 for ; Mon, 19 May 2025 08:13:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1747667606; x=1748272406; darn=lists.php.net; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Za65iOtaDy30Kn7ePa4LKgpYF/+HdcyPJHinhbHlDCA=; b=g5HAaTOHgaCem2JDlCCiYcuRDNfIacVm8n/FovuvD3QqUSxNHZ7evmmbumYUywR66l Gp5z1JN3uagkrNQiK+gcfFbKBQqYXaUs8rzoKppAlY2gjtPguq6lQFEUKss6BaZLHvUG PEr96e3SmLIbok1XSYVYrNpuXICSp8XW33Wv/EpgcmzO4mBa6fIw9SpoIjrYWRtRRqB9 eyUp03S82D/ZxgZjdfO2MAaJsUMDFGb1Y+mB4qishBe/1ebPbdB/43zsJ/fiL4/s8PyA efz+1x1N0PsgoxjPkkr303pQNBZOWL6jvA/GXsD6o55NU/9HLG+bR9Coaml+dt9onWpo RizQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747667606; x=1748272406; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Za65iOtaDy30Kn7ePa4LKgpYF/+HdcyPJHinhbHlDCA=; b=N6knxqWMeHv5eBg57YjPnqtw/OGX7OvgYZo4L6A+7aK5srwu7OYRZIL0gOrrP1Z8ea 0EnW1uQCzIHdooUvheEWCnUERbi0B1l9XRFEfCkB0kHjMgq5+YPLN9vBHej0OCoTQL25 rxwb0ZYcgCczIJbSIe/7Q63OQSlauBoavNrkPNtCGMVqyYIuh6VQCkeYzwQIp5M/Cp30 NH4zJOQddLmo3TwtCQSkQsFKI9TH4t0n6YKovY5UQIeJ7n/kHpaMzH/DmqoKSJO4awRk wVtBeT+dfMKla5ppY+mpcVRiluHa0mVNXPdCBuGDzDP3cPGY9nyysuNGmQ0fC3KST4Gt ZdKQ== X-Forwarded-Encrypted: i=1; AJvYcCWy5QJJz5PHoAv4IhkJB7V3Cx4yKbXvi7HBUIs7j1DUwKdMrn5ezRXp+9ajAv4pplLu9KY36451N74=@lists.php.net X-Gm-Message-State: AOJu0Yw5zmmsNduN1RTI5gq+2A8C+BUcSnWo70mRWmBb2AL1i9Girmwp Ka9TZ2vEPUaJrmBRgd/qZH5Hxk17JabSfqSkVUOUHWtVLtcvfG1UAW/3tmVirDfI/YgwBC7nRJ3 SU0RjdsoalMY8j0H3iHan25hZM9Ijxl72eqlv X-Gm-Gg: ASbGncsCLcrv065jHY4o7RkD9jyMbRJo+113C75SuYr+UxGdUwhDeK9ErfB8w866Sj5 Jj3jpIBqsqbikBNNbg58xBwGWp8AjnBVb+HbvvVc9oSHVRhT8HJ6BpQHXnt48px92l8bWa3ueBS d6FiA1NADvE3XG3ouikuxYAhSYlwNokoAv X-Google-Smtp-Source: AGHT+IG1qSrd6lGHBiZaz83ewkufUXkzc2P8nhoTo3eg65iAyzKJyk76tIubsxKz2v3svAvl6CuFgwVKW0W5l4054XU= X-Received: by 2002:a05:6512:2610:b0:545:a70:74c5 with SMTP id 2adb3069b0e04-550e749fce1mr3755332e87.13.1747667605477; Mon, 19 May 2025 08:13:25 -0700 (PDT) Precedence: bulk list-help: list-post: List-Id: internals.lists.php.net x-ms-reactions: disallow MIME-Version: 1.0 References: In-Reply-To: Date: Mon, 19 May 2025 17:13:12 +0200 X-Gm-Features: AX0GCFuZ3B5n3oKTpxNT5B-2rCFV_X9nuujKhJ_ivbWdd09aTo7gsBzxO41DpL4 Message-ID: Subject: Re: [PHP-DEV] [RFC] Clone with v2 To: Andreas Hennings Cc: Larry Garfield , php internals Content-Type: multipart/alternative; boundary="000000000000cf5e5606357e924a" From: nicolas.grekas+php@gmail.com (Nicolas Grekas) --000000000000cf5e5606357e924a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Le lun. 19 mai 2025 =C3=A0 16:30, Andreas Hennings a =C3=A9crit : > On Fri, 16 May 2025 at 21:59, Nicolas Grekas > wrote: > > > > > > > > Le jeu. 15 mai 2025 =C3=A0 16:06, Larry Garfield 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 actua= l > >> > object, vs new properties to apply", "should __clone be called befo= re > >> > 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 th= e > >> > 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 a= t > 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, t= he > result is absolutely awful. > >> > >> Auto-setting the values while using the clone($object, ...$args) synta= x > is the cleanest solution. Given that experimentation, I would not suppor= t > 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 ext= ra > validation or not, since it may be arbitrarily complex. It would also > leave no way to reject the changes other than throwing an exception, thou= gh > 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 tho= se > properties or ignore it entirely, as it prefers. If that handles only > complex cases (eg, firstName was updated so the computed fullName needs t= o > 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 =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. > 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 settin= g > is a potential perf/mem hog built into the engine) : guarding deep-clonin= g > 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 n= ot > 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 --000000000000cf5e5606357e924a Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Le=C2=A0lun. 19= mai 2025 =C3=A0=C2=A016:30, Andreas Hennings <andreas@dqxtech.net> a =C3=A9crit=C2=A0:
On Fri, 16 May 2025 at 21:59,= Nicolas Grekas
<nic= olas.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 f= or the actual
>> > object, vs new properties to apply",=C2=A0 "should = __clone be called before
>> > or after the changes" and "this won't allow reg= ular 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 th= ree
>> > problems?
>> >
>> > There's no potential for a conflicting property name, the= developer can
>> > use the new property values in the order they see fit relativ= e to the
>> > logic in the __clone call, and it's inherently in scope t= o write to any
>> > (unlocked during __clone) readonly properties.
>>
>> I did some exploratory design a few years ago on this front, looki= ng at the implications of different possible syntaxes.
>>
>> https://peakd.com/hiv= e-168588/@crell/object-properties-part-2-examples
>>
>> What that article calls "initonly" is essentially what b= ecame readonly.=C2=A0 The second example is roughly what this RFC would loo= k like if the extra arguments were passed to __clone().=C2=A0 As noted in t= he article, the result is absolutely awful.
>>
>> Auto-setting the values while using the clone($object, ...$args) s= yntax is the cleanest solution.=C2=A0 Given that experimentation, I would n= ot support an implementation that passes args to __clone and makes the deve= loper figure it out.=C2=A0 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 necessar= y.=C2=A0 My concern is whether the method knows it needs to do the extra va= lidation or not, since it may be arbitrarily complex.=C2=A0 It would also l= eave no way to reject the changes other than throwing an exception, though = in fairness the same is true of set hooks.=C2=A0 Which also begs the questi= on of whether a set hook would be sufficient that __clone() doesn't nee= d to do extra validation?=C2=A0 At least in the typical case?
>>
>> One possibility (just brainstorming) would be to update first, the= n call __clone(), but give clone a new optional arg that just tells it what= properties were modified by the clone call.=C2=A0 It can then recheck just= those properties or ignore it entirely, as it prefers.=C2=A0 If that handl= es only complex cases (eg, firstName was updated so the computed fullName n= eeds to be updated) and set hooks handle the single-property ones, that wou= ld probably cover all bases reasonably well.
>
>
> I like where this is going but here is a variant that'd be even mo= re 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 {
=C2=A0 public function __construct(
=C2=A0 =C2=A0 public readonly int $x,
=C2=A0 =C2=A0 public readonly int $y,
=C2=A0 =C2=A0 public readonly int $z,
=C2=A0 ) {}
=C2=A0 public function __clone(object $original, array &$values): void = {
=C2=A0 =C2=A0 // Set a value directly, and modify it.
=C2=A0 =C2=A0 if (isset($values['x'])) {
=C2=A0 =C2=A0 =C2=A0 $this->x =3D $values['x'] * 10;
=C2=A0 =C2=A0 =C2=A0 // Prevent that the same property is assigned again. =C2=A0 =C2=A0 =C2=A0 unset($values['x']);
=C2=A0 =C2=A0 }
=C2=A0 }
}

$obj =3D new C(5, 7, 9);
$clone =3D clone($obj, x: 2, y: 3);
assert($clone->x =3D=3D=3D 20);=C2=A0 // x was update in __clone().
assert($clone->y =3D=3D=3D 3);=C2=A0 // y was auto-updated after __clone= ().
assert($clone->z =3D=3D=3D 9);=C2=A0 // 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 h= ook instead, not a cloner.
=C2=A0
>
> 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 setti= ng is a potential perf/mem hog built into the engine) : guarding deep-cloni= ng 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&#= 39;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 e= asy 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 a= nd 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.

Nic= olas
--000000000000cf5e5606357e924a--