On Thu, 5 Jun 2025 at 16:43, Volker Dusch <volker@tideways-gmbh.com> wrote:
On Wed, Jun 4, 2025 at 6:41 PM Larry Garfield <larry@garfieldtech.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 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 multiple long explanations, when this was thankfully unearthed during RFC discussion.
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 thread.
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 currently, and it has multiple issues and edge cases. It is making the language more 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 initially 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 alternative syntax would introduce extra documentation effort, inconsistency in how PHP core functions work, and generate bug reports stemming from the edge cases outlined. I'd rather not have the feature at all than burden maintainers with this. Especially given how negligible the difference is in typing effort 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 discussion into things like `#[NamedParameterOnly]`, a potential 3rd array syntax `[foo: "bar"]` people argued for, or ideas like this Make `array()` a function by TimWolla · Pull Request #18613 · php/php-src · GitHub . But none of this has a place in clone-with as introducing 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 seemed 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 declaring it out of scope in the initial RFC text and explaining the issues with 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 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.
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 regrettably 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 others, 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önigswinterer Str. 116
53227 Bonn
Imprint – Tideways
Sitz der Gesellschaft: Bonn
Geschäftsführer: Benjamin Außenhofer (geb. Eberlei)
Registergericht: Amtsgericht Bonn, HRB 22127