On Thu, Jun 8, 2023, at 6:15 PM, Nicolas Grekas wrote:
>> On Tue, May 30, 2023, at 10:04 PM, Alexandru Pătrănescu wrote:
>> > > On Tue, May 30, 2023, 19:39 Larry Garfield <la...@garfieldtech.com>
>> > wrote:
>> > >
>> > >> On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
>> > >> > To be honest, the current behavior seemed like the natural choice
>> for
>> > >> > me, and I didn't really consider to execute the __clone() method
>> after
>> > >> the
>> > >> > clone assignments.
>> > >> > Do you have a use-case in mind when you need to forward-pass
>> > information
>> > >> to
>> > >> > __clone()?
>> > >>
>> > >> Not a specific one off hand.  It's more a conceptual question.  `with`
>> > has
>> > >> more contextual awareness than __clone(), so it should have "first
>> > crack"
>> > >> at the operation, so that if necessary it can make changes that
>> > __clone()
>> > >> can then respond to.  The inverse doesn't make sense.
>> > >>
>> > >> The only reason for `with` to come after would be to allow `with` to
>> > >> "override" or "undo" something that __clone() did.  Generally
>> speaking,
>> > if
>> > >> you have to undo something you just did, you shouldn't have done it in
>> > the
>> > >> first place, so that's a less compelling combination.
>> > >>
>> > >> This one isn't a deal breaker, but we should be sure to think it
>> through
>> > >> as it's kinda hard to reverse later.
>> > >>
>> > >
>> > > To me so far also it was natural to assume that __clone is first and
>> only
>> > > after that the rest of the operations.
>> > > But `with` operations, be it properties assignment or even a closure,
>> > would
>> > > run in the context of the caller of clone and sometimes this might be
>> run
>> > > not from a method of the cloned class.
>> > >
>> > > An example:
>> > > There is a class that represents persons of a fictive country/planet.
>> > > Each person has many properties but has also a first name and a last
>> name
>> > > and there is a rule: the two names must not start with the same letter.
>> > > Both names cannot be changed as they are defined readonly.
>> > > Creation of new persons can be done using new for new random properties
>> > or
>> > > using clone to preserve existing properties. But in both cases the
>> first
>> > > name and last name are randomly chosen.
>> > > If we want to control the last name value during clone that would be
>> > > possible using the `with` operation but the logic to allocate a first
>> > name
>> > > will only happen in `__clone()`method.
>> > >
>> > > To be able to achieve this we must have __clone last, as there we have
>> > the
>> > > internal validations, operations and also access to private/protected
>> > > members that are not accesible from where clone is being called.
>> > >
>> > > Regards,
>> > > Alex
>> >
>> > I... could not understand that in the slightest.  Can you show it in
>> code?
>> >
>> >
>>
>> Sorry for that. Here you go: https://3v4l.org/JIBoI/rfc#vgit.master
>> If __clone would be first, there is no way to enforce the rule that a
>> person cannot have their first name starting with the same letter as last
>> name.
>>
>
> I'm not sure that's what __clone should be used for.
> This looks like a perfect use case for property hooks, isn't it?
>
> On my side, I would find it unexpected that __clone is called after because
> that would break cloning expectations:
> Imagine you have a __clone that does some deep cloning (a much more typical
> scenario for this construct),
> Let's say __clone() does $this->foo = clone $this->foo;
>
> Imagine now that you do: clone $obj with (foo: $bar)
> I'd expect $obj->foo === $bar after this. But if __clone is called after,
> that won't be true, and I don't see how that could be "fixed" if we swap
> the order. Would you?
>
> Nicolas

Oh, interesting.  There's a nice example case.  To make it a bit more concrete, 
and think aloud... 

class Pet {
  public function __construct(
    public readonly string $name = 'Fluffy',
  ) {}
}

class Address { ... }

class Person {
  public function __construct(
    public readonly Pet $pet,
    public readonly Address $address,
  }

  public function __clone() {
    // Legal now thanks to a previous RFC.
    $this->address = clone ($this->address);
  }
}

$p = new Person(new Pet(), new Address(blah));

// The person gets a new pet.
$p2 = clone $p with (pet: new Pet('Bonzo'));

In this case, the order of operations is irrelevant.

// The person moves.
$newAddr = new Address(whatever);
$p3 = clone $p2 with (address: $newAddr);

In this case, if the `with` happens first, then the new address object is 
cloned needlessly, but that *probably* doesn't hurt anything.  But $newAddr !== 
$p3->address.

If the `__clone()` happens first, then the existing address object is cloned 
needlessly, but that *probably* doesn't hurt anything.  Because the assignment 
happens second, $newAddr === $p3->address.

So I suppose that's a point in favor of __clone() first.

Another option could be to run `with` first, but then pass a list of the keys 
that were modified (probably not their values, just the keys?) to __clone().  
It can then do conditional cloning based on that if appropriate.  I... don't 
know if that's a good idea or not, nor what the BC implications might be.

Máté, your thoughts?

--Larry Garfield

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to