On Wed, 11 Jun 2025 at 21:37, Andreas Hennings <andr...@dqxtech.net> wrote: > > On Thu, 5 Jun 2025 at 16:43, Volker Dusch <vol...@tideways-gmbh.com> wrote: > > > > On Wed, Jun 4, 2025 at 6:41 PM Larry Garfield <la...@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 > > 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 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.
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 be > 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 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 > > https://tideways.io/imprint > > > > Sitz der Gesellschaft: Bonn > > Geschäftsführer: Benjamin Außenhofer (geb. Eberlei) > > Registergericht: Amtsgericht Bonn, HRB 22127
