Everything

php

[RFC] [Vote] Resource to object conversion

php.internals

Máté Kocsis

2 years ago

Hi Everyone,

As mentioned a few days ago, I've just opened the vote about resource to
object conversion.
The vote will be open for 2 weeks.

RFC link: https://wiki.php.net/rfc/resource_to_object_conversion
Discussion thread: https://externals.io/message/121660

Regards,
Máté

Kamil Tekiela

2 years ago

Hi Máté,

I have one question. If the primary stream resources get the
is_resource hack, what hampers the changes from landing in a minor
version? I'd assume that the migration would be mostly seamless. Is it
the get_resource_type checks? Is it because is_resource also checks
whether the resource is closed?

Regards,
Kamil

Máté Kocsis

2 years ago

Hi Kamil,

Good question... After the workaround, I think there's less reason to
do the conversion in the next major version indeed. To be honest,
I am not able to recall any use-case where a real BC break could happen.
I mean, there is a very tiny one where resources can be casted to
integers, while objects by default cannot be, but this problem is also
easily solvable (and Nikita had to do it for CurlHandle in the past),
so I don't have any other idea.

In my opinion, the main argument for converting stream resources to objects
in a major version is the "marketing value" of this achievement and possibly
the unintended side-effects/edge cases which we don't yet foresee.

Regards,
Máté

Máté Kocsis

2 years ago

Hi Everyone,

I've just closed the votes with the following outcomes:
- The primary vote for the described approach for converting resources to
objects was accepted unanimously (30 yes, 0 no)
- Primary stream resources are going to be migrated in a major version,
rather in any minor or major version (23 vs. 7 votes)
- Auxiliary stream resources are going to be migrated in a major version,
rather in any minor or major version (16 vs. 14 votes)
- The Process resource is going to be migrated in the next major version,
rather in the next minor or major version (15 vs. 14 votes)
- Resources of other extensions are going to be migrated in any minor or
major version, rather than in any major version (17 vs. 12 votes)

Thanks to everyone who participated in the discussion or the vote.

Regards,
Máté

Nicolas Grekas

2 years ago

Hello Mate,

I've just closed the votes with the following outcomes:

> - The primary vote for the described approach for converting resources to
> objects was accepted unanimously (30 yes, 0 no)
> - Primary stream resources are going to be migrated in a major version,
> rather in any minor or major version (23 vs. 7 votes)
> - Auxiliary stream resources are going to be migrated in a major version,
> rather in any minor or major version (16 vs. 14 votes)
> - The Process resource is going to be migrated in the next major version,
> rather in the next minor or major version (15 vs. 14 votes)
> - Resources of other extensions are going to be migrated in any minor or
> major version, rather than in any major version (17 vs. 12 votes)
>

Congrats for this result :)

I just had a look at the Symfony codebase and FYI we do use
cast-to-integers for a few resources:

- for curl - that's already supported by CurlHandle so this is seamless
- for DBA, because dba_list() <https://www.php.net/dba_list> works by
resourceid
- for resources created by stream_socket_server() and stream_socket_accept()

I only checked situations where the resource id is used as an index in an
array, there might be some others (but not a lot, if any).

It looks like we should implement the cast to int for these resource types
at least.

Cheers,
Nicolas

Máté Kocsis

2 years ago

Hi Nicolas,

- for curl - that's already supported by CurlHandle so this is seamless

> - for DBA, because dba_list() <https://www.php.net/dba_list> works by
> resourceid
> - for resources created by stream_socket_server() and
> stream_socket_accept()
>
> I only checked situations where the resource id is used as an index in an
> array, there might be some others (but not a lot, if any).
>
> It looks like we should implement the cast to int for these resource types
> at least.
>

Thank you very much for your research, this is very valuable information!
Surely, we will implement the necessary code to keep
supporting these use-cases.

Regards,
Máté