Hi,
On Aug 30, 2024 at 16:47:43, Claude Pache <claude.pa...@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
