Le mer. 13 août 2025 à 17:34, Nicolas Grekas <nicolas.grekas+...@gmail.com> a écrit :
> > > Le mer. 13 août 2025 à 14:33, Tim Düsterhus <t...@bastelstu.be> a écrit : > >> Hi >> >> On 8/13/25 13:58, Nicolas Grekas wrote: >> > I wish we had some process to revert that decision. Now is the less >> costly >> > time to do it, even if it's already late. >> >> There is always the option of ”not implementing” the decision. Reverting >> would mean another RFC that would also require a 2/3 majority. >> > > I'm happy to do it this way if that's the consensus. > > > >> > The discussion period failed to raise the points made by Jakub in the >> PR ( >> > https://github.com/php/php-src/pull/19435) and failed a serious impact >> > analysis IMHO. >> > This should be enough to raise a flag and allow us to reconsider. >> > I wonder if people that voted in favor of deprecating __sleep/wakeup >> would >> > still vote the same now? >> >> I already participated in the PR discussion and having slept over it, I >> think it would be helpful to consider both __sleep() and __wakeup() >> separately in this discussion of whether or not to actually follow >> through with doing the deprecation. >> >> As I've mentioned in the PR, `__sleep()` is actually broken when >> inheritance is involved and there's also a trivial way to implement >> `__serialize()` while preserving compatibility: >> >> public function __serialize() { >> return get_mangled_object_vars($this); >> } >> > > Well, the way to introduce __serialize while maintaining compatibility > with both child classes and existing payloads is to copy/paste the same > boilerplate over and over: > > public function __serialize(): array > { > $data = []; > foreach ($this->__sleep() as $key) { > $data[$key] = $this->$key; > } > return $data; > } > > And then, this has exactly the same issue regarding private properties, > without any better way since this has to call __sleep to account for child > classes. > This is just moving the concern elsewhere without any benefit. > On their side, child classes that do have an issue with private properties > can already implement __serialize on their side, there's nothing to fix > here: __sleep works well, and there's already an upgrade path when it's not > enough. > > > >> So while this one might result in some amount of migration work, it's >> reasonably easy to do. >> >> `__wakeup()` on the other hand is not broken and migrating to >> `__unserialize()` is non-trivial. >> >> So deprecating `__sleep()` with PHP 8.5 still makes sense to me. For >> `__wakeup()` I don't have a strong opinion either way and I've also >> intentionally abstained from voting on this deprecation. >> > > implementing __unserialize to satisfy a proper upgrade path for payload > and child classes might be done this way: > > public function __unserialize(array $data): void > { > foreach ($data as $key => $value) { > $this->{("\0" === $key[0] ?? '') ? substr($key, 1 + > strrpos($key, "\0")) : $key} = $value; > } > $this->__wakeup(); > } > > Again, this doesn't work with private properties. > > The previous code was better and safer, and contained less boilerplate. > > I made a draft PR for SYmfony here: > https://github.com/symfony/symfony/pull/61407 > I'm not sure the implementation is correct yet. > That's just to show the kind of impact this could have. > > Deprecating only sleep and not wakeup would at least fix the complexity of > the __unserialize replacement. > But then, this would fail the purpose of the RFC, which was to get rid of > sleep/wakeup altogether. > > This deprecation is a net loss on every aspect. > > Nicolas > Here is the result of my investigation; the kind code that everybody that cares about FC/BC for child classes and payloads will have to use (above PR is up to date): public function __serialize(): array { $data = []; foreach ($this->__sleep() as $key) { try { if (($r = new \ReflectionProperty($this, $key))->isInitialized($this)) { $data[$key] = $r->getValue($this); } } catch (\ReflectionException) { $data[$key] = $this->$key; } } return $data; } public function __unserialize(array $data): void { \Closure::bind(function ($data) { foreach ($data as $key => $value) { $this->{("\0" === $key[0] ?? '') ? substr($key, 1 + strrpos($key, "\0")) : $key} = $value; } $this->__wakeup(); }, $this, static::class)($data); }
