Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127652
Precedence: bulk
MIME-Version: 1.0
References: <CAMzRQTcN0VpQ_Ap_3WokH+aXp+EuSs=j=FLAL-43-0XgzgNtjg@mail.gmail.com>
 <CAOWwgpmWm5i-KmSAfLBQ-VN7t7SnbvGBoogT0b-fVZGhTRTsnQ@mail.gmail.com>
 <fa5a84e6-b5ad-43b8-b56f-f8f52ddcbc6d@app.fastmail.com> <CAMzRQTeZUtFiV=EgLNetik1HxhTuUghUQ3S4D1GW2+OJuTvdkQ@mail.gmail.com>
 <CAB-BujZgMd9hLjReKzMg9qdUbsc8civ+mVuMKbAGMxM-kopW9A@mail.gmail.com>
In-Reply-To: <CAB-BujZgMd9hLjReKzMg9qdUbsc8civ+mVuMKbAGMxM-kopW9A@mail.gmail.com>
Date: Wed, 11 Jun 2025 22:09:31 +0200
Message-ID: <CAB-BujZYiMgvRst2wChw4x9RnpV76F7g02UVzwGqaBV-8PKhRg@mail.gmail.com>
Subject: Re: [PHP-DEV] [VOTE] Clone with v2
To: Volker Dusch <volker@tideways-gmbh.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 Wed, 11 Jun 2025 at 21:37, Andreas Hennings <andreas@dqxtech.net> wrote:
>
> On Thu, 5 Jun 2025 at 16:43, Volker Dusch <volker@tideways-gmbh.com> wrot=
e:
> >
> > On Wed, Jun 4, 2025 at 6:41=E2=80=AFPM Larry Garfield <larry@garfieldte=
ch.com> wrote:
> >>
> >> While I support this RFC and want to see it in, I have voted no for 2 =
reasons.
> >>
> >> 1. The switch to an array parameter, as previously noted, is a major D=
X loss for unclear benefit.  It's just all-around a worse design, and "mayb=
e we can change how arrays work in the future" is not an answer.
> >
> >
> > It is frustrating that you claim the benefits to be unclear after multi=
ple long explanations, when this was thankfully unearthed during RFC discus=
sion.
> > Inventing a new syntax for this specific function call is something, I =
couldn't go forward with this, and I think we explained well why in the thr=
ead.
> >
> > But to sum up again for the benefits of readers: Using a ...variadic to=
 generate a key-value array is something php-src doesn't do anywhere curren=
tly, and it has multiple issues and edge cases. It is making the language m=
ore inconsistent and harder to use. Adding this special syntax for a single=
 function is unacceptable to me. While it looks nicer to people who  don't =
use or like PHP arrays much, an array is PHP's, especially php-src's, main =
way of passing a list of key-value pairs.
> > While vibes-based language design is tempting, and I've fallen for it i=
nitially in this RFC. I want to work with the reality of how PHP works here=
 and not leave all problems coming from this to the core team. The alternat=
ive syntax would introduce extra documentation effort, inconsistency in how=
 PHP core functions work, and generate bug reports stemming from the edge c=
ases outlined. I'd rather not have the feature at all than burden maintaine=
rs with this. Especially given how negligible the difference is in typing e=
ffort is in the end, and how it doesn't change static analysis or IDE suppo=
rt. But I know we disagree on that point.
>
> While these are valid arguments, I don't know that the other thread
> had enough time to settle and agree on this array syntax.
> The last time I looked at it, it still had the named arguments.
> (Unfortunately I don't have permissions to see the RFC edit history,
> so not sure how long ago this was changed.)
>
> >
> > This RFC is also leaving room for future improvement by still allowing =
to add further parameters if the unforeseen need for this should come up.
> >
> > Ideas around changing PHPs syntax for key-value pair lists shouldn't ha=
ve been attached to this in the first place. Extracting the ideas from this=
 discussion into things like `#[NamedParameterOnly]`, a potential 3rd array=
 syntax `[foo: "bar"]` people argued for, or ideas like this https://github=
.com/php/php-src/pull/18613 . But none of this has a place in clone-with as=
 introducing a new way of defining an array here instead of somewhere gener=
ic isn't something I feel helps PHP.
> >
> >>
> >> 2. There was still an active discussion with Nicolas going on that see=
med rather important.  Opening the vote while that was still going on is, a=
s noted above, problematic.
> >>
> >> --Larry Garfield
> >
> >
> > We answered the concerns multiple times in the discussion, including de=
claring it out of scope in the initial RFC text and explaining the issues w=
ith adding parameters to __clone in the discussions.
> > This RFC also leaves these, very much out of scope for this RFC, open i=
n the future. They would massively increase the scope of this and require a=
 ton of discussion that killed the first attempt at incremental improvement=
 already.
>
> There is a big thing here that will narrow the possibilities for a follow=
-up.
>
> "A magic __clone() method will be called before the new properties are
> assigned."
>
> The proposed change by Nicolas would require the __clone() method to
> be invoked _after_ the new properties are assigned, (and pass the
> original object as a parameter).
> By accepting the RFC as it is, we close the door to Nicolas' proposal,
> at least once this is released to the public.
>
> In the other thread I proposed an alternative where instead of passing
> the original object as a parameter to __clone(), we are passing the
> values from the "clone with" call.
> This would be more suitable when __clone() is called before the values
> are assigned.
> However, both Nicolas and me found problems with this approach.

I should add to this.
Nicolas pointed out the symmetry of the arguments made around calling
__clone before vs after.
E.g. in the RFC we currently see this:

> Calling __clone() afterward would mean the new properties would already b=
e set and the old ones gone, contrasting existing behavior and thus users' =
expectations.

But in fact both of the following sentences are true:
- Currently, no further changes are applied to a cloned object after
__clone() is called.
- Currently, no changes are applied to a cloned object before
__clone() is called.

Both of these statements can describe user expectations, but each of
them justifies a different version of the RFC.

>
> >
> > So from my perspective, there was no active discussion going on as nobo=
dy else spoke up for a week and nothing changed with Nicolas, admittedly re=
grettably timed, last email. Which we also answered in detail. So I fail to=
 see how this problematic.
>
> I don't see Nicolas' last email from 4 Jun being answered.
> It is in fact the last email in that thread.
> And one week is not very long.
>
> >
> > Letting the RFC peter out and die in the discussion, like so many other=
s, is not a desired outcome for me and as this implementation doesn't block=
 any future improvements or make anything worse in userland.
>
> As always, the RFC will block any alternative version of itself.
> E.g. if we release the array version (as currently proposed), we
> cannot later switch to a named arguments version.
> If we release the "call __clone() before assigning values", we cannot
> later switch to "call __clone() after assigning values".
> So, this does indeed feel rushed.
>
> --- Andreas
>
> >
> > Regards,
> > Volker
> >
> > --
> > Volker Dusch
> > Head of Engineering
> > Tideways GmbH
> > K=C3=B6nigswinterer Str. 116
> > 53227 Bonn
> > https://tideways.io/imprint
> >
> > Sitz der Gesellschaft: Bonn
> > Gesch=C3=A4ftsf=C3=BChrer: Benjamin Au=C3=9Fenhofer (geb. Eberlei)
> > Registergericht: Amtsgericht Bonn, HRB 22127