Re: [VOTE] Clone with v2

From: Date: Wed, 11 Jun 2025 20:09:31 +0000
Subject: Re: [VOTE] Clone with v2
References: 1 2 3 4 5  Groups: php.internals 
Request: Send a blank email to internals+get-127652@lists.php.net to get a copy of this message
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> 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 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


Thread (15 messages)

« previous php.internals (#127652) next »