Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104362
MIME-Version: 1.0
References: <CAF+90c-q631w16DahA9bZPWyZPtYMaA0yB+vvKLxKi3AApUQDg@mail.gmail.com>
In-Reply-To: <CAF+90c-q631w16DahA9bZPWyZPtYMaA0yB+vvKLxKi3AApUQDg@mail.gmail.com>
Reply-To: bishop@php.net
Date: Tue, 12 Feb 2019 10:29:38 -0500
Message-ID: <CAEYWF=75d2wik1r1_p7VxMPSgncBRhp9od=dVuzMGYebOT0+-Q@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000006cc0a40581b419e1"
Subject: Re: [PHP-DEV] Convert ext/xml to use an object instead of resource
From: bishop@php.net (Bishop Bettini)

--0000000000006cc0a40581b419e1
Content-Type: text/plain; charset="UTF-8"

On Tue, Feb 12, 2019 at 10:00 AM Nikita Popov <nikita.ppv@gmail.com> 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).
>

+1 for movement away from resources, generally.

Resources represent connections to external resources, in a manner that's
opaque to userland [1]. Would it make sense to update is_resource (and
friends) to be aware that "resources" returned from xml_parser_* are not
resources proper, but rather resources nominal?

If userland needed to strictly determine what was a resource proper and
what was a resource nominal, we could improve the signature:
is_resource(mixed $var, bool $strict = false). But, honestly, the
aforementioned opaqueness makes me think this wouldn't be necessary.

[1]: http://php.net/manual/en/language.types.resource.php

--0000000000006cc0a40581b419e1--