Hi
Am 2025-06-03 16:24, schrieb Nicolas Grekas:
- We decided to __clone before updating properties to avoid BC issues.
Not sure which BC issues you've in mind, especially as that's a new feature.
As I see it, before or after wouldn't change anything as far as __clone
implementations are concerned.
While clone-with is a new feature, it is automatically supported on every class. Currently calling `__clone()` is the first thing that happens after cloning the object and the implementation of `__clone()` might rely on the properties being unchanged from the original object. If the reassignment of properties would happen before the call to `__clone()` using clone-with with classes that have such a `__clone()` implementation might result in subtle and hard to debug issues.
That's a very common mistake about readonly: it's also very useful for
mutable classes. Here is an example that the Symfony Request/Response
objects might follow one day:
class Response
{
public function __construct(
public string $content = '',
public *readonly* ArrayObject $headers = new ArrayObject(),
) {
}
public function __clone() {
$this->headers = clone $this->headers;
}
}
$r1 = new Response();
$r2 = clone $r1;
$r3 = clone($r1, ['headers' => new ArrayObject(['foo' => 'bar'])]);
Note how making "headers" readonly binds the headers to its owner object.
That's very useful since it forbids swapping the headers object by another
one in response objects.
I might lack the Symfony background, but it's not clear to me how preserving the object-identity, but not its contents is important here. I'd like to note, though:
- Given that ArrayObject allows for interior mutability, you could just use `$r3->headers->exchangeArray(['foo' => 'bar']);`.
- Exchanging the entire `ArrayObject` can only happen from within the class itself, since `$headers` is implicitly `protected(set)`.
- You don't actually bind the `$headers` ArrayObject to the owner object, since the ArrayObject might be referenced by multiple Response objects, since passing it in the constructor is legal. The same would be true if it would be `public(set)`.
The issue I highlighted previously is that __clone will "clone
$this->headers" even if the resulting value will be immediately trashed on
the line that creates $r3. This can cascade to nested clones of course
depending on what is being cloned. That's where I see a waste of CPU/mem
"by-design", which is unavoidable at the moment.
The same would be true if you reassign the property after the successful cloning.
About the BC aspect I already shared a proposal that should avoid it. About
the "very limited set of use cases", that's simply not true: people will
start using the new syntax when consuming third-party classes, and authors
of those classes will be "Sorry Outta Luck" when their users will start
complaining about the perf hog I described, with no other options than
telling "don't use that syntax, that perfhog is built-in"...
I don't see that happening, because it would mean that folks suddenly start modifying `public(set) readonly` properties during cloning. In other cases, the RFC does not enable anything new. For non-readonly properties, the user is already able to reassign (see above) and for non-public(set) properties, the class itself controls both `__clone()` and the property itself and thus can take the use-cases into account. On other words, what Volker already said here:
So no situation will be worse than it was before, while some use-cases are
enabled now and some performance critical code will probably stay the same
as it is now.
--------
So my personal suggestion would to be for code that is doing custom things
during cloning and support PHP pre-clone-with as is and only refactor once
8.5 is the minimum requirement, in places where it improves the code.
I didn't answer Tim but he missed that in my example there is a second
argument passed to clone(), which makes it a parse error currently.
In that case you can use:
if (PHP_VERSION_ID >= 80500) {
$b = \clone($a, ['foo' => 'bar']);
}
`\clone` is syntactically valid in PHP 8.0 or higher and a reference to the function `clone` in the global namespace. Incidentally this kind of cross-version compatibility is enabled by making `clone()` a function.
About the second aspect: if a class is designed with the API I gave as
example above (that Response class), then the new syntax is going to be the
only way to swap the headers while cloning. That means there will be cases
where my snippet is going to be useful. I'm fine if you don't want to add
If this syntax is the only way to swap headers while cloning, how would the branch for PHP 8.4 or lower look like? As Volker suggested, you can only switch to clone-with when PHP 8.5 is your minimum version, since you would need the “legacy way of doing things” for older PHP versions (and that one would still remain valid even with the changes this RFC proposes).
So what remains on my side is giving enough care to the deep-cloning case -
see my previous messages of course.
When building a solution it's important to understand the problem in depth and given that we don't currently understand the problem (or even see a problem), we are not in a position to build a solution. Importantly though the RFC leaves sufficient design space for a “Future Scope” solution (e.g. one that passes the $withProperties array to `__clone()` or the original object - which would enable your WeakMap use-case but is orthogonal to clone-with). I have just added that to the “Future Scope” section: PHP: rfc:clone_with_v2
Best regards
Tim Düsterhus