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’s probably some sense in making json_encode 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’m a bit concerned about the large BC surface, even though 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’s more specific to Generators.
Philip
