Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127391
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>
In-Reply-To: <bca6d562-6e31-4908-a01b-fb524f226029@app.fastmail.com>
Date: Fri, 16 May 2025 21:58:00 +0200
Message-ID: <CAOWwgpkvqwf-CXiQd_t1MikpDO31n2xbHrrc1WxR78HtrhOMzQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Clone with v2
To: Larry Garfield <larry@garfieldtech.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000c1aec80635463335"
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000c1aec80635463335
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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 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 i=
s
> the cleanest solution.  Given that experimentation, I would not support a=
n
> 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.

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.

Nicolas

--000000000000c1aec80635463335
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">Le=C2=A0jeu. 15=
 mai 2025 =C3=A0=C2=A016:06, Larry Garfield &lt;<a href=3D"mailto:larry@gar=
fieldtech.com">larry@garfieldtech.com</a>&gt; a =C3=A9crit=C2=A0:<br></div>=
<blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-=
left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, May 15, 2025, at =
1:22 AM, Stephen Reay wrote:<br>
<br>
&gt; I may be missing something here.. <br>
&gt;<br>
&gt; So far the issues are &quot;how do we deal with a parameter for the ac=
tual <br>
&gt; object, vs new properties to apply&quot;,=C2=A0 &quot;should __clone b=
e called before <br>
&gt; or after the changes&quot; and &quot;this won&#39;t allow regular read=
only properties <br>
&gt; to be modified&quot;.<br>
&gt;<br>
&gt; Isn&#39;t the previous suggestion of passing the new property argument=
s <br>
&gt; directly to the __clone method the obvious solution to all three <br>
&gt; problems? <br>
&gt;<br>
&gt; There&#39;s no potential for a conflicting property name, the develope=
r can <br>
&gt; use the new property values in the order they see fit relative to the =
<br>
&gt; logic in the __clone call, and it&#39;s inherently in scope to write t=
o any <br>
&gt; (unlocked during __clone) readonly properties.<br>
<br>
I did some exploratory design a few years ago on this front, looking at the=
 implications of different possible syntaxes.<br>
<br>
<a href=3D"https://peakd.com/hive-168588/@crell/object-properties-part-2-ex=
amples" rel=3D"noreferrer" target=3D"_blank">https://peakd.com/hive-168588/=
@crell/object-properties-part-2-examples</a><br>
<br>
What that article calls &quot;initonly&quot; is essentially what became rea=
donly.=C2=A0 The second example is roughly what this RFC would look like if=
 the extra arguments were passed to __clone().=C2=A0 As noted in the articl=
e, the result is absolutely awful.<br>
<br>
Auto-setting the values while using the clone($object, ...$args) syntax is =
the cleanest solution.=C2=A0 Given that experimentation, I would not suppor=
t an implementation that passes args to __clone and makes the developer fig=
ure it out.=C2=A0 That just makes a mess.<br>
<br>
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.=C2=A0 =
My concern is whether the method knows it needs to do the extra validation =
or not, since it may be arbitrarily complex.=C2=A0 It would also leave no w=
ay to reject the changes other than throwing an exception, though in fairne=
ss the same is true of set hooks.=C2=A0 Which also begs the question of whe=
ther a set hook would be sufficient that __clone() doesn&#39;t need to do e=
xtra validation?=C2=A0 At least in the typical case?<br>
<br>
One possibility (just brainstorming) would be to update first, then call __=
clone(), but give clone a new optional arg that just tells it what properti=
es were modified by the clone call.=C2=A0 It can then recheck just those pr=
operties or ignore it entirely, as it prefers.=C2=A0 If that handles only c=
omplex cases (eg, firstName was updated so the computed fullName needs to b=
e updated) and set hooks handle the single-property ones, that would probab=
ly cover all bases reasonably well.</blockquote><div><br></div><div>I like =
where this is going but here is a variant that&#39;d be even more capable:<=
/div><div><br></div><div>we could pass the original object to __clone.</div=
><div><br></div><div>The benefits I see:</div><div>- Allow implementing thi=
s validation logic you&#39;re talking about.</div><div>- Allow to skip deep=
-cloning of already updated properties (that&#39;s a significant drawback o=
f the current proposal - deep cloning before setting is a potential perf/me=
m hog built into the engine) : guarding deep-cloning with a strict comparis=
on would be enough.</div><div>- Allow implementing WeakMap that are able to=
 clone weak-properties as objects are cloned.</div><div><br></div><div>On t=
his last aspect, I think it&#39;s new to the discussion but it&#39;s someth=
ing I&#39;ve always found very limiting when using weak-map: let&#39;s say =
some metadata are stored about object $foo in a weakmap, it&#39;s currently=
 not possible to track those metadata across clones without using some nast=
y tricks. If __clone were given the original object, it&#39;s be easy to du=
plicate meta-data from $foo to $clone.</div><div><br></div><div>I have just=
 one concern significant with adding an argument to __clone: it&#39;dbe a B=
C break to mandate this argument at the declaration level, and adding one r=
ight now generates an error with current versions of PHP.</div><div>However=
, I think we could (and should if confirmed) provide some FC/BC layer by al=
lowing one to use func_get_args() in __clone. The engine could then verify =
at compile time that __clone has either one non-nullable &quot;object&quot;=
 argument, or zero.</div><div><br></div><div>Nicolas</div><div><br></div><d=
iv><br></div><div><br></div><div>=C2=A0</div></div></div>

--000000000000c1aec80635463335--