[RFC] Clone with v2

Hi

Am 2025-05-15 14:14, schrieb Rob Landers:

For example, if you have a Money type, you'd want to be able to ensure
it cannot be negative when updating via with(). This is super
important for ensuring constraints are met during the clone.

That's why the assignments during cloning work exactly like regular
property assignments, observing visibility and property hooks.

The only tiny difference is that an “outsider” is able to change a
public(set) readonly property after a __clone() method ran to
completion and relied on the property in question not changing on the
cloned object after it observed its value. This seems not to be
something relevant in practice, because why would the exact value of the
property only matter during cloning, but not at any other time?

Best regards
Tim Düsterhus

Hey Tim,

why would the exact value of the
property only matter during cloning, but not at any other time?

For example, queueing up patches to store/db to commit later; during
the clone, it may register various states to ensure the patches are
accurate from that point. That's just one example, though, and it
suggests calling __clone before setting the values is the right
answer.

I think Larry's idea of just using hooks for validation is also pretty
good. As Larry said, the only thing you can really do is throw an
exception, and the same would be true in a constructor as well.

— Rob

The limit of hooks is that they're single-property. So depending on how your derived properties are implemented, it may be insufficient. I could easily write such an example (the hooks RFC included some), but how contrived they are, I don't know.

--Larry Garfield

On Fri, May 16, 2025 at 8:46 PM Rowan Tommins [IMSoP] imsop.php@rwec.co.uk
wrote:

However, readonly properties are "protected(set)" by default, so the
situations where this can happen are actually quite limited:

• Code inside the class itself (private scope) can reasonably be
  considered to be "opting in" to the feature it's using.
• Code in a sub-class (protected scope) can by default over-ride the
  constructor anyway.
• Code outside the class (public scope) will fail unless the property is
  explicitly "readonly public(set)", which would be pointless if it was
  always initialised in the constructor.

So the only case I can think of where something surprising could happen is:

1. A public or protected readonly property is initialised in a
   constructor marked "final"
2. A sub-class adds code that uses "clone with" to set that property to
   a new value

The question then is, how worried are we about that scenario?

Your summary matches my ideas around the topic and I've updated the RFC [1]
to better clarify that we consider touching __clone to be out of scope.

From our perspective, we are not worried about the Points (1&2) you raised
and are not interested in solving this by providing parameters to __clone
as the additional cost, issues, and complexity are not a worthwhile
tradeoff in our eyes.

Kind Regards,
Volker

[1] https://news-web.php.net/php.internals/127397

--
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

Le lun. 19 mai 2025 à 19:06, Andreas Hennings andreas@dqxtech.net a
écrit :

On Mon, 19 May 2025 at 17:13, Nicolas Grekas
nicolas.grekas+php@gmail.com wrote:

Le lun. 19 mai 2025 à 16:30, Andreas Hennings andreas@dqxtech.net a
écrit :

On Fri, 16 May 2025 at 21:59, Nicolas Grekas
nicolas.grekas+php@gmail.com wrote:

Le jeu. 15 mai 2025 à 16:06, Larry Garfield larry@garfieldtech.com
a écrit :

I may be missing something here..

So far the issues are "how do we deal with a parameter for the
actual
object, vs new properties to apply", "should __clone be called
before
or after the changes" and "this won't allow regular readonly
properties
to be modified".

Isn't the previous suggestion of passing the new property arguments
directly to the __clone method the obvious solution to all three
problems?

There's no potential for a conflicting property name, the
developer can
use the new property values in the order they see fit relative to
the
logic in the __clone call, and it's inherently in scope to write
to any
(unlocked during __clone) readonly properties.

I did some exploratory design a few years ago on this front, looking
at the implications of different possible syntaxes.

https://peakd.com/hive-168588/@crell/object-properties-part-2-examples

What that article calls "initonly" is essentially what became
readonly. The second example is roughly what this RFC would look like if
the extra arguments were passed to __clone(). As noted in the article, the
result is absolutely awful.

Auto-setting the values while using the clone($object, ...$args)
syntax is the cleanest solution. Given that experimentation, I would not
support an implementation that passes args to __clone and makes the
developer figure it out. That just makes a mess.

Rob makes a good point elsewhere in thread that running __clone()
afterward is a way to allow the object to re-inforce validation if
necessary. My concern is whether the method knows it needs to do the extra
validation or not, since it may be arbitrarily complex. It would also
leave no way to reject the changes other than throwing an exception, though
in fairness the same is true of set hooks. Which also begs the question of
whether a set hook would be sufficient that __clone() doesn't need to do
extra validation? At least in the typical case?

One possibility (just brainstorming) would be to update first, then
call __clone(), but give clone a new optional arg that just tells it what
properties were modified by the clone call. It can then recheck just those
properties or ignore it entirely, as it prefers. If that handles only
complex cases (eg, firstName was updated so the computed fullName needs to
be updated) and set hooks handle the single-property ones, that would
probably cover all bases reasonably well.

I like where this is going but here is a variant that'd be even more
capable:

we could pass the original object to __clone.

My proposal earlier was to pass the original object and the values
that were passed to the clone call, by reference.

And this would happen before those values are assigned to the object.

class MyClass {
public function __construct(
public readonly int $x,
public readonly int $y,
public readonly int $z,
) {}
public function __clone(object $original, array &$values): void {
// Set a value directly, and modify it.
if (isset($values['x'])) {
$this->x = $values['x'] * 10;
// Prevent that the same property is assigned again.
unset($values['x']);
}
}
}

$obj = new C(5, 7, 9);
$clone = clone($obj, x: 2, y: 3);
assert($clone->x === 20); // x was update in __clone().
assert($clone->y === 3); // y was auto-updated after __clone().
assert($clone->z === 9); // z was not touched at all.

I'm not sure I understand, there might be missing bits to your idea, eg
where is visibility enforced? why is pass-by-ref needed at all?
Pass-by-ref makes me think this is a bad idea already :)

Maybe we are looking at different problems to be solved.
To me, the main questions are:

• Could a __clone() method want to behave differently depending which
  property values are passed with the clone call?

Definitely yes, at least to skip triggering a costly deep cloning operation.

• Can there be conflicts between operations we would normally do in

__clone() and values passed to __clone()?

I don't see any. BTW, I had a look at what is done within __clone methods
in Symfony, and all implementations fall down into 4 categories:

• incrementing some counter for tracking management purposes
• resetting the state of the clone (at least for some transient properties)
• deep-cloning
• forbidding clone at all (throwing or making the method private)

• Should we prevent or allow double-write to a readonly property? That
  is, if one write happens in __clone(), and the other write happens
  automatically due to the property value passed to clone(..).

This question doesn't really make sense IMHO. What matters is readonly
semantics, which must be preserved. The fact that there are two or more
steps to achieve the target state doesn't matter.

And to clarify my proposal:

• Everything is the same as in the RFC (except points below)
• Same as in the RFC, the __clone() method is called after the
  original object values have been copied over, but before any of the
  property values passed as arguments to clone($obj, ...$values) are
  assigned.
• Same as in the RFC, the values passed to clone($obj, ...$values) are
  assigned automatically after the __clone() method.
• Unlike the RFC, the __clone() method can see (and validate) the
  values that were passed to __clone($obj, ...$values)
• Unlike the RFC, the __clone() method can alter the values passed
  to __clone($obj, ...$values) before they are assigned.
• As in the RFC, readonly properties can be written only once on
  clone. The __clone() method can prevent a double write by unsetting
  that key in $values.

Thanks for the clarification.
About this last item: the RFC has been updated on this topic.
Also preventing double-writes by unsetting a by-ref array looks terrible,
no chance this can be the best API, sorry :)

Consequence:
By leaving the __clone() method empty, all values are assigned
automatically, as in the RFC.

where is visibility enforced?

Exactly as in the RFC.
When clone($obj, ...$values) is called, and before __clone() is
invoked, php has to verify which of the properties are legal to be
updated in this way, based on property visibility and readonly status,
and depending on the scope from which it is called.

Actually this raises some questions that I did not think of before:

• After __clone() is invoked, does php need to validate again?
• What happens to private properties that are not accessible from the
  scope of the __clone() method? Are they also passed in the $values
  array?

That's definitely an issue with any approach that relies on passing
property names.
This doesn't happen of course if we pass only the original object and rely
on === to know what changed.

Also, WDYT of my simpler proposal itself? Wouldn't it cover all use
cases?

First, I want to make sure I understand correctly:

• Unlike the RFC, we want to call __clone() after the values from
  clone($obj, ...$values) are assigned.
  (I assume this because otherwise the two objects would be identical,
  and the original object would be useless)
• Unlike the RFC, we pass the original object as a parameter to __clone().

You've got it right, yes.

Tbh I am not sure if the use cases I think of are relevant or not :)
I mostly think of it in terms of functional completeness, without
trying to speculate why a developer would want to do this or that
during __clone().

Let's look at the "benefits" section from your earlier mail.

The benefits I see:

• Allow implementing this validation logic you're talking about.

Having __clone() called after the values are assigned, as you propose,
makes it possible to run an integrity check on the object itself. On
the other hand, this may leave us with a short moment of possibly
"bad" property values.

Having __clone() called before the values are assigned means we have

to validate the values array, not the object.

This concern matters only if that temporary state is observable from the
outside. Which isn't going to be the case.

• Allow to skip deep-cloning of already updated properties (that's a
  significant drawback of the current proposal - deep cloning before setting
  is a potential perf/mem hog built into the engine) : guarding deep-cloning
  with a strict comparison would be enough.

With __clone() called after the values are assigned, and with access
to the original object, we can check $this->prop === $old->prop to see
whether a specific property was updated (and therefore should not be
deep cloned).
With __clone() called before the values are assigned, and with access
to the values array, we would check isset($values['prop']) instead.
(or really array_key_exists())
In general this would produce the same result, unless a property was
assigned the same value that it had before.

Another question is about "dependent properties", e.g. for lazily

filled calculated values.
With access to the old object we would have to check $this->prop !==
$old->prop to then see which dependent properties need to be reset or
recalculated.
With access to the values we would check isset($values['prop']) instead.

• Allow implementing WeakMap that are able to clone weak-properties as
  objects are cloned.

I don't feel informed or qualified to talk about this one..

So, with __clone() called after and with the original object, we
compare old and new when looking for a specific property.
With access to an array of updated (or to be updated) properties, we
could iterate over the changes, or we could check whether the
changelist is empty, which is less obvious to do by comparing old and
new instance.

I think the above concern with private properties and the strange by-ref
API both rule out the approach.
The one I propose actually works and is simpler.

Note that I don't see the need for operations like in your example
(transforming a value while cloning).

This looks like the job of a setter or a hook instead, not a cloner.

Tbh, most of the classes I wrote that would benefit from a "clone
with" did not really need a __clone() method, because everything in
there was already immutable.
To me, the scenarios where we want both are quite speculative, so this
is why my examples might not be the most realistic.

Think deep-cloning. This does require __clone + has to support clone-with.

The benefits I see:

• Allow implementing this validation logic you're talking about.
• Allow to skip deep-cloning of already updated properties (that's a
  significant drawback of the current proposal - deep cloning before setting
  is a potential perf/mem hog built into the engine) : guarding deep-cloning
  with a strict comparison would be enough.
• Allow implementing WeakMap that are able to clone weak-properties
  as objects are cloned.

On this last aspect, I think it's new to the discussion but it's
something I've always found very limiting when using weak-map: let's say
some metadata are stored about object $foo in a weakmap, it's currently not
possible to track those metadata across clones without using some nasty
tricks. If __clone were given the original object, it's be easy to
duplicate meta-data from $foo to $clone.

I have just one concern significant with adding an argument to
__clone: it'dbe a BC break to mandate this argument at the declaration
level, and adding one right now generates an error with current versions of
PHP.
However, I think we could (and should if confirmed) provide some
FC/BC layer by allowing one to use func_get_args() in __clone. The engine
could then verify at compile time that __clone has either one non-nullable
"object" argument, or zero.

This seems reasonable.

I'd be happy to know what Volker and Tim think about this? I read they
excluded any change to __clone in the RFC, but I think it should still be
possible to discuss this, especially if it provides the path to the desired
solution.

Nicolas