Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104363
To: Nikita Popov <nikita.ppv@gmail.com>,
 PHP internals <internals@lists.php.net>
References: <CAF+90c-q631w16DahA9bZPWyZPtYMaA0yB+vvKLxKi3AApUQDg@mail.gmail.com>
Message-ID: <5029d353-53ba-37e5-c5cd-1e5e0e2edab5@gmx.de>
Date: Tue, 12 Feb 2019 16:49:26 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.0
MIME-Version: 1.0
In-Reply-To: <CAF+90c-q631w16DahA9bZPWyZPtYMaA0yB+vvKLxKi3AApUQDg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: 7bit
Subject: Re: Convert ext/xml to use an object instead of resource
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 12.02.2019 at 16:00, Nikita Popov wrote:

> The ext/xml extension currently has GC issues, see
> https://bugs.php.net/bug.php?id=76874. The tl;dr is that uses of xml_parser
> will usually result in a cyclic structure, but resources do not support
> cycle GC. This means that the user is required to take care of breaking GC
> cycles manually, which is very unusual for PHP.
> 
> I would like to port the xml extension from using a resources towards using
> an object, which is fully GC integrated. This is implemented in
> https://github.com/php/php-src/pull/3526.
> 
> An XmlParser class is used instead of a resource. The class is final and
> cannot be manually constructed. It is still used with the normal
> xml_parser_* APIs. This is intended as an internal representation change,
> not a conversion to OO APIs. The xml_parser_free() method becomes
> effectively a no-op and may be deprecated in some future version.
> 
> This change is intended for PHP 7.4. While it is technically BC breaking
> (in that code that goes out of the way to use is_resource or similar may
> break), but we've done a couple of these resource->object migration in
> minor versions in the past (ref
> https://wiki.php.net/rfc/operator_overloading_gmp and
> https://wiki.php.net/rfc/hash-context.as-resource).
> 
> Any thoughts?

I'm all for switching to objects in 7.4.  I'm a bit concerned regarding
the name of the new `XmlParser` class, though, since the
xml_set_object() (a terrible API, by the way) example[1] uses exactly
this class name, and there may be quite some code which followed suit.
Maybe we should call the class `XMLPushParser`, although that would be
inconsistent with functions like xml_parser_create().

[1]
<http://php.net/manual/en/function.xml-set-object.php#refsect1-function.xml-set-object-examples>

-- 
Christoph M. Becker