Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:128468
Feedback-ID: i8414410d:Fastmail
Precedence: list
MIME-Version: 1.0
Date: Wed, 13 Aug 2025 11:54:25 -0500
To: "php internals" <internals@lists.php.net>
Message-ID: <870631ac-05a1-429a-94b9-6caa8ab577bf@app.fastmail.com>
In-Reply-To: 
 <CAOWwgpnCzr3UmwQg8LLS2G7gmCar5SCbW9_Ut15OXkAHqdkqaQ@mail.gmail.com>
References: 
 <bHOrFNomgRBQ5FCRh1oK4cXbtPudF22WfgTqtqUIO7SsTMRulRakqBgOvRRFfm8xdDNYFr0AIV6NZ_r5p_h4E5Nz-LZLwZoBw0S9J9NALcA=@gpb.moe>
 <Yy5Yk-VCtT880CdOOXp4RO1DsMnUmDP0n2JJ59nRBlQRTeTv3zQo3ERkMpQkwukArEdpCX7P49_oc9DZbxa0ZfzgO4Q4Z_KrCYpgIgLS7PM=@gpb.moe>
 <CAOWwgpkO6yKrp5oosfRXW0bcxc4-5FEPf4kOuMVBtjqb3PshDw@mail.gmail.com>
 <b2dcca97-7bc3-43f3-b8ac-082966e5dfb7@gmx.de>
 <CAGBsUreXOKpQJjxmNuYsEiSF6hm8usvacSGLW8OZ2779ux1UMg@mail.gmail.com>
 <CAOWwgpk=GqTNB2x=gf0BONZV9mnJ=r0xruwV7Oz2oCrT7bTq5A@mail.gmail.com>
 <b816f64b-76c9-4ed0-9c9f-c4dc0410b887@bastelstu.be>
 <CAOWwgp=bPGQZVaNg2fyJcUCr-tT+CAbUouj7scC8YT2nKG=3zw@mail.gmail.com>
 <CAOWwgpnCzr3UmwQg8LLS2G7gmCar5SCbW9_Ut15OXkAHqdkqaQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [VOTE] Deprecations for PHP 8.5
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From: larry@garfieldtech.com ("Larry Garfield")

On Wed, Aug 13, 2025, at 11:36 AM, Nicolas Grekas wrote:
> Le mer. 13 ao=C3=BBt 2025 =C3=A0 17:34, Nicolas Grekas=20
> <nicolas.grekas+php@gmail.com <mailto:nicolas.grekas%2Bphp@gmail.com>>=20
> a =C3=A9crit :
>>=20
>>=20
>> Le mer. 13 ao=C3=BBt 2025 =C3=A0 14:33, Tim D=C3=BCsterhus <tim@baste=
lstu.be> a =C3=A9crit :
>>> Hi
>>>=20
>>> On 8/13/25 13:58, Nicolas Grekas wrote:
>>> > I wish we had some process to revert that decision. Now is the les=
s costly
>>> > time to do it, even if it's already late.
>>>=20
>>> There is always the option of =E2=80=9Dnot implementing=E2=80=9D the=
 decision. Reverting=20
>>> would mean another RFC that would also require a 2/3 majority.
>>=20
>> I'm happy to do it this way if that's the consensus.
>>=20
>> =20
>>> > 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 im=
pact
>>> > 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/wake=
up would
>>> > still vote the same now?
>>>=20
>>> I already participated in the PR discussion and having slept over it=
, I=20
>>> think it would be helpful to consider both __sleep() and __wakeup()=20
>>> separately in this discussion of whether or not to actually follow=20
>>> through with doing the deprecation.
>>>=20
>>> As I've mentioned in the PR, `__sleep()` is actually broken when=20
>>> inheritance is involved and there's also a trivial way to implement=20
>>> `__serialize()` while preserving compatibility:
>>>=20
>>>      public function __serialize() {
>>>          return get_mangled_object_vars($this);
>>>      }
>>=20
>> Well, the way to introduce __serialize while maintaining compatibilit=
y with both child classes and existing payloads is to copy/paste the sam=
e boilerplate over and over:
>>=20
>>     public function __serialize(): array
>>     {
>>         $data =3D [];
>>         foreach ($this->__sleep() as $key) {
>>             $data[$key] =3D $this->$key;
>>         }
>>         return $data;
>>     }
>>=20
>> And then, this has exactly the same issue regarding private propertie=
s, 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 prope=
rties can already implement __serialize on their side, there's nothing t=
o fix here: __sleep works well, and there's already an upgrade path when=
 it's not enough.
>>=20
>> =20
>>> So while this one might result in some amount of migration work, it'=
s=20
>>> reasonably easy to do.
>>>=20
>>> `__wakeup()` on the other hand is not broken and migrating to=20
>>> `__unserialize()` is non-trivial.
>>>=20
>>> So deprecating `__sleep()` with PHP 8.5 still makes sense to me. For=20
>>> `__wakeup()` I don't have a strong opinion either way and I've also=20
>>> intentionally abstained from voting on this deprecation.
>>=20
>> implementing __unserialize to satisfy a proper upgrade path for paylo=
ad and child classes might be done this way:
>>=20
>>     public function __unserialize(array $data): void
>>     {
>>         foreach ($data as $key =3D> $value) {
>>             $this->{("\0" =3D=3D=3D $key[0] ?? '') ? substr($key, 1 +=
 strrpos($key, "\0")) : $key} =3D $value;
>>         }
>>         $this->__wakeup();
>>     }
>>=20
>> Again, this doesn't work with private properties.
>>=20
>> The previous code was better and safer, and contained less boilerplat=
e.
>>=20
>> I made a draft PR for SYmfony here: https://github.com/symfony/symfon=
y/pull/61407
>> I'm not sure the implementation is correct yet.
>> That's just to show the kind of impact this could have.
>>=20
>> Deprecating only sleep and not wakeup would at least fix the complexi=
ty of the __unserialize replacement.
>> But then, this would fail the purpose of the RFC, which was to get ri=
d of sleep/wakeup altogether.
>>=20
>> This deprecation is a net loss on every aspect.
>>=20
>> Nicolas
>
> Here is the result of my investigation; the kind code that everybody=20
> that cares about  FC/BC for child classes and payloads will have to us=
e=20
> (above PR is up to date):
>
>     public function __serialize(): array
>     {
>         $data =3D [];
>         foreach ($this->__sleep() as $key) {
>             try {
>                 if (($r =3D new \ReflectionProperty($this,=20
> $key))->isInitialized($this)) {
>                     $data[$key] =3D $r->getValue($this);
>                 }
>             } catch (\ReflectionException) {
>                 $data[$key] =3D $this->$key;
>             }
>         }
>         return $data;
>     }
>     public function __unserialize(array $data): void
>     {
>         \Closure::bind(function ($data) {
>             foreach ($data as $key =3D> $value) {
>                 $this->{("\0" =3D=3D=3D $key[0] ?? '') ? substr($key, =
1 +=20
> strrpos($key, "\0")) : $key} =3D $value;
>             }
>             $this->__wakeup();
>         }, $this, static::class)($data);
>     }


Eew. :-)

I support the goal of having only one serialization process for devs to =
think about, but it does need to have a clear and graceful transition fo=
r existing code.  The above code does not strike me as "clear and gracef=
ul transition."

From the RFC:

> Having multiple serializations methods which can be superseded by newe=
r methods is somewhat confusing and add unnecessary complexity to the la=
nguage.=20

I agree, which is why I voted Yes on that part.

> Because the __serialize()/__unserialize() mechanism is a straight up i=
mprovement over __sleep()/__wakeup() we propose to deprecate the latter =
in favour of the former.=20

This claim now appears to be incorrect, given Nicolas' investigation.  (=
Ie, it's not a straight up improvement, nor a drop-in replacement.)

I would be in favor of holding off implementing this deprecation until a=
 better transition process is found, and/or an RFC to reverse it.  (I de=
fer to the RMs on the process they want to follow.)

(And I think this does lend still more weight to Juliette's frequent req=
uest for more robust impact analysis for deprecations.  Not because depr=
ecations are bad, but because we should know what the impact is so we kn=
ow how to mitigate it effectively.)

--Larry Garfield