On Tue, 10 Oct 2023 at 14:52, G. P. B. <george.bany...@gmail.com> wrote:
> Hello internals, > > While doing some refactoring of ext/xml to bring it up to modern engine > standards, I discovered some peculiar behaviour of the functions which set > callable handlers. > > The PR in question is: https://github.com/php/php-src/pull/12340 > > The issue is that this function doesn't just accept the usual callabes, > but also passing a method name that should be called on an object provided > by xml_set_object(). > > Moreover, contrary to the usual semantics of checking if the value is > callable when passed to the function, it is checked when attempting to call > it. > The PR changes this to validate that the function or method name provided > is actually callable. > This requires a BC break in that xml_set_object() MUST be called prior to > setting the method names, otherwise a ValueError is now thrown. > > The only project I could find that called xml_set_object() after already > setting method names for handlers is semsol/arc2 > <https://github.com/semsol/arc2> to which I have sent a PR to update this. > > The existence of the already set methods is now also checked when calling > xml_set_object() again with a different object to ensure that the handlers > are always well-defined. > > The plan is to also deprecate this feature out right, but this can be done > by adding it to the mass 8.4 deprecation RFC. [1] > > Point of this email is to know if anyone has any complaints about this BC > break before merging the PR into master. > > Best regards, > > George P. Banyard > > [1] https://wiki.php.net/rfc/deprecations_php_8_4 > If no one complains I will merge the PR at the end of the week. Best regards, George P. Banyard