Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:125404
Precedence: bulk
MIME-Version: 1.0 (Mimestream 1.3.8)
References: <CAGKNXQGahmJ8nHVy1C8d7B=Zf-jse0U6KkNBeSijDJHgbJet1Q@mail.gmail.com>
 <783E4F90-4823-4224-B45C-E9D6EFC4479F@gmail.com>
In-Reply-To: <783E4F90-4823-4224-B45C-E9D6EFC4479F@gmail.com>
Date: Mon, 2 Sep 2024 23:29:15 -0700
Message-ID: <CAGKNXQFjjdoCy1KqbfMLHkhP9one6r4318R+gNh4uGW8aEPF0w@mail.gmail.com>
Subject: Re: [PHP-DEV] json_encode() and Generators / RFC Karma
To: Claude Pache <claude.pache@gmail.com>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="0000000000003b1cdb0621312dcc"
From: phofstetter@sensational.ch (Philip Hofstetter)

--0000000000003b1cdb0621312dcc
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

 Hi,


On Aug 30, 2024 at 16:47:43, Claude Pache <claude.pache@gmail.com> wrote:

>
> Note that the issue is not limited to Generators. For instance a Closure
> object is also json-encodable but not serialisable.
>
> IMO, every internal class (not just Generator) that is marked as
> non-serialisable, should also be non-json-encodable.
>

Thank you for brining this up.

When working on the implementation to link to the RFC, I came
across ZEND_ACC_NOT_SERIALIZABLE and its usage with serialize() and it
occurred to me that there=E2=80=99s probably some sense in making json_enco=
de look
at that flag in general.

Of course this would greatly increase the amount of possible deprecation
warnings thrown and other objects flagged with ZEND_ACC_NOT_SERIALIZABLE
(resource handles, reflection, and, yes, Closure) are not as likely to
cause harm as Generators do because refactoring something from arrays to
Generators is a relatively common operation which also is mostly
transparent to the majority of callers, so a deprecation warning and later
error will provide actual value in this case whereas json_encode() over an
accidentally present, say, CurlHandle is of no consequence for a user, so
little value is provided by the eventually thrown exception.

Implementation-wise, I can say that using ZEND_ACC_NOT_SERIALIZABLE is
super simple and turning this into a 2 line patch (plus a few lines of test
code), but I=E2=80=99m a bit concerned about the large BC surface, even tho=
ugh I do
agree that conceptionally ZEND_ACC_NOT_SERIALIZABLE should be used.

I will write the RFC based on ZEND_ACC_NOT_SERIALIZABLE and if it turns out
in discussion or in the voting phase that the BC concerns are too grave, I
will find and propose an implementation that=E2=80=99s more specific to Gen=
erators.

Philip

--0000000000003b1cdb0621312dcc
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<html><body><div dir=3D"ltr">
    Hi,</div><div dir=3D"ltr"><br></div>
<br>
<div class=3D"gmail_quote">
    <div dir=3D"ltr" class=3D"gmail_attr">On Aug 30, 2024 at 16:47:43, Clau=
de Pache &lt;<a href=3D"mailto:claude.pache@gmail.com">claude.pache@gmail.c=
om</a>&gt; wrote:<br></div>
    <blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;bor=
der-left:1px solid rgb(204,204,204);padding-left:1ex" type=3D"cite">
       =20
<div>
<div><br>Note that the issue is not limited to Generators. For instance a C=
losure object is also json-encodable but not serialisable.<br><br>IMO, ever=
y internal class (not just Generator) that is marked as non-serialisable, s=
hould also be non-json-encodable.<br></div></div></blockquote><br></div><di=
v class=3D"gmail_quote" dir=3D"ltr">Thank you for brining this up.=C2=A0</d=
iv><div class=3D"gmail_quote" dir=3D"ltr"><br></div><div class=3D"gmail_quo=
te" dir=3D"ltr">When working on the implementation to link to the RFC, I ca=
me across=C2=A0ZEND_ACC_NOT_SERIALIZABLE and its usage with serialize() and=
 it occurred to me that there=E2=80=99s probably some sense in making json_=
encode look at that flag in general.</div><div class=3D"gmail_quote" dir=3D=
"ltr"><br></div><div class=3D"gmail_quote" dir=3D"ltr">Of course this would=
 greatly increase the amount of possible deprecation warnings thrown and ot=
her objects flagged with=C2=A0ZEND_ACC_NOT_SERIALIZABLE (resource handles, =
reflection, and, yes, Closure) are not as likely to cause harm as Generator=
s do because refactoring something from arrays to Generators is a relativel=
y common operation which also is mostly transparent to the majority of call=
ers, so a deprecation warning and later error will provide actual value in =
this case whereas json_encode() over an accidentally present, say, CurlHand=
le is of no consequence for a user, so little value is provided by the even=
tually thrown exception.</div><div class=3D"gmail_quote" dir=3D"ltr"><br></=
div><div class=3D"gmail_quote" dir=3D"ltr">Implementation-wise, I can say t=
hat using=C2=A0ZEND_ACC_NOT_SERIALIZABLE is super simple and turning this i=
nto a 2 line patch (plus a few lines of test code), but I=E2=80=99m a bit c=
oncerned about the large BC surface, even though I do agree that conception=
ally=C2=A0ZEND_ACC_NOT_SERIALIZABLE should be used.</div><div class=3D"gmai=
l_quote" dir=3D"ltr"><br></div><div class=3D"gmail_quote" dir=3D"ltr">I wil=
l write the RFC based on ZEND_ACC_NOT_SERIALIZABLE and if it turns out in d=
iscussion or in the voting phase that the BC concerns are too grave, I will=
 find and propose an implementation that=E2=80=99s more specific to Generat=
ors.</div><div class=3D"gmail_quote" dir=3D"ltr"><br></div><div class=3D"gm=
ail_quote" dir=3D"ltr">Philip</div></body></html>

--0000000000003b1cdb0621312dcc--