Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:127358
Feedback-ID: i8414410d:Fastmail
Precedence: bulk
MIME-Version: 1.0
Date: Wed, 14 May 2025 10:38:12 -0500
To: "php internals" <internals@lists.php.net>
Message-ID: <533ea02a-dd69-4de9-8526-f808c5a9f4a1@app.fastmail.com>
In-Reply-To: 
 <CAMzRQTdePFXUMWO38g=gQA2vbqL5zp=o1cPt00ec41g95xys=g@mail.gmail.com>
References: 
 <CAMzRQTdePFXUMWO38g=gQA2vbqL5zp=o1cPt00ec41g95xys=g@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] Clone with v2
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
From: larry@garfieldtech.com ("Larry Garfield")

On Wed, May 14, 2025, at 8:04 AM, Volker Dusch wrote:
> Hi internals,
>
> A couple of weeks ago, we stumbled over the fact that the original 
> "clone with" RFC was abandoned and thought it would still be useful to 
> suggest a trimmed down version of the proposal.
>
> The main goal of this RFC is to propose a lightweight, low friction 
> implementation of this feature to satisfy the couple of use cases we 
> see and "round out" PHP in how it handles cloning, especially around 
> read-only-properties.
>
> https://wiki.php.net/rfc/clone_with_v2
>
> We are actively looking for some feedback on 
> https://wiki.php.net/rfc/clone_with_v2#open_issues, but of course all 
> points are welcome.
>
> If this turns out to be more complex than anticipated, for some reason, 
> specifically around syntax or BC implications, we'd rather not add this 
> at all.
>
> A preliminary implementation that shows how small the change set needs 
> to be exists at https://github.com/TimWolla/php-src/pull/6
>
> Kind Regards,
> Tim and Volker

As discussed off list, I really like this approach.  It's definitely cleaner than the earlier versions.

> A magic __clone() method will be called before the new properties are assigned.

Why before and not after?  I could probably make a good argument either direction, but we should be explicit about why we're making whatever decision.

The last example, on readonly, is a bit confusing.  It looks like it should work, but the comments say "but if we did this other thing we'd get this error."  Just make it two separate classes and show one works and one doesn't.  That makes it easier to follow.

> Alternatively, we could drop the variadic syntax and instead only accept an array as the second parameter. We're looking for feedback here in the discussion. 

Oh lord please no. :-)  Not using an array here is what I like about this syntax.  An __object parameter is fine with me as a workaround.  One shouldn't be using a named argument for that anyway.

--Larry Garfield