# [RFC] Clone with v2 - Thread: https://externals.io/message/127353 - Original archive: https://news-web.php.net/php.internals/127353 - Started by Volker Dusch on 2025-05-14T13:04:06+00:00 - Messages: 52 - RFC: Clone with v2 (Implemented (PHP 8.5)) — https://externals.io/rfc/clone_with_v2.md - Page 1 of 3: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/127353.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #127353 — Volker Dusch — 2025-05-14T13:04:06+00:00 URL: https://externals.io/message/127353#127353 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 -- 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 --- ## #127358 — Larry Garfield — 2025-05-14T15:38:12+00:00 URL: https://externals.io/message/127353#127358 · In reply to: #127353 > 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 --- ## #127359 — Volker Dusch — 2025-05-14T19:54:47+00:00 URL: https://externals.io/message/127353#127359 · In reply to: #127358 On Wed, May 14, 2025 at 5:40 PM Larry Garfield wrote: > > […] 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. From a user-facing perspective, calling __clone() before makes it easy to know which state the object is in; all parameters are how they've been before the clone operation started and are therefore consistent. When calling __clone() afterward access to the new properties would already be set and the old ones gone, making this potentially harder to work with and harder to adapt existing implementations. So we chose the one that causes fewer potential issues and gotchas. > 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. I've added the output to the var_dump statements to show what works. Standalone examples for the "missing" (set) felt more confusing than showing the difference in context and because its behavior is unaffected by the RFC and mostly documented in case someone isn't familiar with this detail in PHP, which I assume readers here will be. > > […] 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. Thank you for your feedback! I'm looking forward to hearing what other people say as well, but it's nice to get a range of voices giving input here :) Let me just reiterate here that I find the difference to be minor to negligible. IDE/Tooling support needs to be built explicitly either way and is not harder or easier to do, and the ergonomics are quite unaffected in my opinion. So the worst outcome, for me, would be that we get hung up on the syntax here. > One shouldn't be using a named argument for that anyway. I'm not sure what you mean by that. Just so we're clear: `...["__object" => "foo"]` would also not work with the current implementation, as that's the same thing as a `__object: $value` named argument. (Should we decide to name the first parameter __object here). Sorry if I'm misunderstanding. Kind 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 --- ## #127360 — Matthew Weier O'Phinney — 2025-05-14T20:06:18+00:00 URL: https://externals.io/message/127353#127360 · In reply to: #127353 On Wed, May 14, 2025 at 8:06 AM Volker Dusch wrote: > 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. > The only question that arose for me is: what happens if a property name is provided to `clone()` that does not exist in the class definition; what will be the behavior at that time? Will an exception or error be thrown? If so, will it be a new one, or an existing one? -- Matthew Weier O'Phinney mweierophinney@gmail.com https://mwop.net/ he/him --- ## #127362 — tim@bastelstu.be — 2025-05-14T21:00:01+00:00 URL: https://externals.io/message/127353#127362 · In reply to: #127360 Hi Am 2025-05-14 22:06, schrieb Matthew Weier O'Phinney: > The only question that arose for me is: what happens if a property name > is > provided to `clone()` that does not exist in the class definition; what > will be the behavior at that time? Will an exception or error be > thrown? If > so, will it be a new one, or an existing one? It's mentioned in the "Technical details" section: > Property assignments are made just as a regular assignment would be It literally goes through the same code path. Thus a dynamic property will be created (and the associated warnings triggered). You can also see that in test `Zend/tests/clone/clone_with_002.phpt` of the implementation: > Deprecated: Creation of dynamic property C::$c is deprecated in %s on > line %d The internal implementation is roughly equivalent to: $cloned = clone $object; foreach ($withProperties as $key => $value) { $cloned->{$key} = $value; } return $cloned; Just with the exception that writing readonly properties is allowed on the clone (unless already touched by `__clone()`). Best regards Tim Düsterhus --- ## #127364 — Larry Garfield — 2025-05-14T22:04:46+00:00 URL: https://externals.io/message/127353#127364 · In reply to: #127362 > The internal implementation is roughly equivalent to: > > $cloned = clone $object; > foreach ($withProperties as $key => $value) { > $cloned->{$key} = $value; > } > return $cloned; > > Just with the exception that writing readonly properties is allowed on > the clone (unless already touched by `__clone()`). Subtle point here. If the __clone() method touches a readonly property, does that make the property inaccessible to the new clone-with? Or is it a separate "unlock scope"? I would expect the latter. A single unlock block would be confusing to me. --Larry Garfield --- ## #127365 — Larry Garfield — 2025-05-14T22:08:47+00:00 URL: https://externals.io/message/127353#127365 · In reply to: #127359 > > > […] > > From a user-facing perspective, calling __clone() before makes it easy > to know which state the object is in; all parameters are how they've > been before the clone operation started and are therefore consistent. > When calling __clone() afterward access to the new properties would > already be set and the old ones gone, making this potentially harder to > work with and harder to adapt existing implementations. So we chose the > one that causes fewer potential issues and gotchas. Please include this in the RFC. > Let me just reiterate here that I find the difference to be minor to > negligible. IDE/Tooling support needs to be built explicitly either way > and is not harder or easier to do, and the ergonomics are quite > unaffected in my opinion. So the worst outcome, for me, would be that > we get hung up on the syntax here. I think the current syntax is optimal. > > […] > > I'm not sure what you mean by that. Just so we're clear: > `...["__object" => "foo"]` would also not work with the current > implementation, as that's the same thing as a `__object: $value` named > argument. (Should we decide to name the first parameter __object here). > Sorry if I'm misunderstanding. The concern is someone calling clone($foo, object: new Thingie()), which would complain that "object" is defined twice. Making the first argument named __object sidesteps that issue. If someone then calls clone(prop: 'new val', __object: $foo), that... I suppose would technically compile, but one should never do that, and I am perfectly happy if that breaks. Similar for if someone called clone(prop: 'new val', object: $foo) if we didn't rename that parameter. No one should be doing that and I am fine if that breaks. --Larry Garfield --- ## #127367 — Volker Dusch — 2025-05-14T23:26:13+00:00 URL: https://externals.io/message/127353#127367 · In reply to: #127360 On Wed, May 14, 2025 at 10:06 PM Matthew Weier O'Phinney < mweierophinney@gmail.com> wrote: > The only question that arose for me is: what happens if a property name is provided to `clone()` that does not exist in the class definition; what will be the behavior at that time? Will an exception or error be thrown? If so, will it be a new one, or an existing one? Thank you for the question. I had that one myself looking at the initial implementation and forgot to write that down more explicitly. Tim already answered and linked the test, but I've also added an example to the RFC. -- 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 --- ## #127369 — Stephen Reay — 2025-05-15T06:22:45+00:00 URL: https://externals.io/message/127353#127369 · In reply to: #127367 > > > > > > […] > > Thank you for the question. I had that one myself looking at the initial implementation and forgot to write that down more explicitly. > > Tim already answered and linked the test, but I've also added an example to the RFC. > > -- > 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 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. Cheers Stephen --- ## #127372 — Andreas Hennings — 2025-05-15T09:32:04+00:00 URL: https://externals.io/message/127353#127372 · In reply to: #127369 [..] > > > 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? What exactly should happen then? Would the __clone() method be responsible for assigning those properties? Or does the __clone() method get the chance to alter the values before they are assigned? (this would mean they have to be passed by reference) I think this last option is the best, because the values in the array can be changed without any readonly constraints. Another option I was thinking of would be to call __clone() after the changes are applied, and pass both the original object and the array of changes as first parameter. But I think this is a dead end. -- Andreas > > 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. > > > > Cheers > > Stephen --- ## #127374 — Stephen Reay — 2025-05-15T11:56:15+00:00 URL: https://externals.io/message/127353#127374 · In reply to: #127372 > > > On Thu, 15 May 2025 at 08:24, Stephen Reay wrote: > [..] > > […] > > What exactly should happen then? > Would the __clone() method be responsible for assigning those properties? > Or does the __clone() method get the chance to alter the values before > they are assigned? > (this would mean they have to be passed by reference) > I think this last option is the best, because the values in the array > can be changed without any readonly constraints. > > Another option I was thinking of would be to call __clone() after the > changes are applied, and pass both the original object and the array > of changes as first parameter. > But I think this is a dead end. > > -- Andreas > > > […] > I would suggest that the __clone method should be directly responsible for making any changes, just as it is now when it comes to deep cloning or resetting values. Yes I realise it means developers need to then opt in and provide the functionality to support `clone $foo with(bar: "baz")` or whatever syntax is used. If the properties are public properties, there's nothing stopping someone writing their own clone_with() in userland now; If someone is using readonly properties I'd suggest they want to specifically manage updates to those properties themselves anyway. Additionally it means "clone with" would be usable for non-public properties at the discretion of the developer writing their code. The mental model is also very clear with this: copy the object in memory, and then call __clone(), with the arguments passed to the clone action - which may be none in the case of code that doesn't accept any clone arguments. The only change from the current model is that it *may* be passing arguments. Cheers Stephen --- ## #127375 — Rob Landers — 2025-05-15T12:14:48+00:00 URL: https://externals.io/message/127353#127375 · In reply to: #127374 > > > > > […] > > I would suggest that the __clone method should be directly responsible for making any changes, just as it is now when it comes to deep cloning or resetting values. > > Yes I realise it means developers need to then opt in and provide the functionality to support `clone $foo with(bar: "baz")` or whatever syntax is used. > > If the properties are public properties, there's nothing stopping someone writing their own clone_with() in userland now; If someone is using readonly properties I'd suggest they want to specifically manage updates to those properties themselves anyway. > > Additionally it means "clone with" would be usable for non-public properties at the discretion of the developer writing their code. > > The mental model is also very clear with this: copy the object in memory, and then call __clone(), with the arguments passed to the clone action - which may be none in the case of code that doesn't accept any clone arguments. The only change from the current model is that it *may* be passing arguments. > > Cheers > > Stephen > When I was working on Records, one of the important things was the ability to "hook into" the update that was about to happen. 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. — Rob --- ## #127377 — tim@bastelstu.be — 2025-05-15T13:53:46+00:00 URL: https://externals.io/message/127353#127377 · In reply to: #127364 Hi Am 2025-05-15 00:04, schrieb Larry Garfield: > Subtle point here. If the __clone() method touches a readonly > property, does that make the property inaccessible to the new > clone-with? Yes. Quoting from the RFC: > The currently linked implementation “locks” a property if it modified > within __clone(), if this is useful is up for debate. - > A single unlock block would be confusing to me. We’ve implemented it like that, because it felt most in line with what was decided in https://wiki.php.net/rfc/readonly_amendments#proposal_2readonly_properties_can_be_reinitialized_during_cloning, which says: > Reinitialization of each property is possible once and only once: We expect “public(set) readonly” + “__clone()” to be rare and from within the class, the author knows how their `__clone()` implementation works and can make sure it is compatible with whatever properties they might want to update during cloning. The lack of “use cases” is the primary reason we made the more conservative choice, but we are not particularly attached to this specific behavior. Best regards Tim Düsterhus --- ## #127378 — Larry Garfield — 2025-05-15T14:03:07+00:00 URL: https://externals.io/message/127353#127378 · In reply to: #127369 > 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. --Larry Garfield --- ## #127379 — Larry Garfield — 2025-05-15T14:16:59+00:00 URL: https://externals.io/message/127353#127379 · In reply to: #127377 > Hi > > Am 2025-05-15 00:04, schrieb Larry Garfield: > > […] > > Yes. Quoting from the RFC: > > > […] > > - > > > […] > > We’ve implemented it like that, because it felt most in line with what > was decided in > https://wiki.php.net/rfc/readonly_amendments#proposal_2readonly_properties_can_be_reinitialized_during_cloning, > which says: > > > […] > > We expect “public(set) readonly” + “__clone()” to be rare and from > within the class, the author knows how their `__clone()` implementation > works and can make sure it is compatible with whatever properties they > might want to update during cloning. The lack of “use cases” is the > primary reason we made the more conservative choice, but we are not > particularly attached to this specific behavior. > > Best regards > Tim Düsterhus Fair. I could probably think of a use case if I tried hard, but I can't think of one off hand. It's just surprising in the abstract. I suppose making it a single unlock scope leaves open the option to split it in two later, but the reverse is not true. I won't fight for this one, just noting it as surprising. --Larry Garfield --- ## #127380 — tim@bastelstu.be — 2025-05-15T15:32:11+00:00 URL: https://externals.io/message/127353#127380 · In reply to: #127375 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 --- ## #127382 — Andreas Hennings — 2025-05-15T16:49:11+00:00 URL: https://externals.io/message/127353#127382 · In reply to: #127374 > > > > > > […] > > I would suggest that the __clone method should be directly responsible for making any changes, just as it is now when it comes to deep cloning or resetting values. > > Yes I realise it means developers need to then opt in and provide the functionality to support `clone $foo with(bar: "baz")` or whatever syntax is used. I don't really like this. It would mean that if you add an empty __clone() method, it would prevent all of the automatic setting of values. > > If the properties are public properties, there's nothing stopping someone writing their own clone_with() in userland now; If someone is using readonly properties I'd suggest they want to specifically manage updates to those properties themselves anyway. But they already do that in the ->withXyz() methods. A public non-readonly property can be set from anywhere without validation or clean-up, so by default the __clone() method would want to leave it alone. A readonly or non-public property can only be initialized from within the class, so the ->withSomething() method would be the place for cleanup and validation. The default behavior of an empty __clone() method should therefore be to just allow all of the properties being set as they would be without a __clone() method. > > Additionally it means "clone with" would be usable for non-public properties at the discretion of the developer writing their code. This is already the case, because that "clone with" for non-public properties can only happen from within methods of the same class (hierarchy). -- Andreas > > The mental model is also very clear with this: copy the object in memory, and then call __clone(), with the arguments passed to the clone action - which may be none in the case of code that doesn't accept any clone arguments. The only change from the current model is that it *may* be passing arguments. > > Cheers > > Stephen --- ## #127383 — Volker Dusch — 2025-05-15T17:07:26+00:00 URL: https://externals.io/message/127353#127383 · In reply to: #127365 On Thu, May 15, 2025 at 12:10 AM Larry Garfield wrote: > Please include this in the RFC. Done > The concern is someone calling clone($foo, object: new Thingie()), which would complain that "object" is defined twice. Making the first argument named __object sidesteps that issue > If someone then calls clone(prop: 'new val', __object: $foo), that... I suppose would technically compile, but one should never do that, and I am perfectly happy if that breaks. Similar for if someone called clone(prop: 'new val', object: $foo) if we didn't rename that parameter. No one should be doing that and I am fine if that breaks Yes. I think we're on the same page here. But just to be clear, I'll restate it again with my own words: Whatever the first parameter (the to-be-cloned object) of the `clone()` function is called will be completely inaccessible as a property name. clone($foo, __object: "bar") will fail (Named parameter overwrites previous argument) clone($foo, ...["__object" => "bar"]) will also fail. While `clone(prop: 'new val', __object: $foo)` just works. For example, using the current implementation where the parameter is named `object`: ``` class Foo { public int $bar; } $x = new Foo(); var_dump(clone(bar: 5, object: $x)); // object(Foo)#2 (1) { // ["bar"]=> // int(5) // } ``` -- 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 --- ## #127384 — Stephen Reay — 2025-05-15T17:56:46+00:00 URL: https://externals.io/message/127353#127384 · In reply to: #127382 > > > > > […] > > I don't really like this. > It would mean that if you add an empty __clone() method, it would > prevent all of the automatic setting of values. > To repeat myself: yes. It requires the class developer to opt-in to supporting this feature of the language. Just the same way promoted constructor parameters are opt-in, and don't just create a bunch of properties with the names the caller specified to `new`. An empty or missing clone method would mean it behaves **exactly the way it does now**. > > […] > > But they already do that in the ->withXyz() methods. > > A public non-readonly property can be set from anywhere without > validation or clean-up, so by default the __clone() method would want > to leave it alone. Again, what happens when cloning should be entirely the purview of the developer that writes the class. > A readonly or non-public property can only be initialized from within > the class, so the ->withSomething() method would be the place for > cleanup and validation. > The default behavior of an empty __clone() method should therefore be > to just allow all of the properties being set as they would be without > a __clone() method. > I agree that no __clone and an empty __clone should behave the same way. But as I said, I believe they should behave the same way as they do *now*, until the developer opts in to support cloning with new values. > > […] > > This is already the case, because that "clone with" for non-public > properties can only happen from within methods of the same class > (hierarchy). > > -- Andreas > > > […] --- ## #127385 — Rob Landers — 2025-05-15T19:56:30+00:00 URL: https://externals.io/message/127353#127385 · In reply to: #127380 > Hi > > Am 2025-05-15 14:14, schrieb Rob Landers: > > […] > > 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 --- ## #127386 — Larry Garfield — 2025-05-15T20:11:13+00:00 URL: https://externals.io/message/127353#127386 · In reply to: #127385 > > > […] > > Hey Tim, > > > […] > > 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 --- ## #127387 — Dmitry Derepko — 2025-05-16T08:40:01+00:00 URL: https://externals.io/message/127353#127387 · In reply to: #127383 --- ## #127388 — Rob Landers — 2025-05-16T09:32:58+00:00 URL: https://externals.io/message/127353#127388 · In reply to: #127386 > > > […] > > 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 > Yeah, the validation won't be too automatable (ie, in a base class) without at least having reusable hooks. It's important to be mindful that there are approximately three different paradigms when it comes to validating objects (and they apply to frameworks differently) in PHP. -- validate on serialization -- This paradigm is mostly used in symfony via doctrine/serializer. An object is allowed to be in an "invalid" state and is usually constructed in a "zero" state, and then state is applied over the lifetime of the application. Only during serialization to the wire is it usually validated. So, it usually looks something like this: $user = new User(); $user->name = "Rob" $user->id = 123; When you receive an object in a function, you will likely have to validate that specific properties are set before operating on it, otherwise you can end up with bugs. The nice thing about this style is that you can build up an object's state over a longer period (such as processing a form input or a database query result). -- validate on construction -- This paradigm is commonly used in value objects and more functional-style PHP. The idea is that an object must be valid by the time it is constructed and does not allow partially formed objects to exist. All properties are validated in the constructor, and invalid combinations throw exceptions immediately. This tends to lead to more robust code, particularly when the object represents a meaningful invariant (e.g., `Money`, `EmailAddress`, `Uuid`). Here's what it looks like: $user = new User(name: "Rob", id: 123); The dowside is that its harder to build objects piece by piece and usually requires factories, DTOs, and builder patterns for times when not all the data is available upfront. This approach is usually seen in functional, DDD (domain driven design), or strict typing contexts and plays nicely with immutability. This style is less-common in popular PHP frameworks like Symfony and Laravel, which tend to favor more flexible object construction. -- validate on mutation -- This paradigm is popular in both Symfony and Laravel and favors an active-record-esqe approach. In this paradigm, each mutator (setter) is responsible for ensuring the property remains valid at the point of modification. The downside is that inter-property constraints require redundant checks that can be difficult to maintain. The nice thing is that they're easy to enforce. Of course, these can be mixed-and-matched as needed/desired. The downside with the cloning method here is that it really puts "validation on construction" on a back foot. Developers will likely have absolutely no way to perform validation without rewriting their entire class structures and will make validation during construction basically impossible for immutable objects. — Rob --- ## #127389 — Rowan Tommins [IMSoP] — 2025-05-16T18:44:55+00:00 URL: https://externals.io/message/127353#127389 · In reply to: #127384 > I agree that no __clone and an empty __clone should behave the same > way. But as I said, I believe they should behave the same way as they > do *now*, until the developer opts in to support cloning with new values. I think what Andreas is saying is that the behaviour of this: $foo = clone($bar, someProperty: 42); Should be consistent with the existing behaviour of this: $foo = clone $bar; $bar->someProperty = 42; An empty or missing __clone method won't prevent `someProperty` being assigned in the existing case, and for everything other than readonly properties, the new syntax is purely sugar for the existing one. If I understand the RFC, the only change in behaviour is for readonly properties, which are "unlocked" during the "clone with" process. That means that if they were previously validated only in the constructor, this syntax can put the object in an unexpected state. 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? -- Rowan Tommins [IMSoP] --- ## #127390 — Nicolas Grekas — 2025-05-16T19:39:51+00:00 URL: https://externals.io/message/127353#127390 · In reply to: #127377 Le jeu. 15 mai 2025 à 15:55, Tim Düsterhus a écrit : > Hi > > Am 2025-05-15 00:04, schrieb Larry Garfield: > > […] > > Yes. Quoting from the RFC: > > > […] > > - > > > […] > > We’ve implemented it like that, because it felt most in line with what > was decided in > > https://wiki.php.net/rfc/readonly_amendments#proposal_2readonly_properties_can_be_reinitialized_during_cloning, > > which says: > > > […] > > We expect “public(set) readonly” + “__clone()” to be rare and from > within the class, the author knows how their `__clone()` implementation > works and can make sure it is compatible with whatever properties they > might want to update during cloning. The lack of “use cases” is the > primary reason we made the more conservative choice, but we are not > particularly attached to this specific behavior. > > Being able to update a readonly property even if __clone already touched it looks critical to me because otherwise, it'd mean that adding a __clone method after publishing a first version of some class that has no __clone method would be a BC break. Nicolas --- Next page: https://externals.io/message/127353.md?page=2