Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:128466
Precedence: list
MIME-Version: 1.0
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>
In-Reply-To: <b816f64b-76c9-4ed0-9c9f-c4dc0410b887@bastelstu.be>
Date: Wed, 13 Aug 2025 17:34:02 +0200
Message-ID: <CAOWwgp=bPGQZVaNg2fyJcUCr-tT+CAbUouj7scC8YT2nKG=3zw@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [VOTE] Deprecations for PHP 8.5
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: Kamil Tekiela <tekiela246@gmail.com>, "Christoph M. Becker" <cmbecker69@gmx.de>, 
	"Gina P. Banyard" <internals@gpb.moe>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a01130063c40e3e9"
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000a01130063c40e3e9
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Le mer. 13 ao=C3=BBt 2025 =C3=A0 14:33, Tim D=C3=BCsterhus <tim@bastelstu.b=
e> a =C3=A9crit :

> 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 =E2=80=9Dnot implementing=E2=80=9D the deci=
sion. 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 P=
R
> (
> > 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 =3D [];
        foreach ($this->__sleep() as $key) {
            $data[$key] =3D $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 =3D> $value) {
            $this->{("\0" =3D=3D=3D $key[0] ?? '') ? substr($key, 1 +
strrpos($key, "\0")) : $key} =3D $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

--000000000000a01130063c40e3e9
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=
=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">Le=C2=A0mer. 13 ao=
=C3=BBt 2025 =C3=A0=C2=A014:33, Tim D=C3=BCsterhus &lt;<a href=3D"mailto:ti=
m@bastelstu.be" target=3D"_blank">tim@bastelstu.be</a>&gt; a =C3=A9crit=C2=
=A0:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px=
 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi<br>
<br>
On 8/13/25 13:58, Nicolas Grekas wrote:<br>
&gt; I wish we had some process to revert that decision. Now is the less co=
stly<br>
&gt; time to do it, even if it&#39;s already late.<br>
<br>
There is always the option of =E2=80=9Dnot implementing=E2=80=9D the decisi=
on. Reverting <br>
would mean another RFC that would also require a 2/3 majority.<br></blockqu=
ote><div><br></div><div>I&#39;m happy to do it this way if that&#39;s the c=
onsensus.</div><div><br></div><div>=C2=A0</div><blockquote class=3D"gmail_q=
uote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,2=
04);padding-left:1ex">
&gt; The discussion period failed to raise the points made by Jakub in the =
PR (<br>
&gt; <a href=3D"https://github.com/php/php-src/pull/19435" rel=3D"noreferre=
r" target=3D"_blank">https://github.com/php/php-src/pull/19435</a>) and fai=
led a serious impact<br>
&gt; analysis IMHO.<br>
&gt; This should be enough to raise a flag and allow us to reconsider.<br>
&gt; I wonder if people that voted in favor of deprecating __sleep/wakeup w=
ould<br>
&gt; still vote the same now?<br>
<br>
I already participated in the PR discussion and having slept over it, I <br=
>
think it would be helpful to consider both __sleep() and __wakeup() <br>
separately in this discussion of whether or not to actually follow <br>
through with doing the deprecation.<br>
<br>
As I&#39;ve mentioned in the PR, `__sleep()` is actually broken when <br>
inheritance is involved and there&#39;s also a trivial way to implement <br=
>
`__serialize()` while preserving compatibility:<br>
<br>
=C2=A0 =C2=A0 =C2=A0public function __serialize() {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return get_mangled_object_vars($this);<br=
>
=C2=A0 =C2=A0 =C2=A0}<br></blockquote><div><br></div><div>Well, the way to =
introduce __serialize while maintaining=C2=A0compatibility with both child =
classes and existing payloads is to copy/paste the same boilerplate over an=
d over:</div><div><br></div><div>=C2=A0 =C2=A0 public function __serialize(=
): array<br>=C2=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 $data =3D [];<br=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 foreach ($this-&gt;__sleep() as $key) {<br>=C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $data[$key] =3D $this-&gt;$key;<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 return $data;<=
br>=C2=A0 =C2=A0 }</div><div><br></div><div>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.</div><div>This is just mo=
ving the concern elsewhere without any benefit.</div><div>On their side, ch=
ild classes that do have an issue with private properties can already imple=
ment __serialize on their side, there&#39;s nothing to fix here: __sleep wo=
rks well, and there&#39;s already an upgrade path when it&#39;s not enough.=
</div><div><br></div><div>=C2=A0</div><blockquote class=3D"gmail_quote" sty=
le=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddi=
ng-left:1ex">

So while this one might result in some amount of migration work, it&#39;s <=
br>
reasonably easy to do.<br>
<br>
`__wakeup()` on the other hand is not broken and migrating to <br>
`__unserialize()` is non-trivial.<br>
<br>
So deprecating `__sleep()` with PHP 8.5 still makes sense to me. For <br>
`__wakeup()` I don&#39;t have a strong opinion either way and I&#39;ve also=
 <br>
intentionally abstained from voting on this deprecation.<br></blockquote><d=
iv><br></div><div>implementing __unserialize to satisfy a proper upgrade pa=
th for payload and child classes might be done this way:</div><div><br></di=
v><div>=C2=A0 =C2=A0 public function __unserialize(array $data): void<br>=
=C2=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 foreach ($data as $key =3D&g=
t; $value) {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 $this-&gt;{(&quot=
;\0&quot; =3D=3D=3D $key[0] ?? &#39;&#39;) ? substr($key, 1 + strrpos($key,=
 &quot;\0&quot;)) : $key} =3D $value;<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 $this-&gt;__wakeup();<br>=C2=A0 =C2=A0 }</div><=
div><br></div><div>Again, this doesn&#39;t work with private properties.</d=
iv><div><br></div><div>The previous code was better and safer, and containe=
d less boilerplate.</div><br><div>I made a draft PR for SYmfony here:=C2=A0=
<a href=3D"https://github.com/symfony/symfony/pull/61407">https://github.co=
m/symfony/symfony/pull/61407</a></div><div>I&#39;m not sure the implementat=
ion is correct yet.</div><div>That&#39;s just to show the kind of impact th=
is could have.</div><div><br></div><div>Deprecating only sleep and not wake=
up=C2=A0would at least fix the complexity of the __unserialize replacement.=
</div><div>But then, this would fail the purpose of the RFC, which was to g=
et rid of sleep/wakeup altogether.</div><div><br></div><div><div>This depre=
cation is a net loss on every aspect.<br><br></div></div><div>Nicolas</div>=
</div></div>
</div>

--000000000000a01130063c40e3e9--