Skip to content

Resource to opaque object migration #6

Open

Description

@Girgias

A long term project is to move all extension resources to objects as they are all in all better.

This issue aims to list the remaining usages of resources within php-src

Related RFC: https://wiki.php.net/rfc/resource_to_object_conversion

PHP 8.0:

PHP 8.1:

Scheduled for PHP 8.4:

Scheduled for PHP 9.0:

Remaining:

  • stream (main/, ext/standard/) [libxml_set_streams_context() is also affected]
    • file/dir streams: le_stream / le_pstream
    • context: le_stream_context
    • filter: le_stream_filter
    • Bucket API: le_bucket / le_bucket_brigade

Persistent resources that are only used internally and not exposed to userland:

  • MySQLi: le_pmysqli
  • PDO: le_ppdo
  • PGSQL: le_plink
  • ODBC: le_pconn
  • DBA: le_pdb

Activity

  1. remicollet commented on Jun 18, 2020

    @remicollet
    Member

    About ZIP, I don't know if it worth the work, as we already have a full OO API.
    Perhaps we should rather deprecate the old functional and minimal API.

  2. kocsismate commented on Jun 19, 2020

    @kocsismate
    Member

    @remicollet I was wondering about this a while ago. See my question at php/php-src#5601 (comment)

    I even had a try at the conversion, but for me, it didn't go as easy as it seemed first, so I stopped. As far as I remember, my main problem was that the data structures used by the OO and procedural interface don't align really well.

    Is it really the case? If so, then I'd also prefer deprecating the old API.

  3. Ayesh commented on Jun 21, 2020

    @Ayesh
  4. Girgias commented on Jun 22, 2020

    @Girgias
    Author
  5. Ayesh commented on Jun 22, 2020

    @Ayesh
  6. cmb69 commented on Jun 22, 2020

    @cmb69
  7. nikic commented on Jun 22, 2020

    @nikic
    Member

    Because this is an anticipated failure mode of hosting this internal coordination tracker on GitHub, I'm going to err on the side of over-moderation and say: This is not the place to have this discussion. These issues have been discussed extensively on the PHP internals list in the past, and that would also be the place to revisit any decisions.

  8. sgolemon commented on Aug 5, 2020

    @sgolemon
    Member

    BZ2 should be filed under ext/standard/streams
    The only resources it interacts with are streams.

  9. sgolemon commented on Aug 6, 2020

    @sgolemon
    Member

    Regarding streams (which are going to be biggest PITA by far), I've been thinking it might make transition smoother (and make calling semantics generally nicer for extension authors) to add a new ZPP type F which returns a php_strea*, so:

      php_stream *stream;
    
      if (zend_parse_parameters(ZEND_NUM_ARGS(), "F", &stream) == FAILURE) {
        RETURN_THROWS();
      }
    

    The idea being that we can update the rest of the runtime where streams are taken as arguments to remove any remaining hint of resources first, then work on the internals without a massive chunk of the userspace API breaking underneath us. Obviously a handful of functions will still need special attention (fclose(), a few stream_*() functions), but they'll be the minority.

    We could even update the stubs file with a not-yet-extant "File" class and initially have the arginfo generator regard this as ZEND_ARG_TYPED_INFO(IS_RESOURCE), but when we do the big switchover, just have the generator now use ZEND_ARG_OBJ_INFO(File). Again, this should let us focus on the hard parts without being bogged down by a thousand callsites.

    Edit to add: I just remembered why I never moved on this. php_stream is a PHP thing, Zend shouldn't call back to PHP. Back to the drawing board.

  10. sgolemon commented on Aug 6, 2020

    @sgolemon
    Member

    Okay, expanded idea:

    typedef void* (zend_parse_param_callback*)(zval *arg);
    
    // In zpp arg handler:
        case 'R': { /* Resolver */
            void **ptr = va_arg(*va, void **);
            zend_parse_param_callback *callback = va_arg(*va, zend_parse_param_callback*);
    
            *ptr = callback(arg);
            if (!*ptr) {
                ZEND_ASSERT(EG(exception));
                return "";
           }
           break;
        }
    

    Then callsites look like:

    {
        php_stream *stream;
    
        if (zend_parse_parameters(ZEND_NUM_ARGS(), "R", &stream, php_stream_from_zval) == FAILURE) {
            RETURN_THROWS();
        }
    }
    

    Edit to add: While php_stream_from_zval() doesn't currently throw exceptions, I'm sure we could use a version which does throw a proper TypeError.

  11. sgolemon commented on Aug 6, 2020

    @sgolemon
    Member

    On the other hand.... I've just actually sat down to do grep -r php_stream_to_zval ext/ | wc -l and the answer is only 60. So I may be making a tempest in a teapot.

  12. added a commit that references this issue on Sep 17, 2020
  13. jrfnl commented on Oct 29, 2020

    @jrfnl

    For completeness of the list, the following extensions also saw resource to object changes in PHP 8.0:

    The status of the Enchant extension is not 100% clear to me. The change from resource to object is mentioned in the changelog, but the associated PR discussion gives the impression that the commit got reverted: php/php-src#5533

  14. jrfnl commented on Oct 29, 2020

    @jrfnl

    Another one for which I can't seem to determine the status is SNMP.
    I can see some PHP 8.0 commits mentioning a resource to object change, but I can't find an entry in the changelog, nor an associated PR:

  15. 11 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions