Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127651
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>
In-Reply-To: <CAMzRQTeZUtFiV=EgLNetik1HxhTuUghUQ3S4D1GW2+OJuTvdkQ@mail.gmail.com>
Date: Wed, 11 Jun 2025 21:37:38 +0200
Message-ID: <CAB-BujZgMd9hLjReKzMg9qdUbsc8civ+mVuMKbAGMxM-kopW9A@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 Thu, 5 Jun 2025 at 16:43, Volker Dusch <volker@tideways-gmbh.com> wrote:
>
> On Wed, Jun 4, 2025 at 6:41=E2=80=AFPM Larry Garfield <larry@garfieldtech=
.com> wrote:
>>
>> While I support this RFC and want to see it in, I have voted no for 2 re=
asons.
>>
>> 1. The switch to an array parameter, as previously noted, is a major DX =
loss for unclear benefit.  It's just all-around a worse design, and "maybe =
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 multipl=
e long explanations, when this was thankfully unearthed during RFC discussi=
on.
> Inventing a new syntax for this specific function call is something, I co=
uldn't go forward with this, and I think we explained well why in the threa=
d.
>
> But to sum up again for the benefits of readers: Using a ...variadic to g=
enerate a key-value array is something php-src doesn't do anywhere currentl=
y, and it has multiple issues and edge cases. It is making the language mor=
e inconsistent and harder to use. Adding this special syntax for a single f=
unction is unacceptable to me. While it looks nicer to people who  don't us=
e or like PHP arrays much, an array is PHP's, especially php-src's, main wa=
y of passing a list of key-value pairs.
> While vibes-based language design is tempting, and I've fallen for it ini=
tially in this RFC. I want to work with the reality of how PHP works here a=
nd not leave all problems coming from this to the core team. The alternativ=
e syntax would introduce extra documentation effort, inconsistency in how P=
HP core functions work, and generate bug reports stemming from the edge cas=
es outlined. I'd rather not have the feature at all than burden maintainers=
 with this. Especially given how negligible the difference is in typing eff=
ort is in the end, and how it doesn't change static analysis or IDE support=
. 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 have=
 been attached to this in the first place. Extracting the ideas from this d=
iscussion into things like `#[NamedParameterOnly]`, a potential 3rd array s=
yntax `[foo: "bar"]` people argued for, or ideas like this https://github.c=
om/php/php-src/pull/18613 . But none of this has a place in clone-with as i=
ntroducing a new way of defining an array here instead of somewhere generic=
 isn't something I feel helps PHP.
>
>>
>> 2. There was still an active discussion with Nicolas going on that seeme=
d rather important.  Opening the vote while that was still going on is, as =
noted above, problematic.
>>
>> --Larry Garfield
>
>
> We answered the concerns multiple times in the discussion, including decl=
aring it out of scope in the initial RFC text and explaining the issues wit=
h adding parameters to __clone in the discussions.
> This RFC also leaves these, very much out of scope for this RFC, open in =
the future. They would massively increase the scope of this and require a t=
on of discussion that killed the first attempt at incremental improvement a=
lready.

There is a big thing here that will narrow the possibilities for a follow-u=
p.

"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.

>
> So from my perspective, there was no active discussion going on as nobody=
 else spoke up for a week and nothing changed with Nicolas, admittedly regr=
ettably timed, last email. Which we also answered in detail. So I fail to s=
ee 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 others,=
 is not a desired outcome for me and as this implementation doesn't block a=
ny 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