Re: [PHP-DEV] [RFC] [Vote] Add dedicated StreamBucket object

[Máté Kocsis]

Hi Internals,

The vote has been closed and unanimously accepted.

Thanks to everyone who participated during the discussion and the vote!

Regards,
Máté

[PHP-DEV] [RFC] [Vote] Add dedicated StreamBucket object

[Máté Kocsis]

Hi Everyone,

As promised last week, I am opening the vote for the dedicated StreamBucket
object RFC.

RFC: https://wiki.php.net/rfc/dedicated_stream_bucket
Discussion thread: https://externals.io/message/122190

The vote remains open until 2024-04-11 10:00 UTC.

Regards,
Máté

Re: [PHP-DEV] [RFC] Add dedicated StreamBucket object

[Máté Kocsis]

Hey Nicolas,

Sorry for the multi-months pause, I was working on other stuff recently.

Thank you again for the insights! I'm not sure anyone besides you has ever
used this property, but now we know
about at least one use-case. :) That's why I slightly changed the proposal:
the property would only be deprecated
as soon as the resource to object migration happens, and it would be kept
at least for an additional major release
cycle.

I hope that the proposal is fine this way. If there will be no complaints,
I would like to start the vote
sometime next week.

Regards,
Máté

Re: [PHP-DEV] [RFC] Deprecate implicitly nullable parameter type

[Máté Kocsis]

Hi Nicolas,

Thanks for your input and proactive measures! I've just finished the
analysis
you asked for:
- code: https://gist.github.com/kocsismate/c0d10c820606dfe297f09374aa634df5
- results:
https://gist.github.com/kocsismate/cf3bdfbf35eb10224ee5ecd29b39656b

TLDR: there are 880 packages out of 2000 which use implicit nullable
parameter types.
We will mention this fact in the RFC soon, as well as the need for
additionally taking
care of non-optional arguments with default values.

Regards,
Máté

Re: [PHP-DEV] Re: [RFC] [Vote] Resource to object conversion

[Máté Kocsis]

Hi Nicolas,

- for curl - that's already supported by CurlHandle so this is seamless
> - for DBA, because 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é

Re: [PHP-DEV] [RFC] Add dedicated StreamBucket object

[Máté Kocsis]

> What should users replace $bucket property with in PHP 8.4? Is there
> an alternative or is this a deprecation without a way to solve it? If
> there is currently no alternative I would not deprecate it. Just
> remove it once it becomes useless.
>

Do users actually use the $bucket property? I would be surprised if they
did. They don't
have to use it as far as I can see.

Cheers,
Máté

[PHP-DEV] [RFC] Add dedicated StreamBucket object

[Máté Kocsis]

Hi Internals,

As I have already shared my intentions in
https://externals.io/message/122149, I'd like to add a dedicated object for
stream buckets as opposed to the stdClass instances which are used nowadays.

Please read https://wiki.php.net/rfc/dedicated_stream_bucket

Regards,
Máté

Re: [PHP-DEV] Dedicated StreamBucket class

[Máté Kocsis]

Hi Jakub,


> The only issue that I see that if you migrate the resource to object you
> effectively drop that property which might be a BC break but based on the
> recent RFC results it will happen in PHP 9.0 so it's not such a big issue.
> I think this might be actually an opportunity to deprecate the usage of
> that property. So if this targeted 8.4, then it would allow us to keep the
> $bucket property there but also deprecate its accessing so users can
> migrate the unlikely use of this property. Other option would be to keep
> the property a an object self-reference but that would be pointless and not
> nice.
>

Totally agreed! Actually I have already written the RFC text with the same
proposal for deprecating the property. :) Will share it in a new thread to
start the discussion period.

Thank you for your feedback,
Máté

>

[PHP-DEV] Re: [RFC] [Vote] Resource to object conversion

[Máté Kocsis]

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é

Re: [PHP-DEV] Dedicated StreamBucket class

[Máté Kocsis]

Hi Jakub,

I just submitted feedback to the PR but will also mention it here as it's
> probably more an API thing. The problem that I see is that it combines two
> distinct things and create quite ugly self reference inside the proposed
> StreamBucket object. I would prefer we split it and introduce
> StreamBucketHandle opaque object that will completely replace the current
> use of bucket resource. The actual StreamBucket could be introduced later
> (ideally in PHP 9 as it's a sort of breaking change - change of class).
>

I agree with using a two step migration approach, and that's why I only
changed one part for now:
the containing object, leaving its resource property intact. However, as
far as I can tell,
there's no need to create a separate StreamBucketHandle object when
migrating the stream bucket resource to an
object, but we could inline it directly into the containing StreamBucket
object. I'm saying this because the
stream bucket resource property is only used by two functions:
stream_bucket_append() and stream_bucket_prepend(),
and I cannot imagine any other use-case where having a
StreamBucket::$bucket property would be useful (but correct
me if I'm wrong, I'm not the most experienced with stream buckets).


> I think it's more typing issue if someone passes this object for further
> processing and type hint stdClass. Possibly the pattern above could be used
> for copying but seems quite unlikely. Still I would see this as a BC break
> and it is not really related to resource to object migration.
>

For me, it seems like a subtle edge case which could be addressed by
explicitly mentioning the change in the UPGRADING file.
But I got your point, and I'm ok to submit an RFC.

Regards,
Máté

[PHP-DEV] Dedicated StreamBucket class

[Máté Kocsis]

Hi Everyone,

Recently, I realized that the stream_bucket_new() and
stream_bucket_make_writeable() functions
create stdClass instances by dynamically adding a "bucket", a "data" and a
"datalen" property to it.

A few days ago, I submitted a PR which makes the above mentioned functions
return a dedicated StreamBucket class which has these parameters properly
declared (https://github.com/php/php-src/pull/13111/). Furthermore,
stream_bucket_prepend() and stream_bucket_append() would accept objects of
this class as their second parameter from now on. Before, they accepted any
kind of objects as long as they had a "bucket" property containing a valid
stream.

As far as I see, my changes are backward compatible as long as people use
the stream bucket API properly (i.e. create stream buckets via
stream_bucket_new() and stream_bucket_make_writeable()). If they manually
construct such objects (i.e. $bucket = new stdClass(); $bucket->bucket =
) then obviously, they would start to face type errors.

So my question is whether anyone has any use-case/preexisting code which
falls into the second case? If no one knows about the invalid usage
pattern, my next question would be whether I have to create an RFC for this
change?

Regards,
Máté

Re: [PHP-DEV] [RFC] [Vote] Resource to object conversion

[Máté Kocsis]

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é

[PHP-DEV] [RFC] [Vote] Resource to object conversion

[Máté Kocsis]

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é

Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hey Larry,

That's fine for the main vote, but the others are all either/or votes, not
> yes/no votes, so 2/3 majority doesn't mean anything.  Do you mean those are
> 50/50 votes, or something else?
>

Thanks for your insight again, You are right, it was a silly bug in the
RFC. I've just fixed it so that the vote for the implementation itself is
now a primary vote requiring 2/3 majority, while the
rest of the votes are now secondary ones requiring a simple majority.

Thanks,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hi Everyone,

If there are no further complaints, I intend to start the votes (
https://wiki.php.net/rfc/resource_to_object_conversion) the day after
tomorrow.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hi Larry,

Thanks for your input. I'm fine with adding a separate vote whether votes
are ok with the described approaches of converting resources to objects,
and then the 3 way vote can be eliminated.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hey Everyone,

Sorry for the radio silence, I was busy with other tasks. However, I
managed to improve the RFC in the recent days the following way:
- most importantly, I changed the suggested approach of the conversion in
case of primary stream resources: the is_resource() hack mentioned a few
times
before would be used in this case.
- I added impact analysis for each resource category + listed all functions
which return streams (even though they are not impacted).
- I added more examples, reasoning etc.

Please read the updated proposal because I intend to start the vote shortly
after the holiday season.

Thanks,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hi Derick, Jakub, Phiip


> Did you do an analyses as to how much either of these changes could break
> anything?


I updated the RFC with some impact analysis. The numbers support my
hypothesis that the conversion
of auxiliary stream resources would cause hardly any BC break - at least in
case of the top 2000 PHP
packages. That's why I believe we can migrate them separately from the
primary ones, which are used
much more often.

 I wonder whether it was previously discussed to have all these converted
> objects implement a `Resource` interface and have `is_resource()` check for
> that.
>

Yes, the RFC links two threads about this topic:
https://externals.io/message/116127 and
https://externals.io/message/104361#104369. Even though
there's a clear interest in changing how is_resource() works, doing so
would bring us to a minefield...

Please could you add a separate vote for primary streams if the resource to
> object conversion should be done at all (requiring 2/3 votes to be
> accepted).

I will personally vote against this if there is no is_resource change as I
> think it's just too big BC break even for 9.0 - it will likely require
> massive update of many code bases.


Yes, I added the option.


> Both the function and maybe also the interface could then also be marked
> as deprecated, but it would allow for a much more painless transition.
>

While I would love to see resources go altogether, deprecating
is_resource() is way too early. Even if php-src itself manages to sunset
all the built-in resources,
there will still be lots of third party extensions which will still rely on
them.

Personally, I’m now relying on psalm to detect such issues, so if I had a
> vote I would selfishly vote yes anyways, but still: for those without
> static analysis,
>
this would IMHO make things much easier.
>

Yeah, using static analysis should be mandatory for mission critical
systems, since these tools can easily detect issues like wrong
is_resource() checks.
However, I acknowledge that doing so is not always possible due to some
constraints (i.e. budget limits). Fortunately, the possible issues
caused by resource
to object conversions are usually not too difficult to find out. For
example, in case of the Process resource, one has to search for all
proc_open() invocations, and
check whether the return values are correctly checked. According to my
experience, the is_resource() checks are usually very close to the creation
of resources, so
they can be fixed easily when needed.

Regards,
Máté

[PHP-DEV] [RFC] [Discussion] Resource to object conversion

[Máté Kocsis]

Hi Internals,

Following my straw poll about the Process resource name, I would like to
present an RFC which clarifies the rough timeline and the BC promises of
the "resource to object conversion" project.

Link: https://wiki.php.net/rfc/resource_to_object_conversion

I'm looking forward to your feedback!

Regards,
Máté

Re: [PHP-DEV] [STRAW POLL] [VOTE] Process resource to object migration

[Máté Kocsis]

Hi Internals,

The vote was closed, and thanks to Derick's calculations, we have a
winner: ProcessHandle.

Thanks to everyone who participated in the vote!

I think we should have a proper vote if this should go to 8.4 at all. I
> would vote against this due to the BC break. I would be fine with 9.0
> though.


I'm currently working on a general resource to object migration RFC in
order to make it clear when the remaining resources can be migrated.

Regards,
Máté

>

Re: [PHP-DEV] Change the signature of odbc_connect

[Máté Kocsis]

Hi Saki,

In my opinion, this is a simple and useful bugfix + nice consistency
improvement with negligible backward compatibility break, so
I don't think it needs any vote.

Regards,
Máté

Re: [PHP-DEV] [STRAW POLL] [VOTE] Process resource to object migration

[Máté Kocsis]

Hi Tim,

>
> I assume this should read 2023-**11**-07? This would match the 14 days
> for regular votes.
>

Yes, such a silly typo, thank you for pointing that out. The deadline is
indeed in November.

Regards,
Máté

[PHP-DEV] [STRAW POLL] [VOTE] Process resource to object migration

[Máté Kocsis]

As I have indicated in my last mail (
https://externals.io/message/121164#121364), I would like to decide the
naming question of the "le_proc_open" resource via a straw poll. Therefore,
I've just opened the vote: https://wiki.php.net/rfc/process_object_name

It is a ranked choice poll (following STV) and voting is open until
2023-10-07 18:00:00 UTC.

Thanks:
Máté

Re: [PHP-DEV] proc_open() resource to opaque object migration

[Máté Kocsis]

Hi Tim,


> I agree here. While I'm totally in favor of using namespaces in core, it
> should be done somewhat consistently. If the proc_* functions are in the
> global namespace, then so should the resource object.
>

While I don't necessarily want to add a dedicated namespace for Process (or
whatever name we settle on), I don't agree with the reasoning: the resource
to
opaque object migration project is just a first step, afterwards, a new
object
oriented API could be added (as you already noted), and later on,
the procedural
one should be deprecated. That's why I think this kind of consistency is
not important
in this case. Please note that many such opaque objects have been added to
namespaces recently (e.g. FTP\Connection, IMAP\Connection,
LDAP\Connection etc.). so we already have some precedent.

I agree with the rest of your answer though. My favourite name is Process
and
I am fine with ProcessHandle as well. The latter name was proposed by
Nicolas in a private conversation.

In my opinion, a straw poll should decide the naming question. I honestly
don't
think that the change needs any more discussion/an RFC, as it doesn't cause
any larger BC break than some of the other resource migrations we performed
since PHP 8.0 -- and there were a lot of them: resources of around 20
extensions
got converted to proper objects among curl, openssl, and pgsql just to
mention some.
I'd argue that at least ext/curl is more widespread than proc_* functions
are.

Regards,
Máté

[PHP-DEV] proc_open() resource to opaque object migration

[Máté Kocsis]

Hi Everyone,

I'm writing in connection with a question coming up lately during the
"resource to opaque object migration" project (
https://github.com/php/php-tasks/issues/6) which we have been working on
for quite a long while.

During the review of my PR migrating the resource returned by proc_open()
to an object (https://github.com/php/php-src/pull/12098), it quickly became
evident that there was no consensus about the new class name, since the
originally proposed "Process" name has a non-negligible BC break likelihood.

That's why we should find the best class name in accordance with Nikita's
namespace naming convention RFC (
https://wiki.php.net/rfc/namespaces_in_bundled_extensions). Even though my
PR currently implements "Standard\Process", this name is not a good
candidate according to the policy:

Because these extensions combine a lot of unrelated or only tangentially
> related functionality, symbols should not be namespaced under the Core,
> Standard or Spl namespaces. Instead, these extensions should be considered
> as a collection of different components, and should be namespaced according
> to these.


Does anyone have a good suggestion?

Thanks,
Máté

Re: [PHP-DEV] Arginfo Mismatch For Optional Parameters

[Máté Kocsis]

Hi,

The problem is that your arginfo declares parameters of type double, while
you accept the double|null type in ZPP.

P.s. i'm not sure based on your message whether you use stubs for declaring
symbols, but if that's not the case, then doing so is highly recommended.
You can find plenty of examples in php-src (*.stub.php files).

Regards,
Máté

[PHP-DEV] Re: [RFC] [Vote] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Everyone,

The votes have ended with the following results:

- array_keys: rejected (13 vs. 1 vs. 15 votes)
- DatePeriod::__construct(): accepted, to be removed in PHP 9.0 (23 vs. 0
vs. 0 votes)
- dba_fetch(): accepted, to be removed in PHP 9.0 (24 vs. 0 votes)
- FFI::cast(), FFI::new(), and FFI::type(): accepted, to be removed in PHP
9.0 (19 vs. 0 votes)
- get_class() and get_parent_class(): accepted, to be removed in PHP 9.0
(22 vs. 0 vs. 0 votes)
- IntlCalendar::set(): accepted, to be removed in PHP 9.0 (16 vs. 3 vs. 4
votes)
- IntlGregorianCalendar::__construct(): accepted, to be removed in PHP 9.0
(16 vs. 3 vs. 4 votes)
- ldap_connect(): accepted, to be removed in PHP 9.0 (21 vs. 0 votes)
- ldap_exop(): accepted, to be removed in PHP 9.0 (19 vs. 0 votes)
- pg_fetch_result(), pg_field_prtlen(), and pg_field_is_null(): accepted,
to be removed in PHP 9.0 (19 vs. 1 vs. 0 votes)
- Phar::setStub(): accepted, to be removed in PHP 9.0 (20 vs. 0 votes)
- ReflectionMethod::__construct(): accepted, to be removed in PHP 9.0 (18
vs. 1 vs. 0 votes)
- ReflectionProperty::setValue(): accepted, to be removed in PHP 9.0 (16
vs. 5 votes)
- session_set_save_handler(): accepted, to be removed in PHP 9.0 (17 vs. 4
votes vs. 1 votes)
- stream_context_set_option(): accepted, to be removed in PHP 9.0 (18 vs. 5
votes vs. 1 votes)
- Policy about new functions: accepted (24 vs. 0 votes)

Thanks to everyone for their participation!

Regards,
Máté

>

[PHP-DEV] Re: [RFC] [Vote] PHP 8.3 deprecations

[Máté Kocsis]

Hi Everyone,

The votes have ended with the following results:

- "Passing negative $widths to mb_strimwidth()": accepted (27 vs. 0 votes)
- "The NumberFormatter::TYPE_CURRENCY constant": accepted (29 vs. 0 votes)
- "Unnecessary crypt() related constants": rejected (14 vs. 10 votes)
- "MT_RAND_PHP": accepted (28 vs. 0 votes)
- "Global Mersenne Twister": rejected (13 vs. 16 votes)
- "Calling ''ldap_connect'' with 2 parameters": accepted (26 vs. 0 votes)

Thanks for everyone for their participation!

Regards,
Máté

Máté Kocsis  ezt írta (időpont: 2023. jún. 22., Cs
12:14):

> Hi Everyone,
>
> As previously announced on the list, we have just started the vote about
> the annual PHP deprecation RFC.
>
> Link to the RFC: https://wiki.php.net/rfc/deprecations_php_8_3
> Link to the discussion thread: https://externals.io/message/120422
>
> The vote is open until 2023-07-06 12:00:00 UTC.
>
>

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hi Everyone,

I'd like to let you know that I'm not going to propose "clone with" for PHP
8.3 in its current form due to
the questions and possible complexity which were revealed during the last
couple of emails.

Hopefully, we'll be able to come up with something for PHP 8.4...

Regards,
Máté

[PHP-DEV] [RFC] [Vote] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Everyone,

As previously announced on the list, I have just started the vote about the
"Deprecate functions with overloaded signatures".

Link to the RFC:
https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures
Link to the discussion thread: https://externals.io/message/120146

The vote is open until 2023-07-10 16:00:00 UTC.

Regard,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Derick,

In any case, I don't mind this — I'm actually going to suggest to change
> the constructor to:
>
> public function __construct(DateTimeInterface $start, DateInterval
> $interval, DateTimeInterface|int $end, int $options = 0) {}
>
> And then *only* add:
>
> public static function createFromISO8601String(string $specification, int
> $options = 0): static {}
>
> This solves the original problem of not being able to define the
> signatures in stubs, and also extracts the most problematic of methods
> into a factory method where it should always have belonged.
>

Thanks for your response, I've just updated the RFC accordingly. It solves
the main issue indeed, and we still have the possibility to
add a second factory method using the $recurrences parameter later if we
deem it useful at some point.

With all that said, I don't want to update the RFC anymore, so I plan to
start the vote on Monday.

Regards,
Máté

[PHP-DEV] [RFC] [Vote] PHP 8.3 deprecations

[Máté Kocsis]

Hi Everyone,

As previously announced on the list, we have just started the vote about
the annual PHP deprecation RFC.

Link to the RFC: https://wiki.php.net/rfc/deprecations_php_8_3
Link to the discussion thread: https://externals.io/message/120422

The vote is open until 2023-07-06 12:00:00 UTC.

Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Rowan and Larry,


> Isn't that exactly what a deprecation period is for?
>

Yes, sure, but I wrote my arguments with the "Short deprecation period" in
mind so that the removal of
these functions causes the least pain.


> If we want to give people longer, just leave the functionality deprecated
> for longer before removing it. If we want to phase that in gradually, start
> with a documentation-only deprecation, and add the deprecation notice
> later.



> If the plan is to keep the current function name, we can't get any of the
> (very small) benefits of removing the extra signature until the final
> removal anyway.
>

I don't completely understand this sentence, but my current plan is to go
with the original function
name as this causes the least impact and at the same time makes the scope
of the RFC
smaller. I'm tired of the debates (even if they were useful), and I don't
want to add any tangentially
related things in the RFC anymore. Anyway, adding an alias
(session_set_handler()) and/or deprecating
the original function name later is trivial, if someone wants to pursue
this.


> I'd propose a doc-only deprecation now (with session_set_handler() added
> for the object version),

an E_DEPRECATED in 9.0, and full removal in 10.0.  Assuming the expected
> release schedule,

that gives people ~2 years before they see even an E_DEPRECATED, and 6-7
> years before they

are forced to change. That should be ample time for anyone that still needs
> to make the switch.


I want to keep the original voting choices (the short and long path). Then
votes can decide how much
notice period they want to give. Since I don't insist on changing the
function name anymore,
the impact of this change is a lower, as only people using the callback
signature would be affected.
The fortunate thing is that session handling is used less in library code
so at least open source maintainers
rarely have to deal with the deprecation. And proprietary code maintainers
can mostly ignore or suppress it
for a while.

Regards,
Máté

Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Bruce,

For those who use callbacks now, how is this in any way better? They will
> eventually end up using an OOP approach anyway (as that's the strategic
> goal).
> Migrating from `session_set_handler_callbacks()` would still be
> (potentially)
> non-trivial. And what's the point migrating *to*
> `session_set_handler_callbacks()`
> if we already know it will be deprecated and removed soon enough?


The reason why I think it's a good approach to have an intermediate state
is to give
these people the possibility to defer the actual migration until the
very end. So they can
choose what to do when the 6+ parameter version of
session_set_save_handler()
becomes deprecated: they can either go straight to session_set_handler()
*if they are ready* or
they can also choose gaining time with a very straightforward change and use
session_set_handler_callbacks() for at least a couple of years more.

The important thing is that people are not yet forced to make a bigger
effort, unless they are already
ready for that.

Regards,
Máté

Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

HI Nicolas and Rowan,

Mate, WDYT of 2)?


Your sentence about better typing rang a bell with me: I think that's the
best argument
for removing the signature accepting callables. But before coming to a
conclusion,
let me answer Rowan's question first:

This is where I don't understand what you or Maté consider to be the
> alternative. If we create any function with a new name, then we have two
> names for the user to choose between, regardless of whether that's one old
> name and one new, or two new names. That's why I'm saying that if we have
> two names at all, they should both be chosen with new users in mind, not
> fudged based on a combination of current and new naming styles.
>
> Once the deprecation period is over, we should be left with a pair of names
> that obviously signals the difference; or, we should be left with only one
> function.
>

Of course, the best situation would be to have only the new function name
with a single
signature. Personally, I also prefer the OO style, but this doesn't
really matter now. However,
the sad truth is that we have a very widespread function with a slightly
incorrect name supporting
2 signatures for more than a decade...

On top of that, while most of the other deprecations in the RFC either
affect rare functionality, or are
suppressable via a simple polyfill or some automation/search+replace, only
supporting the
OO style signature of session_set_save_handler() would mean that people
would have to
wrap the callbacks in a class. This is not automatable, and requires some
possibly non-trivial manual work.
This means that we have to change anything really carefully here.

With all these in mind, I'd say that the strategic goal should be to have a
single signature +
the new function name only. Reaching this state would affect every single
proprietary codebase in some
way or another, so I don't think it's feasible or fair with users to try it
in one single step. So in order to reduce
the damage, my plan is the following:

* Let's keep one of the two signatures which are compatible with older PHP
versions so that at least a fraction of
all users (maybe 50-70%?) are not impacted at all. That's why
session_set_save_handler() should not be removed
just yet. Since the OO version seems more future-proof based on Nicolas'
great argument (it accepts an interface,
while callbacks don't have a formal signature), we should keep this form.
* People who use the signature to be deprecated should not be forced to do
non-trivial changes. So introducing
a new function for them will keep their code run with a minimal
effort (search + replace), while we managed to
eliminate the overloaded signature. At the same time, we try to educate
them (mostly in the manual) that the best
way forward is to migrate their code to use session_set_handler(). There's
no deprecation just yet, and everyone
can do it voluntarily at their own pace.
* In order to finally achieve the strategic goal, we deprecate the old
function along with session_set_handler_callbacks()
as soon as the right time comes. That may be PHP 9.x or 10.y or 11.z,
whatever.

This process may sound complicated, but in my opinion, this is the least
disruptive upgrade path for achieving the goal
I outlined above. Sometimes going full ninja mode is possible, but I'd
prefer being careful in this very case, especially
because I guess some people do not consider the motivation of the RFC
particularly important, so I would like if their
cost-benefit analysis would stay positive (I don't only mean voters but end
users as well, since their perception also
matters when they are fixing the incompatibilities of their code).

Máté

Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

[Máté Kocsis]

Hi Juliette,

There are plenty of situations where it is of absolutely no interest to
> get a crypographically secure value, like for generating some
> semi-random test data and I strongly believe the impact of these
> deprecations to be large and fixing it won't always be trivial for
> codebases which support a range of PHP versions.


Please note that Tim has just added a clarification regarding the removal of
mt_rand():

For the Global Mersenne Twister specifically the removal will be left to an
> additional later vote,

allowing to defer the removal based on the remaining usage.
>

This means that mt_rand()  will most probably have a longer phasing out
period than normally, but at least
this buys us time to evaluate the timeframe of the removal. I hope that we
addressed your concerns.

As a matter of principle I believe there should be an impact analysis
> for anything being deprecated. It can inform the voters as to the
> appropriateness of the timing of the deprecation - early on in a major
> cycle vs late in a major cycle -. Others may just take it as a FYI, but
> at least they have access to the information if they wanted to assess it.
>

I don't think an impact analysis is useful all the time: sometimes because
it doesn't make much sense
*trying* to measure the impact of deprecating very rare or broken
functionality (e.g.
CRYPT_STD_* or the NumberFormatter::TYPE_CURRENCY constants). In other
cases, the analysis is
likely going to be misleading, since we don't have access to proprietary
code, and some functionality
is used in such code more often than in open-source projects (e.g. the
typical example is session-related
functions).


> While I appreciate what you are saying about deprecations being an
> "action list to migrate at your own pace", the reality for open source
> packages is different as the sheer amount of deprecations over the past
> few years have taken huge amounts of time to address and "leaving those
> till later" is just not an option as the amount of time needed is the
> same and time can only be spend once.
>

Please let me clarify first that we do appreciate your dedication and
efforts a lot for maintaining such critical projects.
At the same time, - since PHP has almost 3 decades long of track record of
doing silly things -, we as maintainers
are also dedicated very much about improving our language and getting rid
of its unreliable/unexpected/unsafe behavior.

I also do believe that we do care about our users, and it's very rare that
some change is introduced without a heads up
multiple years before the removal. I know these deprecations and removals
usually feel like a burden for people who
tirelessly work on following these changes in their open-source projects in
their free time, but please do understand that
PHP would still be the same crazy language without the fundamental changes
which have been being introduced
in the recent years. We are happy to discuss the underlying problem
separately from this RFC, since the root cause
may be something else than the too many deprecations, starting from things
which we do not have control over (the underfincaning
of open source work) to other factors which we can control (e.g. the
minimum length of deprecation before
the removal, shorter/longer major version cycles, educating end-users to
use automation like Rector).

With all that said, we're going to start the vote on Thursday, since we do
believe the proposal is clear enough now, and
there's not much to discuss anymore.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Juliette,

Respectfully though, in my opinion selectively leaving out impact analysis
> without mentioning why they are missing in the RFC, reeks of trying to hide
> information which could influence the vote.
> Maybe just mentioning why the impact analysis is missing in these cases in
> the RFC could take that stench away ?
>

I respectfully disagree that it would affect the vote result, but I added
the clarification you suggested nevertheless. :) Doing so resulted in a
very nice side-effect: I noticed that the array_keys_search() function name
is slightly inconsistent/misleading, since what it really does is to filter
keys based on their *value* (thus the $filter_value param name). So I
renamed it to array_keys_filter() as well as came up with
a new suggested replacement which is compatible with at least PHP 4: using
the combination of array_keys() and array_filter() retains the original
behavior (example is added to the RFC). I know
that it's less than ideal performance-wise, but it works anyway (and not
all code is performance sensitive).

Thanks,
Máté

Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

[Máté Kocsis]

Hi Juliette,

For the mb_strimwidth() proposal an impact analysis is provided at the end
("over 100 projects were reviewed").

For the other proposals there isn't and we do not believe this to be
useful. We consider deprecations to be different from other RFCs that add
new features,
because functionality usually is deprecated because it is "harmful" in one
way or another. A large impact (i.e. broad usage) should not necessarily be
an
argument against deprecation. Please also keep in mind that a deprecation
is just that, a deprecation. It gives developers a heads up to migrate off
the
functionality at their own pace. We know that package maintainers are
bugged by users whenever a new deprecation message emerges in a new PHP
version,
but this doesn't mean maintainers must react right away.

Let us elaborate why we don't think impact analysis is needed here:

* As per the TYPE_CURRENCY constant is (a) completely non-functional and
(b) unfixable. As such any code that uses the constant is already broken and
the deprecation is just pointing this out, making the user aware.
* The CRYPT_* constants are possibly the least harmful part of the RFC, but
they still can be misleading to a new developer. They are trivially
polyfilled and
also trivially removed from existing code by replacing them with 1.
* MT_RAND_PHP is broken, because the sequences are not well-defined,
defeating any reason for seeding. More reasons are already given in the RFC.
* mt_rand() is everywhere and we don't believe anyone is denying that fact.
But this should not be a reason against deprecation. As the RFC outlines,
it is trivial to find this function misused with GitHub's search. We
believe that the depreciation benefit outweighs the costs for existing
users.
* ldap_connect() is already soft-deprecated in the documentation.

Regards,
The authors of th RFC

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi,

The impact analysis on userland code seems to be missing for some of the
> proposals, most notably for the proposals which I expect will have the
> highest impact. I'd like to ask for an impact analysis to be added to
> each of these:
> * array_keys()
> * ReflectionProperty::setValue()
>

These are intentionally left out due to different reasons:

* Even though the 1 parameter version of array_keys() is used a lot, its 2+
parameter signature is much less known.
Personally, I've never used it. Of course, this doesn't mean no one uses
it, but I don't think this deprecation will really
matter in practice, so I didn't care to write a script for this (the rest
of the deprecations were easy to analyze via regexes).

* ReflectionProperty::setValue() would be a lot of work to analyze since
it's very difficult to track whether the
setValue() method is called on a ReflectionProperty instance. Not to
mention the fact that the deprecation only affects
function invocations where either a single parameter is provided or static
properties where the first parameter is not null
or an object instance. Since the suggested alternative is basically
available since forever, I think it's OK not to do
an exhaustive analysis in this case.


> For the `get*_class()` deprecation, I wonder if an additional impact
> analysis is needed for packages which may not have removed usages of
> `get_class( null )`, which would now be double-impacted (and not caught
> by the current analysis).
>

I'm not sure I can follow you, but get_class(null) is a TypeError since 8.0
(and have been warning since 7.2). If a package
didn't feel the need to migrate its get_class() usages then it is likely a
dead project. And if someone tries to upgrade their
(proprietary) code from PHP 7.1 to PHP 8.3 directly then they will notice
the exception first and also - if they followed the upgrading
notes carelly - will learn the relevant suggestions regarding the
deprecation. But realistically speaking, these projects will most probably
be bound by lots of other more severe issues, so I don't think the above is
a viable upgrade path...

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

[Máté Kocsis]

Hi everyone,

As there was no discussion and complaint for a long time, we would like to
move forward with the RFC. We believe Go and Tim answered Nikita's doubts
elaborately, so we should make the question decided during the vote.

Therefore, we'll start the vote on Monday, unless new problems emerge.

Thanks,
Máté

Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

>
> Given that you've agreed that neither signature is "primary" or "better",
> doesn't that argument apply equally to both signatures? If it's OK to force
> anyone using individual callbacks to change their code, why is it not
> equally OK to force anyone using an object to change their code?
>

As far as I remember, my reasoning was two-fold:
- naming: passing *multiple* callbacks to a function name in singular
(session_set_save_handler())
is definitely not ideal
- a decision had to be made and I thought that keeping the OO approach in
place had less BC impact,
especially because previously I got feedback from Nicolas that they used
this version.


> I do wonder if this change is too disruptive relative to the benefit it
> brings, but in that case, we should just leave the whole function as it is.
>

In my opinion, phasing out session_set_save_handler() completely would be
worth the effort if this
was the only option to fix the overridden signature. However, it's not the
case, since strictly speaking,
only one of the two signatures has to be removed in order to achieve the
desired goal of the RFC.
The whole discussion about also deprecating the other one started only
because of improving naming:
it is also a nice thing to pursue but fails the cost-benefit analysis. All
in all, I think neither not doing
anything, nor deprecating the whole function is a good choice. But at least
I definitely want the question
to be settled with putting this to vote.

I don't think "handler" particularly implies "object". The "handler" passed
> to set_error_handler is a function, and your original suggestion for a new
> function was "session_set_save_handlers", where "handler" would also mean
> "function".
>

Without any suffix at all, it seems like "set a session handler the normal
> way" vs "set a session a special different way"; like how
> "array_merge_recursive" is a special version of "array_merge".
>

I think my reasoning is easier to understand if we go back to my original
suggestion:
session_set_save_handler() and session_set_save_handlers(), which would be
session_set_handler() and session_set_handlers() now. That was the starting
point,
but you didn't like that they only differ by an "s". That's why I swapped
the "s" with "callbacks".

According to your deduction, session_set_handler() is not the most correct
name indeed, but I don't think
that we always have to choose the most correct and the entirely descriptive
one. After all, neither the
parameters of session_set_handler() are called "$open_callback",
"$close_callback" etc., they are just
"$open", "$close" etc. and I guess they are still clear enough, given that
their type is declared which
completes the missing context.

Similarly, we all would know that the session_set_handler() function
expects an object of
SessionHandlerInterface type, while its sibling,
session_set_handler_callbacks()  expects some
callbacks. Yes, having the _object suffix is the most pedantic name, but
personally, I don't really like it
because I find it ugly (and I was pretty much content with
session_set_handlers()). I'm curious about
the point of view of other people though, because if it turns out that
people generally also favor some
other name then I'm ok with a compromise.

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hey Nicolas,

Instead of passing arguments to __clone(), I wondered about a new
> __clone_with(array
> $properties) that could be implemented next to __clone(), with the
> following behavior:
>
>- if only __clone is implemented, same behavior as always
>- if __clone_with is implemented, __clone is never called. Instead, the
>engine calls __clone_with with the map of properties in the "with" (or
>an empty array if no "with"). In addition and *before* the call, the
> engine
>sets the properties according to the "with" (potentially failing
>visibility/type checks as expected)
>

Interesting approach! The rules seem clear, and as far as I'm qualified to
tell,
the implementation is viable. I'm not sure though what should happen if one
of the "clone with" assignments throws an exception? By default,
__clone_with()
would never be called in this case. Are we OK with this, right? Because
this way,
it's easy to circumvent any validation performed inside __clone_with() by
triggering
an exception (and then catching it). I don't think this is a huge problem,
since visibility
rules apply, so one cannot just blindly do silly things with someone else's
code.

Also, I'm wondering whether it's worth the complexity to add support for
another magic method,
or would we be fine without this feature and its benefits? To put it
another way: how much
complexity is it worth to fix the problem of the wasteful deep cloning?
Does anyone have
an informed answer/opinion?

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Nicolas,

On my side, I'd very much prefer keeping the constructor of DatePeriod and
> thus making it non-overloaded with this signature:
>
> public function __construct(DateTimeInterface $start, DateInterval
> $interval, DateTimeInterface|int $end, int $options = 0) {}
>
> That'd help a lot with the migration/BC path and this signature also just
> makes sense.
>

I completely agree with this (more so since this used to be the original
proposal), so I'll
ask Derick again about his stance on this.

Unrelated: I don't think adding "ReflectionMethod::createFromMethodName" is
> useful. Using explode is just fine for those that need this style. And
> since this is the recommended migration path, better not give a reason to
> change twice the code.
>

I don't really mind removing ReflectionMethod::createFromMethodName, so if
anyone has a
use-case which heavily relies on creating the reflection method from a
callable name ("Class::method" format),
now is the time to speak up for this method.

As far as I see the relevant impact analysis file (
https://gist.github.com/kocsismate/b457f8fef074039b58fbee9121d05e92),
Laminas, Symfony and Nette DI would also be impacted by the removal. I
don't think that it causes a big problem to
explode the string instead, but I'm wondering if performance sensitive code
may be negatively affected (?).

Thank you, Nicolas, for your insights!
Máté

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

(Resending my earlier message to Levi:)

Hi Levi,

I apologize if this has been discussed before, as I have fallen very
> behind on internals discussions. I think it would be helpful to add an
> example where the object being cloned accesses its properties.
> Something like this:
>
>$b = clone $a with ["count" => $a->count * 2];
>
> If this is not valid, or if it has unexpected behavior or semantics,
> that should also be included in the RFC.
>

It hasn't been discussed yet, but thankfully, there's nothing special about
it:
the LHS is evaluated first, then comes the RHS. Nevertheless, I've just
clarified this in the RFC,
and thanks for pointing this out!

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hi Larry,

In this case, if the `with` happens first, then the new address object is
> cloned needlessly, but that *probably* doesn't hurt anything.  But $newAddr
> !== $p3->address.
>

Yes, I agree with this: "clone $this with ["x" => "y"];" is the easiest to
mentally model as a shorthand for "$self = clone $this; $self->x = "y";".
If we agree with this model, then it would be weird to execute __clone() at
the end indeed. But I think Nicolas' example explained this fact much
better than I could. :) Also, separating __clone() from the rest of the
clone opcode would be .

And you are right, some objects may be cloned unnecessarily, which doesn't
hurt, but isn't ideal for sure. I should mention that in ideal
circumstances (when all properties are readonly + all of them are
initialized during construction + none of them are mutable internally),
deep cloning is not 100% required, unless only for defensive programming
purposes.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

>
> It looks like the current preferred signature was only introduced in PHP
> 8.2. Previously, the signatures were dba_fetch($key, $handle) and
> dba_fetch($key, $skip, $handle) - it effectively had a non-final optional
> parameter.
>

Yes, that's the case! Thanks for reminding me again, I clarified in the RFC
that PHP 8.2 is the first version which supports the preferred signature.


> Whether that makes it too early to deprecate the older 3-parameter form,
> I'm not sure. As you say, it's probably pretty rare, particularly because
> according to the docs that parameter is ignored for most file formats the
> function supports anyway.
>

Normally, I wouldn't do this, but I'm certain that deprecating this
signature won't cause a big havoc in the PHP ecosystem, so that's the only
reason I went ahead.

>

Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

>
> In my mind, the only reason to change anything about this function is that
> we can't properly overload a function based on its argument types. There's
> nothing particularly "primary" or "better" about either of the two
> signatures, it's just hard to document and use named parameters while
> they're both part of one function.
>

Yes, that's the case exactly.


> My suggestion was very explicitly that everyone using sessions should have
> to change their code, to a name that explicitly mentions the parameter type
> in its name (because that's the difference between the two). As I said, I
> think it's much simpler to communicate "this function is deprecated, choose
> the appropriate from these two names", rather than "this function is
> deprecated in one format, but not the other, and if you are using the
> deprecated format, you can use this instead".
>

I understand that communication is much simpler with your suggested
approach, however, deprecating something which everyone surely uses partly
undermines the efforts we have made for ensuring as much backward
compatibility as reasonably possible. That's why I'm not in favor of
officially deprecating the object oriented version of
session_set_save_handler() just yet. I think soft deprecating it for now
results in a smoother upgrade path by letting people optionally use its
alias first (session_set_handler()), and then we can start nagging them
later on. Otherwise, I would question why we had to have such a long
discussion focusing mainly on the BC aspects of the proposal. I also
understand that polyfilling session_set_save_handler() would also be easy
to do, but the migration is even easier if we leave the 2-parameter version
alone for a while.

Both the "_object" suffix (or some equivalent) and deprecating the original
> regardless of signature are inherent in my suggestion. You seem to have
> interpreted it as something else, and I'm not entirely sure what you're
> actually proposing - how does an alias fit in to something that's about
> splitting a function into two?
>

In my opinion, the "_object" suffix is superfluous. There would be two
session_set_handler*() functions: the first one accepts a session handler
(instance) (session_set_handler()) and the second one accepts session
handler callbacks (session_set_handler_callbacks()). They are not primary
or secondary, I just don't like the "_object" suffix, and I couldn't come
up with a better alternative.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hey Everyone,

In the meanwhile, I noticed yet another overloaded function (dba_fetch())
and I included it into the proposal. In my opinion, it's a no-brainer
to deprecate due to its low adoption rate and very odd behavior - to say at
least.

The RFC evolved a lot during the discussion phase, thanks for everyone who
was involved! I believe my proposal is ready now, as I don't intend to
change anything in it anymore. This means that I'll start the vote mid-next
week, unless new concerns emerge.

Thanks,
Máté

Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

I'm forwarding my mails which I managed to only address for Rowan:

Hi Rowan,


> If we go down that route, perhaps we should come up with a corresponding
> name for the object based version - "session_set_handler_object" perhaps?
> That would also mean the deprecation messages can be much simpler: if
> you're using the old name, you need to do something.
>

Hmm, that's also a very good idea, and I support this. However, I'm
hesitant to deprecate the 2 parameter version of session_set_save_handler()
just yet,
since doing so would mean that everyone using sessions has to use a new
function... Instead, I opted for aliasing it to the new
session_set_handler() function (I'm not fond
of the "_object" postfix, because I regard it unnecessary as it operates on
SessionHandlerInterface instances). And we could maybe deprecate
session_set_save_handler()
altogether in a few years.

Then I guess we should just pack up and go home, because right now PHP
> doesn't even have an official static analyser, let alone a mandatory one;
> and the evolution of Hack shows just how much the language would need to
> change for such a tool to guarantee full coverage.
>

I'm not 100% sure an official static analyser would be needed. I think it's
already a good situation to have very well usable and maintained userland
implementations, which we do have. After all, we neither bundle Composer
or PHPUnit.

To expand on my previous statement in my other main: I don't say that
everyone uses static analysers I know many people don't... My message
would have rather been that bugs don't cost enough for these projects
if they don't have to actively prevent the introduction of bugs via static
analysis. I have realized in the meanwhile though that there may be many
other reasons why someone doesn't use these tools, so I think this argument
of mine is invalidated.

I think Claude is taking the same premise, and reaching a  different
> conclusion: returning true is consistent with the other methods on the
> class, and that consistency makes it more predictable and more accessible.
>

However, as George highlighted (I also tried, but most probably I couldn't
convey my message), declaring true return types is an intermediary step
before changing them to void. So replacing the old IntlCalendar methods
having "incorrect" return types with new ones having the "correct" return
types is a very nice by-product of this RFC, since we (and users) don't
have to deal with the return type migration separately at a later point of
time.
I still think that the benefit of immediately using void for the new
methods outweighs the risks.

Regards:
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Claude,

The very risk is that the programmer must be aware that the return
> convention has changed. Recall that not everyone run static analysers,
> especially for such a trivial migration, and that not every code is
> statically analysable (e.g.. `$foo->$bar()`).
>

I hope that I don't sound elitist, but codebases not using static
analysis... are kind of hopeless... That is, in my opinion, there's no
other option
to run a larger PHP project successfully than to either use static analysis
or to have programmers who don't commit any mistakes. For the rest
of the less valuable projects, the cost of messing up with the return value
check is not much.

What is the purpose of changing the return convention of IntlCalendar
> methods, and is it worth?
>

The purpose is to have the "correct" return type. Is it worth it? Well, it
depends... For me, what's important is that PHP becomes a more and
more predictable and accessible language, and I care less about minimizing
backward compatibility breaks.

The introduction of the "true" type was useful, since we now have more
correct type information, but adding new functions with this return type
would be unfortunate, as many people (especially the ones coming from other
languages) would be puzzled about the meaning of
the "true" type more than if the return type was "void". I'm pretty sure
that George - the author of the true type - agrees that the main purpose
of this type is to eliminate its declarations from internal functions and
methods.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Claude,

The replacement methods for IntlCalendar::set() (namely
> IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a
> return type of `void`, but `true`, like the original method, for the two
> following reasons:
>
> 1. By changing the returned value, you are introducing an unnecessary
> hazard when migrating code.
>
> 2. Per https://www.php.net/IntlCalendar, all modification methods of that
> class (clear(), roll(), setTime(), etc.) consistently return `true` on
> success and `false` on failure, even when the method is infallible (and
> thus would always return `true`). Don’t break consistency.
>

1.  I don't see much risk with this change, since we clearly indicate at
the very beginning that the new methods return void, so programmers
migrating their code
should pay attention to the return types. IDEs and static analysers can
surely warn them in case of inattention.

2. Some of the methods you mentioned have a "true" return type for a few
weeks now. The biggest motivation for introducing these was to at least
have some chance to
make them void in the future. Doing so is a risky move indeed, so should be
carried out very carefully, if it's possible at all... That's why I
consider it beneficial to spare the
effort and start with a clean slate by adding the new methods in question
with their correct return type.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hi Michał and Larry,

As Tim has already clarified, using literal strings in the left-hand side
of "clone with expressions" won't cause any issues
for IDEs and static analysers to identify the correct property. I got rid
of the shorthand syntax from the proposal because
it is not strictly required (since it has an equivalent alternative), and I
acknowledge the fact that there are certain people
who don't want to have two syntaxes to express the same thing. If it turns
out that we really want to have the short-hand
syntax as well, we can always add support for it later.

Conversely, using the named arguments style syntax (what future scope calls
> a shorthand, which I don't think is accurate as it's nearly the same amount
> of typing)


I'm just using the same wording as Nikita used in his named argument RFC.
Please take a look at the following section:
https://wiki.php.net/rfc/named_params#shorthand_syntax_for_matching_parameter_and_variable_name
Especially this sentence is important:

If I wanted to put these ideas into a general framework, I think one way to
> go about this would be as follows:
> - Consider identifier: $expr as a shorthand for "identifier" => $expr.


Even though the shorthand syntax is only 4 characters shorter than the
normal one (including the missing space before the colon),
for a typical property name which may be 4-8 characters long (I guess),
it's a significant reduction of typing.

That approach, taken now, gives us all the flexibility people have been
> asking for, reuses the existing named arguments syntax entirely, and is the
> most refactorable option.
>

Your suggestion is very interesting/promising... However, I'm a bit worried
that if I only supported the "identifier: expression" syntax for
"clone assignments", and I required people to pass an array (or splatted
array) instead of the "expression => expression" syntax,
they would start complaining why they cannot directly use expressions as
property names... That's why I think all the alternatives are useful
on their own, but only the "expressions => expression" syntax is
vital, since all the other use-cases can be written like this.


> Additionally, the current "property names expression" section shows a very
> odd example.  It's recloning the object N times, reinvoking the __clone
> method each time, and reassigning a single property.  If you're updating
> several properties at once, that is extremely inefficient.  Dynamic
> property names are not the solution there; allowing the list to be dynamic
> in the first place is, and that should not be punted to future scope.
>

I'm aware that it's currently inefficient, but it's the case only when you
want to update a dynamic list of properties. In my opinion, you update
a fixed set of properties most of the time
(e.g. Psr\Http\Message\ResponseInterface::withStatus()), and it's only a
rare case when you need
more dynamic behavior. That's why I believe it's not a huge problem to keep
cloning objects unnecessarily until we add support for the stuff
mentioned in the future scope section.

I am also confused by the choice to make __clone() run before with with
> properties.  I would have expected it to be the other way around.  Can you
> explain (in the RFC) why you did it that way?  That seems more limiting
> than the alternative, as you cannot forward-pass information to __clone()
> this way.
>

To be honest, the current behavior seemed like the natural choice for
me, and I didn't really consider to execute the __clone() method after the
clone assignments.
Do you have a use-case in mind when you need to forward-pass information to
__clone()?

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hi Everyone,

In the meanwhile, I changed my proposal to use [] instead of {} after the
"with" clause due to its better receptance.
Additionally, I removed support for the shorthand "property assignment"
syntax (clone $this with [property1: "foo"]) in
favor the more powerful one where the left-hand side supports expressions
(clone $this with ["property1" => "foo"])
so that we have more time to decide whether the former one is really needed.

After talking with Nicolas and Alexandru off-list (I accidentally didn't
click "Reply All" in my response), we identified
that their "clone callback" proposal could also be a future-scope of the
current RFC with the following syntax:

clone $this with function (object $clone): void {
$this->property1 = "foo";
}

In my opinion, this syntax is OK, albeit it's a bit long to write and a bit
difficult to decipher its meaning (maybe it's
just me). That's the main reason I prefer my originally proposed syntax.
Apart from this, I don't see a big issue with it,
neither conceptually, nor with the implementation.

Regards,
Máté

[PHP-DEV] [RFC] [Discussion] PHP 8.3 deprecations

[Máté Kocsis]

Hi Everyone,

Together with multiple authors, we'd like to start the discussion of the
usual
deprecation RFC for the subsequent PHP version. You can find the link below:
https://wiki.php.net/rfc/deprecations_php_8_3

Regards:
Máté Kocsis

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Everyone,

Yesterday, I updated the voting choices based on Rowan's suggestions. I
also picked a few signatures where
only one migration path is proposed (the short one).

Now, I'd be curious what to do with the FFI methods in question (
https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures#fficast_ffinew_and_ffitype).
..
Especially, it would be very nice to hear about FFI users' opinions.

Also, there was feedback about the session_set_save_handlers() function
name that it shouldn't resemble this
much to the original session_set_save_handler(). Rowan suggested using the
session_set_save_handler_functions() name.
While I'm not a big fan of this choice, I'd be interested in if there is
any other idea?

Particularly, I have been wondering for a long time, why the original
function includes "save_handler" in its name?
The passed in handlers are not just "save", but also other kinds of
handlers (e.g. "read"). So I'm considering to use
something like "session_set_handlers()" or
"session_set_handler_callbacks()". What do you think about these names?

If we manage to answer these questions then I'd like to start the vote not
long afterwards.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

>
> So I would suggest rewording the options slightly:
>
> a) Deprecate in 8.3, remove in either 9.0 or 10.0
> b) Deprecate in 8.3, remove in 10.0
> c) Do not deprecate
>
> Now if the votes are a:5, b:4, c:4, we can say:
>
> - 9 people voted for deprecation in 8.3, vs 4 against
> - only 5 voted for removal in 9.0, vs 8 against
> - 9 voted for removal in 10.0, vs 4 against
>
> So we conclude that we should deprecate in 8.3, and remove in 10.0
>

While this vote format looks a bit confusing for me at the first sight, I'm
OK to apply it. Although, I don't fully agree with
the "deprecate in 8.3 and remove in 10.0" part due to the reason I wrote
about in my previous email.

In my opinion, if we want to keep a signature for around 5-7 more years
then we should really wait ~2 years with the
deprecation so that people can react voluntarily first. Libraries have to
get rid of deprecated calls in any case, otherwise
their users will surely start to complain. If they do fix these calls in
their PHP 8.x compatible code then there's much less reason
to postpone the removal with 5 more years.

The suggestion to narrow it down to a yes/no proposal in the discussion
> phase is probably even better, though.
>

I agree with this, but so far there was no feedback which functions/methods
people want to deprecate slower or faster,
so I'm eager to hear about your and everyone else's opinion!

Replying to Larry:

That these particular functions and uses are quite rare doesn't really
> change any of that; if anything it strengthens it, that we're willing to be
> graceful about it even in cases where we could probably get away with not
> being so.


I don't think this "generosity" is really necessary, even though I
appreciate your intention. For example,
in case of ldap_connect(), the signature to be deprecated is not even
documented at php.net... and it's only available on
a very specific platform. I see no reason to keep this signature for 5+
years. The same goes for IntlGregorianCalendar::__construct()
which seems to be a very underrepresented class as well.

As I already said, most of the functionality to be removed is easy to
replace, and it could be done via automation:
i.e. you add Symfony Polyfill to your project and then Rector swaps the
problematic function calls to the good ones. Sometimes
not even tooling is needed, a simple search + replace is enough. With all
that said, I think only a smaller subset
of functions may deserve the longer depreciation period.

Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Dan and Tim,


> The RFC author could just say that yes votes for deprecation and
> eventual removal will be added together, with the timescale being a
> preference vote.
>

Thank you, Dan, for the  clarification, that was indeed what I meant. I'll
also
make this clear in the RFC, unfortunately I forgot it so far.

… the following: I wanted to know if


> "Deprecate in 8.x + not remove in 9.0, but only remove in 10.x or 11.x"


Yes, this is also a possible choice. Actually, I'm sure there were multiple
things deprecated in 5.x,
which only got removed in PHP 8.0. At least this is what I remember. :)

The reason why I still opted for the possibility of "deprecate in 9.0 and
remove in 10.0",
because in my opinion this choice would suit better some of the most
widespread functionalities.
Not deprecating them formally straightaway in 8.x versions, but only
providing a suggested
alternative instead of them, would first of all buy some time for people
who don't yet want to deal with this problem at all, but still allow them
to voluntarily migrate
when they are ready without nagging them. Then they had a relatively long
time period when we would
warn them a bit louder. A full major release cycle should be more than
enough time to perform the migration.

With all that said, personally I don't think any of the affected
functionalities (maybe with the exception
of only one of them) warrant such a long phasing out period.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Derick and Tim,

Derick, I incorporated most of your suggestions into the RFC with the sole
exception of the argument
names ($start/$finish,$begi/$end): it would feel odd if these parameters
and their related properties
were called differently:
https://www.php.net/manual/en/class.dateperiod.php#dateperiod.props.start

Actually, when we renamed the majority of parameters together with Nikita,
this was one of few rules that we had:
parameters should be called the same way as the property they initialize.


> Please no consecutive uppercase letters in the function name. I find
> createFromIso8601 (or createFromIso8601String if find the 'String'
> suffix useful) much more readable than createFromISO8601String.
> Uppercase acronyms are especially bad if followed by another ucfirst
> word, because the word boundary is not immediately visible there. Not
> the case here (it's digits), but still bad.


Yes, generally, I wholeheartedly agree, but unfortunately I still "had to"
go
with Derick's suggestion of using consecutive uppercase letters because
there
is already a DateTime::setISODate() method with the same casing and I want
to
follow the established naming convention of ext/date. Personally, I like
the "String"
suffix, probably it makes the name better, but since I don't really care
about this
problem too much, I'd leave it for Derick to decide about it.

Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Dan (and Larry and Tim who responded to the same topic),

Would leaving the current versions in place and working actually cause
> any maintenance issues?


Yes, unfortunately they cause some maintenance issues: since they don't
have a single
signature, gen_stub.php generates the wrong signature for them in the
manual. For example
https://www.php.net/manual/en/function.stream-context-set-option.php
displays two distinct
signatures, however gen_stub.php only knows one signature, so obviously
generation won't work well:

function stream_context_set_option($context, array|string
$wrapper_or_options, ?string $option_name = null, mixed $value = UNKNOWN):
bool {}

I use gen_stub.php for automatically detecting the outdated
function/method/class signatures which I can then
commit with minimal effort. Actually my doc-en PRs (
https://github.com/php/doc-en/pulls?page=1=is%3Apr+is%3Aclosed+author%3Akocsismate
)
are either fixing outdated information in the manual, or ensuring that
gen_stub.php can generate the same
information that we display in the docs. Now, there are only ~50-60
function/method signatures
in master which are not in line  with the output of gen_stub.php, and most
of them are due to
overloaded signatures (the rest is due to PHP 8.3 changes which shouldn't
be documented yet).
For reference, the number of outdated function signatures used to be many
hundreds or probably 1000+
when the project started, just before PHP 8.0 came out.

If not, then we could just move really slowly on deprecating them. We
> could deprecate them at some point in the 9.x release cycle and remove
> them in 10.0.


Although I would love to get rid of as many overloaded signatures as
possible due to the above mentioned fact,
I admit that there might be some signatures which other internals would
prefer to phase out slower than me. So
thanks for the idea! Now, the individual votes in the RFC contain 2 options
for removing a signature: a shorter path (deprecation
in PHP 8.4, removal in PHP 9.0) and a longer one (deprecation in PHP 9.0
and removal in PHP 10.0).

Additionally, I wrote a very explicit and detailed list about the suggested
BC compatible alternatives for each deprecation so that
now it should be very clear how difficult it is to replace a
function/method invocation.

The RFC says:

 Impact analysis: 1 out of the 2000 most popular PHP packages rely on
> calling session_set_save_handler() with 6 or more arguments.


> I doubt analysing github is going to give a useful measure of the
> impact of this RFC. Functions like session_set_save_handler are going
> to be used in custom code written for a company than in shared
> libraries.


Yes, I'm aware of this glitch, but that's the best I could provide for
estimating the impact of the deprecations.

Cheers,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Rowan,


> As an aside, I don't think "other deprecations are already more difficult"
> is a good argument - it's like saying "yes, I punched him, but not as hard
> as someone else already had". I think the change can be defended from other
> angles, but wanted to call that out.
>

I admit that my argument was not a good one, but I intended to mean that
most of
the suggested alternatives are not too difficult to implement: since
numerous other proposals
were accepted in the past with a much more difficult upgrade path, I think
this aspect
of my RFC shouldn't be a deal breaker considering the benefits.

Maybe because the function names are so similar? In general, I hate
> variable and function names that differ only by an "s", it's far too easy
> to misread or mistype them. Maybe "session_set_save_handler_functions"?


To be honest, I'm not a fan of using the longer names
(session_set_save_handler_functions
and stream_context_set_option_array), especially the latter one looks weird
to me, and it
doesn't go well with stream_context_get_options().

Besides, a most interesting development is that I've just discovered the
stream_context_get_params()
and stream_context_set_params() functions which do nearly the same what the
stream_context_*et_option
functions do: the only difference between them is that the latter functions
wrap the options inside the
"options" key and also has an additional "notification" key.

It *is* strictly related, though: the current function has two purposes:
> get an option, and set an option; the RFC proposes to split that into two
> functions, and there are three ways we can do that:


Thanks for the idea, your explanation made sense to me, so finally, I
removed assert_options() from
my proposal and the deprecation of assert_options() relies on the PHP 8.3
deprecations RFC.

Regards:
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Rowan,

As Kamil says, a potential 1-year deprecation is probably not enough,
> and we need to keep in mind that many libraries and applications need to
> support multiple versions of PHP at once. If they are to become errors
> in 9.0, there should be some way to write code that will run on both 8.0
> and 9.0.
>

Yes, I agree that we should make the upgrade path of libraries as easy as
possible,
and I also agree that probably a 1-year deprecation is not enough.. although
the suggested alternatives would be available 2 years in advance of the
removal
so this would allow a 2 year window for fixing code using the deprecated
functionality.


> The only ones in this list I can see that being easy for are
> assert_options (you can write a polyfill for the new assert_set_option,
> and switch all code to use that), and get_class() / get_parent_class()
> (you can start passing $this right away).
>

As far as I see there are more deprecations which are easy to fix. In order
to see the whole picture
better, let's group the functions based on how a possible upgrade path
would look like:

- With existing suggested alternative:
- get_class() and get_parent_class()
- pg_fetch_result(), pg_field_prtlen(), and pg_field_is_null()
- Phar::setStub()
- ReflectionMethod::__construct()
- ReflectionProperty::setValue()
- session_set_save_handler() (because the OO and procedural style are
interchangeable)
- stream_context_set_option()
- Polyfillable:
- assert_options()
- ldap_connect()
- ldap_exop()
- Requires conditional version check:
- DatePeriod::__construct()
- IntlCalendar::set()
- IntlGregorianCalendar::__construct()

(I intentionally did not include the FFI related methods, since this
section is TBD)

As it can be seen, most deprecations have an existing alternative. The 2nd
category (polyfillable ones)
mostly contains very little used functions, especially because the
deprecation of ldap_connect() only
affects PHP compiled with OracleLDAP.

The category which requires conditional PHP version checks is indeed the
most problematic one.
Probably it's not a surprise that it only includes methods because it's not
possible to polyfill methods
according to my knowledge. The good news is that IntlCalendar::set() and
IntlGregorianCalendar::__construct()
seem to be very little used functionality. So in my opinion,
DatePeriod::__construct() has the worst upgrade path,
the rest are either easy or shouldn't cause much issues. Since the
constructor is set to be removed completely
based on Derick's preference, we should discuss with him whether it's worth
to leave the
public function __construct(DateTimeInterface $start, DateInterval
$interval, DateTimeInterface $end, int $options = 0) {}
signature alive so that people can change their code to use this instead of
the other two signatures.

I have particular concerns about session_set_save_handler. The impact is
> hard to estimate, because it will be used directly in applications more
> often than in libraries. The multiple parameter signature is much older,
> dating back to PHP 4, with the object form added only in PHP 5.4. That
> may seem a long time ago, but there is currently no incentive to change
> existing code between the styles, and doing so requires a reasonable
> amount of work.
>

Yes, but changing session_set_save_handler() to session_set_save_handlers()
should be a reasonably small effort, isn't it? I understand that it's going
to affect
lots of codebases, however other PHP 8.x deprecations are much more
difficult
to fix than this one would be.


> On a different point, I think "assert_options" is a peculiar name for
> either setting or getting a single option, and would suggest it be
> replaced with two new functions, assert_get_option and
> assert_set_option. Replacing both variants also makes it easier for
> users to find everywhere they've used it, and polyfill both variants,
> rather than having to examine each.
>

Yes, I agree that the assert_options() name is at least weird but I
wouldn't like to
include changes into this RFC which are not strictly related to overloaded
signatures. Just like in case of implode(), the 1-parameter version of
assert_options()
could be added to the PHP 8.3/8.4 deprecations RFC though.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Kamil,

Thank you for your reply, it is pretty much appreciated!

I think one year of deprecation is not enough. I believe the functions
> that get replacements should be deprecated immediately in PHP 8.3 to
> give people two years of deprecation notice. It doesn't make sense to
> me to add a replacement but not deprecate the old variant.
>

Yes, this upgrade path also makes sense to me, and I'm happy to go with
it if others don't disagree! Probably my approach would be advantageous
if we were earlier in the 8.x lifecycle.


> `($rm = new ReflectionMethod(explode(“::”, $string));) ` I think you
> forgot a splat operator here.
>

Fixed!


> ReflectionProperty::setValue could have an alternative called
> ReflectionProperty::setStaticValue. Then there would be no need for
> the unused parameter anymore.
>

Originally, my proposal had this method, but at last I removed it based on
Nicolas' suggestions. I think both ways make sense, a single setValue() is
probably much easier from the migration point of view, since the suggested
alternative is already there in older PHP versions. And it doesn't mean
we cannot have a ReflectionProperty::setStaticValue() later on. :)


> The overload of array_keys should be replaced with a new function
> array_search_all. This overload is little known and very confusing.
>

Thanks for pointing this out, I'll look into this soon!


>
> The overload of ldap_exop should just be deprecated. An alternative
> already exists and is the preferred way.
>

I've just included ldap_exop() in the RFC, albeit slightly differently to
your
suggestion: since this function performs the extended operation
synchronously
or asynchronously based on whether the $response_data parameter is provided,
we cannot simply deprecate and remove the latter one, because as far as I
can understand, synchronous communication is also a valid use-case. So I
went
with adding an ldap_exop_sync() function.


> I wasn't aware that pg_execute has an overload apart from the default
> connection one. Could you explain what that overload is?
>

You are right, as it turned out for me, there is no other overload indeed,
so I removed
it from the RFC.


> stream_context_set_option should get alternative
> stream_context_set_option_array.
>

Thanks for the suggestion! I incorporated this suggestion into the RFC
(using
the stream_context_set_options name though).

What is the last item in Future scope supposed to be, because I think
> ReflectionProperty is a typo.
>

Right, it's now removed.


> Why is implode() not mentioned in this RFC?
>

Because it's not overloaded anymore. :) Even though the manual lists
multiple signatures
for it, its exact signature is the following:

function implode(string|array $separator, ?array $array = null): string {}

I'm not sure how much people would miss the possibility to use implode with
a
single argument ("implode($array)"), but if that's the case, we can add it
to the
usual PHP 8.3 or PHP 8.4 Deprecations RFC. I wouldn't like to add it to
mine,
because it's already way too long.

Regards:
Máté

[PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures

[Máté Kocsis]

Hi Internals,

As you have possibly already experienced, overloaded signatures cause
various smaller and bigger issues, while the concept is not natively
supported by PHP. That's why I drafted an RFC which intends to phase out
the majority of overloaded function/method signatures and also forbid
the introduction of such functions in the future:
https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures

I'm not sure at all what the best solution would be to get rid of the
overloaded FFI methods, so I hope that someone comes up with a better idea.
Currently, the RFC suggests deprecating and then removing the static
methods in question, but I hope that we can find a more elegant approach.
So this section is incomplete and it's just for starting the discussion.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hey Everyone,

Thank you for the lot of feedback! Sorry, I'm going to have to answer in a
single email otherwise I would have to send too many emails.

Alexandru wrote:

 How about just allowing a block of code after the clone statement that
> would execute it in the same context as the clone context, allowing to
> modify once readonly variables.
> Allows better flexibility compared with clone with syntax or clone method:


 This is an interesting alternative, but as Tim pointed out in a later
reply, the block would run in the private scope, which is contrary to my
intentions (and what I consider
the best solution).

Michał wrote:

I am curious if possible to implement the feature without using `with`
> keyword
> it visually could look pretty close to something like an object
> initializer in the future:
> return clone $this {c: 1};
> return new Bar {c: 1};


That's exactly what I tried first, but unfortunately, this syntax led to
parser ambiguities, so at last I had to settle on using "with".

Rowan wrote:

1) You mention in the Alternatives sometimes needing access to the
> original instance; it would be good to have an example of how this looks
> with the clone-with syntax.


Makes sense, so I've just come up with an example where a linked list of
objects are created. Let me know if you have a better example :)

2) How does this interact with an __clone() method? I'm guessing the
> __clone() would be called first, and then the with-clause applied?


Yeah, thanks for pointing this out! I agree that the clarification is very
much needed. BTW the evaluation order
is exactly how you wrote. This is now added to the RFC.

Tim wrote:

> In which order will __clone(), foo(), bar(), quux(), baz() be evaluated
> and what will happen if one of them throws an Exception? Will
> destructors of the cloned object run? When?


In fact, after the initial ZEND_CLONE opcode (which possibly invokes
__clone()), a separate opcode is generated for each
assignment (the newly added ZEND_CLONE_INIT_PROP). This means that foo(),
bar(), quux(), and baz() will be evaluated
in this very order. If any of them throws an exception, evaluation stops
and the assignments are not rolled back.

Regarding the destructors: yes, the destructor of the cloned object runs
immediately. In order to make sure, I've just added a test case:
https://github.com/php/php-src/pull/9497/commits/4d184f960ac1b5590d87739ee3278c13fac157de
I hope that this result is what
you expect.

Michal wrote:

> Just noticed the "Property name expressions" and am wondering if it could
> be a separate feature
> allowing for passing named arguments to functions/constructors in the same
> fashion?


As far as I can see, Nikita didn't propose the expression1() =>
expression2() syntax in the named params RFC due to
the same ambiguity I mentioned in the RFC (identifier vs. global constant).
But I don't think this is set in stone, however,
I do think that some optimizations would have to be disabled when param
names weren't evaluatable in
compile-time (https://github.com/php/php-src/pull/10831).

Andreas wrote:

What about argument unpacking?
> I don't know if we can combine this with ":" syntax or only with "=>".


For now, argument unpacking (property unpacking?) is not possible. But it
is definitely something that could be added in the future.

Tim wrote:

I'd rather see only the fat-arrow being allowed. Unless I'm missing
> something, braces with colon is not used anywhere else, whereas braces +
> '=>' is known from match() and '=>' more generally is already used with
> array literals [1]. Having two similar syntaxes for the same thing is
> not great when both are commonly needed is not great. They need to be
> documented and learned by developers.


I can only repeat what Rowan answered, since I agree with it completely:

I think it makes sense to have an unquoted form here, because the common
> case is that they are names which analysers can match statically to
> particular properties, not strings which will be analysed at runtime. There
> are plenty of places in the language where dynamic names are allowed, but
> we don't just use strings for the static case


However, I'm not completely sold on making "clone with" look like a
function call (return clone $this with (a: 1);), but
at least I like it more than using an array-like style (return clone $this
with [a: 1];). My intention with
using curly brackets (return clone $this with {a: 1};) is to highlight the
fact that it is a map
of key-value pairs, similarly how the JSON standard does so. Not to mention
that "clone with" serves a very
similar purpose to object initializers, and the different languages I know
to have this feature use
a similar syntax (Java: http://wiki.c2.com/?DoubleBraceInitialization, C#:
https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/object-and-collection-initializers#object-initializers
).

Regards,
Máté

[PHP-DEV] [RFC] [Discussion] Clone with

[Máté Kocsis]

Hi Everyone,

Quite some time after mentioning the "clone with" construct the first time
(at the end of the
https://wiki.php.net/rfc/write_once_properties#run-time_behaviour section),
finally I managed to create a working implementation for this feature which
would make it possible to properly modify readonly properties
while simplifying how we write "wither" methods:
https://wiki.php.net/rfc/clone_with

Regards,
Máté Kocsis

[PHP-DEV] Re: [RFC] [Vote] Typed class constants

[Máté Kocsis]

Hi Everyone,

The vote for the "Typed Class Constants" RFC is closed with the following
result:

The proposal was accepted with 24 yes and 0 no votes (100%).

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

Regards,
Máté Kocsis

[PHP-DEV] [RFC] [Vote] Typed class constants

[Máté Kocsis]

Hi Everyone,

I've just opened the vote for the "Typed class constants" RFC, which is
going to be open for 2 weeks (until 2023-03-13).

Link: https://wiki.php.net/rfc/typed_class_constants
Discussion: https://externals.io/message/119433

Regards,
Máté Kocsis

Re: [PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hi Claude,


> Although they are not explicitly mentioned, I assume that typed constants
> are also available on traits?
>

Yes, sure! I've just clarified the RFC by mentioning this fact as well as
adding an example.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hey Claude,

Sorry for my last response, I didn't understand you indeed... Enums really
do support self/static types. So I've just updated the implementation +
removed the restrictions
in question from the RFC + added a future scope section containing some
information about the existing restrictions.

Thank you very much for catching this! As the RFC text is now finalized, I
intend to start the vote on Monday.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hi Claude,

No, I didn't misunderstand you, I just didn't specify my answer enough:

Enums can be assigned to their own class constants because of their special
behavior, namely that enum cases are evaluated and then instantiated right
away,
during the compilation of the enum case. This is easily possible because
enum cases are scalars and cannot refer to external things.

However, you cannot do the same with class constants because the class
itself has to be registered after all the validations are done, plus the
global constant referring
to the class has to be evaluated too. Unfortunately, I am not able to
detail the problem any deeper, but I can tell you that your enum example is
not applicable for class constants.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hi Claude,

The RFC states that it is technically not possible to have a constant of
> effective type `static` or `self`. While this is probably correct for
> regular classes, this is not true for enums. The following code ostensibly
> works (https://3v4l.org/84W92):
>

To be exact, defining class constants with the class itself as value is
already impossible: https://3v4l.org/J7C30 So this isn't really a
limitation imposed by the current RFC, but a new yet mentioned restriction
of
https://wiki.php.net/rfc/new_in_initializers. I'll try to make this clear
in the RFC text.

Since the discussion has been settled for quite a long time, I'd like to
start the vote in the second half of next week, unless new concerns are
brought up. So it's time to review the RFC carefully :)

Máté

Re: [PHP-DEV] [RFC] [Vote] Readonly amendments

[Máté Kocsis]

Hi Everyone,

The vote for the "Readonly amendments" RFC is closed with the following
results:

Proposal #1 (Non-readonly classes can extend readonly classes):
The proposal was declined with 7 yes and 12 no votes (37%).

Proposal #2 (Readonly properties can be reinitialized during cloning):
The proposal was accepted with 26 yes and 0 no votes (100%).

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

Regards,
Máté Kocsis

Re: [PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hi Alexandru, Mark,


> 1. Why is object type not supported? I can't see a real reason and also
> there is no explanation why.
>

Sorry for this, mentioning object as unsupported was an artifact from the
original version of the RFC which
was created back then when constants couldn't be objects. After your
comments, I removed the object type
from the list. Thank you for catching this issue!


> 2. In the examples for illegal values, it would be good to explain why
> they are not legal.
>   I don't understand why "public const ?Foo M = null;" wouldn't be legal.
>   I think "?Foo" should work the same as "Foo|null" that would be legal.
>
> It was there due to the same reason as above. I removed this example now.

I had updated the RFC page, but it looks like the changes were reverted in
> December 2022. The updated version I was working on was:
> https://wiki.php.net/rfc/typed_class_constants?rev=1648644637


Yeah, the original author of the RFC was surprised to find your changes in
his RFC (https://github.com/php/php-src/pull/5815#issuecomment-1356049048),
so he restored his original version.
Next time, please either consult with the author of an RFC if you intend to
modify the wording, or you can simply create a brand new RFC - even if it's
very similar to the original one (just don't
forget to add proper references).

The updated RFC looks good, thanks for working on it. You may want to
> review the revised version I had worked on for implementation ideas, and
> review the previous conversations.
>

I also saw your proposal, but to be honest, I'm not that fond of the idea.
This doesn't mean though that you shouldn't create a new RFC or an
implementation, as others may find it useful. If you kick off
the project, I'll surely try to review your work.

Regards,
Máté Kocsis

[PHP-DEV] [RFC] [Discussion] Typed class constants

[Máté Kocsis]

Hi Everyone,

A few years ago, Benas Seliuginas announced the "Typed constants" RFC (
https://externals.io/message/110755) which apparently had a
positive reception overall.
Unfortunately, there were some issues with the implementation (namely, with
the parser)
so the RFC was stuck.

A few weeks ago, I reached out to Benas whether he intended to resurrect
the proposal, but
due to time constraints, he couldn't, and was OK with me continuing it.
With some help from
Bob Weinand, I managed to overcome the implementation difficulties, and
adapted it
to the newly added type-related features.

Please find the updated RFC here:
https://wiki.php.net/rfc/typed_class_constants.

Regards,
Máté

[PHP-DEV] [RFC] [Vote] Readonly amendments

[Máté Kocsis]

Hi Everyone,

We've just opened the vote for the "Readonly amendments" RFC, which is
going to be open for 2 weeks (until 2023-02-07).

Link: https://wiki.php.net/rfc/readonly_amendments
Discussion: https://externals.io/message/119007

Regards,
Máté Kocsis

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hey Tim,

Thank you. It would probably make sense to create a separate copy of the
> current version of the branch, possibly creating separate PR and then to
> drop the unrelated commits from #9497 - or alternatively picking the
> first 4 commits and creating a new clean PR from that. The latter is
> probably preferable. There's a large number of comments in the PR itself
> that do not relate to the feature-as-proposed-in-the-RFC.
>

OK, I've just splitted the implementation of the two functionalities, and
submitted
https://github.com/php/php-src/pull/10389, containing only the related
changes.

I'm seeing that Nicolas already spelled out the unset() part in
> __clone(), but I think it would also be useful to have that within the
> example code. It's more easily missed in text. Examples are cheap :-)
>

I've also changed the example to include the unset case as well. :)

Best regards
Máté

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hi Claude, Tim,

One shortcoming around readonly classes that I just figured out, is that it
> is not possible to use them as anonymous class:
>

Nice catch! As you wrote, it's indeed an oversight of the readonly class
implementation. Fortunately, we already have a PR with the fix. :)

I'm confused by the linked PRs for the implementation, because they do
> not appear to match what's proposed. Specifically the one for proposal
> #2 (allow modification in __clone) appears to include unrelated changes
> (a clone-with{} syntax).
>

Sorry for the confusion! The PR contains an implementation for the "clone
with" indeed
just because it builds on top of some specifics of the 2nd proposal in the
"Readonly amendments" RFC. However,
the first few (4) commits are related to allowing modification of readonly
properties in __clone().

Máté Kocsis

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hi Everyone,

As discussion apparently stalled, and since we managed to update the RFC
with the recently brought up arguments, we would like to start the vote
soon, possibly early next week, unless someone finds a new topic to discuss.

Máté

Larry Garfield  ezt írta (időpont: 2022. nov. 30.,
Sze, 20:35):

> On Wed, Nov 30, 2022, at 9:46 AM, Deleu wrote:
> > After reading GPB, Nicolas, Jordan and Larry's considerations, I no
> longer
> > have any objections to this RFC. Here is my summary of it all:
> >
> > - It's very easy for everyone to wrongly interpret readonly as somewhat
> > immutable, but it isn't (docs/education issue)
> > - LSP is about the writer of the child class, not about PHP
> > - If you don't want child classes to violate LSP, make your class `final
> > readonly`
> > - readonly as "constructor-init" properties mindset make it even stronger
> > the argument that child classes should be free to choose their definition
> > because constructors are special methods not bound by inheritance.
>
> Just for the record, my whole point is that "readonly as constructor-init"
> is wrong, and I am angry at the SA tools that have invented that out of
> whole cloth because it just breaks workflows that I am using very
> effectively and safely.  I always turn off that check in those because they
> are wrong.  That is *not* how the language feature is implemented, so
> making other language feature decisions based on that incorrect, artificial
> "rule" is highly dangerous.
>
> --Larry Garfield
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hi Marco,

Sorry for the very late reply!

As explained in
> https://github.com/lcobucci/jwt/pull/979#discussion_r1025280053, a
> consumer relying on a `readonly`
>
implementation of a class probably also expects replacement implementations
> as read-only.
> I am aware that we lack the `readonly` marker at interface level (would be
> really neat), but the practical example is as follows:
>

Regarding readonly interfaces: it would be a good idea given properties
were be able to be declared in interfaces. Then readonly
interfaces could adopt the same semantics as readonly classes have, so that
all *declared* properties become readonly.

Regarding the LSP violation: your example is about immutability (as the
name of the classes also suggest), but the readonly
keyword doesn't guarantee immutability. This is basically what Nicolas
answered not long ago to Larry:
https://externals.io/message/118554#118604

readonly class FancyPerson extends Person {
> private readonly stdClass $middle;


> public function setMiddle(string $mid) {
> $this->middle ??= new stdClass;
> $this->middle->name = $mid;
> }


> public function fullName() {
> return "$this->first $this-middle $this->last";
> }
> }


We proposed this change because it wouldn't break anything that's already
not "broken".

Regards:
Máté

Re: [PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hi Rowan and Alexandru,

I'm not totally clear what this sentence is saying:
>
>  > Reinitialization can only take place /during/ the |__clone()| magic
> method call, no matter if the actual assignment happens in a different
> method or function invoked inside |__clone()|.
>
> Are you saying that invoking a separate method like
> "setSomeProperty($blah)" from __clone() is allowed, or that it isn't?
> Perhaps a couple of extra examples could be added to this section
> demonstrating what would and wouldn't be allowed?
>

Sorry, I tried hard to articulate the behavior, but then the wording still
needs some clarification.
So this sentence was intended to convey that properties can be
reinitialized during the execution
of the __clone() magic method either if the assignment happens directly in
this method, or in any
method invoked by __clone(). Is it clearer now? Thanks for pointing this
problem out, and any help
with the wording is appreciated!

For proposal 2 part: "Readonly properties can be reinitialized during
> cloning"
> From the details there I understand it would affect properties in a
> readonly class as well as explicit

readonly properties in non-readonly class. Is that the intent? If yes,
> maybe we can share an example

without a readonly class as well.


I didn't bother to add a separate example for explicit readonly properties,
because I mainly regard
readonly classes as a shorthand for adding the readonly modifier for all
properties (even though I know
there are some other differences between readonly and non-readonly
classes). That's why I don't think
it's worth adding two different examples, but I'm ok with having one
example with simply readonly properties,
without the readonly class modifier. I think the example will be clear
enough this way.

Regards:
Máté

Re: [PHP-DEV] Re: Issues with readonly classes

[Máté Kocsis]

Hi,

> If my default assumption (and Larry's) was that such a class would be
immutable, it's fair to think that a non-trivial number of other
programmers may make the same faulty assumption, and making this
distinction obvious in the documentation would probably help.

Yes, it makes sense to mention this in the documentation, but fortunately,
the manual already makes the behavior clear at
https://www.php.net/manual/en/language.oop5.properties.php#language.oop5.properties.readonly-properties
:

"However, readonly properties do not preclude interior mutability. Objects
(or resources) stored in readonly properties may still be modified
internally:"

Following our discussion, Nicolas and I announced our proposal to fix two
of the above mentioned issues: https://externals.io/message/119007 Feedback
is welcome!

Regards:
Máté

Jordan LeDoux  ezt írta (időpont: 2022. szept.
25., V, 23:01):

>
>
> On Sun, Sep 25, 2022 at 10:57 AM Máté Kocsis 
> wrote:
>
>> Hi,
>>
>> I agree with Tim, and I also think that both reverting and making any last
>> minute fundamental change is not a good idea, especially
>> because people don't have a clear agreement about how inheritance
>> should work. Readonly classes is an optional feature, so anyone
>> who wants proxies to work can simply not add the readonly flag to their
>> classes. Of course, it's a bit more tricky for library code,
>> but package authors should be aware of this gotcha. Having that said, I'll
>> try my best to fix the current shortcomings for PHP 7.3.
>>
>> Regards,
>> Máté
>>
>
> I tried hard to make it clear that I don't think this makes it "broken",
> it was just a deviation from my expectations and memory, both of which can
> obviously be flawed. I was mostly looking for some kind of information
> about what the reasoning was for this, given that I obviously (since I
> didn't remember it) missed that part of the discussion. The distinction
> between "readonly" and "immutable" is subtle, but fair. This distinction
> should probably be pointed out quite explicitly in the documentation
> though.
>
> If my default assumption (and Larry's) was that such a class would be
> immutable, it's fair to think that a non-trivial number of other
> programmers may make the same faulty assumption, and making this
> distinction obvious in the documentation would probably help.
>
> Jordan
>

[PHP-DEV] [RFC] [Discussion] Readonly class amendments

[Máté Kocsis]

Hi Everyone,

Following Nicolas' thread about "Issues with readonly classes" (
https://externals.io/message/118554), we created an RFC to fix two issues
with the readonly behavior: https://wiki.php.net/rfc/readonly_amendments

Please let us know your thoughts!

Máté

Re: [PHP-DEV] Re: Issues with readonly classes

[Máté Kocsis]

Hi,

I remembered the same thing, and am similarly baffled. How did the RFC pass
> if you can do something as simple as `public readonly stdClass $var;`? I
> thought I followed the discussion on that RFC, but apparently I missed
> something. I would have expected an example like above to block acceptance
> of the RFC. To be clear though, I'm mostly confused about what the
> convincing argument about this was, or if it was something that everyone
> else viewed as an uncontroversial aspect?
>

The readonly class RFC doesn't really bring anything new to the table, just
complements the readonly property RFC. The latter RFC
explicitly mentions that interior mutability of properties is not
restricted in any way. In my opinion, this is a wise and pragmatic approach,
so that use-cases like the body stream of PSR-7 remain possible. Please
note that the feature is called "readonly", not "immutable".

What's your take about 8.2? As I demonstrated, readonly classes are broken
> because of this propagation to child classes. Does that mean we should
> remove this constraint from 8.2? What about reverting the feature and
> considering it again for 8.3, after fixing it?


I agree with Tim, and I also think that both reverting and making any last
minute fundamental change is not a good idea, especially
because people don't have a clear agreement about how inheritance
should work. Readonly classes is an optional feature, so anyone
who wants proxies to work can simply not add the readonly flag to their
classes. Of course, it's a bit more tricky for library code,
but package authors should be aware of this gotcha. Having that said, I'll
try my best to fix the current shortcomings for PHP 7.3.

broken. see thread


I may be biased, but neither I would call the whole feature "broken". It
works as intended for many regular use-cases. However, as you
highlighted in the thread, there are some use-cases with which the
interoperability of readonly classes is suboptimal, to say the least.
This is because the RFC was designed with a conservative approach (e.g.
static properties and inheritance by non-readonly classes
are forbidden). This way, we'll be able to iteratively improve the feature,
while we minimize the risk of introducing undesirable behavior
which would be difficult to get rid of later. I know that there is a thin
line between "conservative" and "incomplete", but I do hope that
PHP 7.3 will be the first version where readonly properties become actually
useful.

Does anyone else recall this?  Máté, are Jordan and I just imagining
> things?


It may be possible that the discussion extended to this question, but
neither me (author of the previous readonly properties RFC), nor
Nikita had intention to restrict the usage this way due to the reasons
detailed in the first paragraph.

Regards,
Máté

Re: [PHP-DEV] Re: Issues with readonly classes

[Máté Kocsis]

Hi Nicolas, Larry,

Mate, WDYT?
>

When I was working on the readonly class RFC, I realized that the readonly
keyword very naturally fits services besides value objects. So my
expectation has been that until we can fix the issue with cloning, people
would mainly apply readonly to services. Not that it is very useful, but I
would also feel some kind of weird fulfillment by doing so.

Regarding cloning: I created a WIP PR not long ago to fix the
aforementioned cloning issue, and I'll pursue a readonly amendment RFC in
the coming weeks (or month) containing the long awaited improvements for
cloning (hopefully together with the "clone with" construct) and possibly
with this inheritance-related change Nicolas proposed, unless someone can
come up with an ultimate counter-argument.

Regards:
Máté

[PHP-DEV] Re: Issues with readonly classes

[Máté Kocsis]

Hi Nicolas,

For 2.: static properties are useful e.g. to cache some shared state (info
> extracted from reflection in my case) and I really don't see why readonly
> classes should forbid static properties. Readonly is only useful for
> instances, because it gives them immutability, but what's the link with
> static properties? Note that this is less important than the previous point
> as it's always possible to store shared global state in another class. But
> still, this feels arbitrary to me.
>

First, I'll answer to your question about static properties, because the
answer is simple: as readonly static properties are not supported (
https://wiki.php.net/rfc/readonly_properties_v2#restrictions), I had to
decide what to do with static properties of readonly classes (
https://github.com/php/php-src/pull/7305#issuecomment-971381308), so Nikita
and I agreed that forbidding them is the better option until we add support
for static readonly properties. In my opinion, being able to set readwrite
static properties for readonly classes would be very counterintuitive.

But I'm also wondering about B: the RFC doesn't contain any rationale about
> why this restriction exists (it says "Similarly how overriding of readonly
> properties works", but the similarity is quite weak, readonly props don't
> apply to other properties of child classes.) If there is no compelling
> reason to have this limitation, it should be removed IMHO. Basically, this
> would mean that child classes of readonly classes wouldn't have to be
> readonly themselves (but the existing properties on the parent would have
> to be of course.)


Regarding your main question: I understand your problem with readonly
classes, and I'd be happy if we found a solution which fits your use-cases
and keeps consistency for the engine at the same time. To give you more
context about the inheritance related restriction in the RFC: I went with
forbidding the extension of readonly classes by non-readonly ones so that
the invariants of the parent (no dynamic or mutable properties are allowed)
don't occasionally get violated by the child. However, I cannot think about
a proper scenario now where this would cause any problems... I'm wondering
if anyone could come up with one?

And at last, regarding the interaction of readonly properties with
__clone(): I'll definitely think more about this topic in the following
weeks. Currently, it seems to me that readonly properties should be able to
be reinitialized once *during* cloning, but not afterwards. Is this in line
with what you want to achieve? If we once manage to add support for the
"clone with" construct then readonly property reassignment(s) due to "with"
arguments should also be allowed. But I'll need more time to also consider
any undesirable side effects/edge cases, which could apply, should we
settle on the approach described above.

Regards,
Máté

[PHP-DEV] Re: [RFC] [Vote] Readonly classes

[Máté Kocsis]

Hi Everyone,

I've just closed the vote: The RFC was accepted with 28 votes in favour, 7
against.

Regards,
Máté

Máté Kocsis  ezt írta (időpont: 2022. ápr. 27.,
Sze, 9:06):

> Hi Internals,
>
> I've just opened the vote about readonly classes, and I'm going to close
> it on 2022-05-11.
>
> Discussion thread: https://externals.io/message/116472
> RFC: https://wiki.php.net/rfc/readonly_classes
>
> Regards,
> Máté
>

[PHP-DEV] Declaring tidyNode properties as readonly?

[Máté Kocsis]

Hi Internals,

When I was working on making the properties of the tidy and tidyNode
classes declared (https://github.com/php/php-src/pull/8515), I noticed that
it doesn't make sense to write the properties of tidyNode, as the property
changes won't have any effect on the parsed HTML node. Christoph provided
an example to highlight the issue with this behavior:
https://github.com/php/php-src/pull/8515#issuecomment-1120400605.

In my opinion, the properties in question should be declared as readonly in
order to make
the behavior of tidyNode more predictable. Of course, this would be a BC
break, but I believe
the use-case is so niche and misleading that we can afford to prevent it
right away in PHP 8.2.

Please let me know what you think about this change, especially if you are
aware of any valid
use-cases regarding the modification of the properties of tidyNode.

Regards,
Máté

[PHP-DEV] [RFC] [Vote] Readonly classes

[Máté Kocsis]

Hi Internals,

I've just opened the vote about readonly classes, and I'm going to close it
on 2022-05-11.

Discussion thread: https://externals.io/message/116472
RFC: https://wiki.php.net/rfc/readonly_classes

Regards,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Readonly Classes

[Máté Kocsis]

Hi Thomas,

The attribute #[AllowDynamicProperties] should not be allowed for
> readonly classes.
>

 Nice catch and I do agree with you, so I will add this restriction.

Máté

[PHP-DEV] Re: [RFC] [Discussion] Readonly Classes

[Máté Kocsis]

Hi Internals,

As there's not much discussion, I will start voting on Wednesday.

Cheers,
Máté

Re: [PHP-DEV] [RFC] [Discussion] Readonly Classes

[Máté Kocsis]

Hi Everyone,

After quite a long pause, I'd like to revive the discussion of readonly
classes and possibly put it to vote in the coming week(s).

If I'm following, then this RFC is about 90% syntactic sugar for putting
> `readonly` on all properties, plus disabling dynamic properties.  That's
> the long-and-short of it, yes?
>

Yes, exactly!


> Does disabling dynamic properties offer any other advantages as have been
> discussed otherwise recently?  (E.g., performance.)
>

No, I'm not aware of any other advantages. The only reason why I included
this feature into the current RFC is because otherwise a readonly class
wouldn't be readonly. :)
The same applies for forbidding the declaration of static properties.

Is there a way to opt a given property back OUT of being readonly, or if
> you have a readonly class and need to add a single mutable property to it
> are you stuck adding `readonly` to all existing properties first?  (I'm not
> sure I'm suggesting having such a mechanism, mostly just clarifying.)
>

Yes, you are right. This is an inconvenient situation for sure, but neither
I can imagine any other way to back a property out of being readonly than
to remove
the readonly class modifier + add readonly property modifier where
possible. Of course, the readonly property modifier doesn't apply to the
internal state of objects...
so doing the above is not necessary if you have to modify a readonly
property which is an object.


> It's interesting that this also provides a backdoor way to force all
> properties to be typed, as a side effect.  However... that also means you
> cannot mark a class readonly if any of its properties are callables, since
> callables cannot be typed. (I guess you could use mixed and then docblock
> callable, which is a bit fugly but not a problem introduced by this RFC.)


Yeah, you are right, an ugly hack is to use the mixed type for properties
that cannot be typed otherwise (e.g. resource). I believe in case of
callables, one could use
the Closure type instead, especially since the first-class callable syntax
has been introduced.

I hope I managed to address your questions/concerns.

Máté

Re: [PHP-DEV] Automatic performance benchmarking: PHP 8.1 is ~30% faster than PHP 7.4

[Máté Kocsis]

Hi Patrick,


> Thank you for those stats. Would you have the chance to have some data
> including PHP 8.0 as well?
> Because without PHP 8.0 data, we can't deduce anything about what PHP
> 8.1 brings over PHP 8.0 and or 8.0 over 7.4.
> Virtually, 8.1 may even be slower than 8.0 if the big increase of
> performance happened in 8.0.
>

Have you seen the results page? It is available at
https://github.com/kocsismate/php-version-benchmarks/blob/main/docs/results/2021_11_25_10_08_1_aws_arm64_c6g_4xlarge/result.md
and it lists the exact results for each release. Besides, the PHP 8.1
release page (
https://www.php.net/releases/8.1/en.php#performance_improvements)
displays the performance improvement based on PHP 8.0 and also includes
some of the optimizations which led to the boost.

Cheers,
Máté

Re: [PHP-DEV] Automatic performance benchmarking: PHP 8.1 is ~30% faster than PHP 7.4

[Máté Kocsis]

Sorry Folks, but I have to provide some update about the results:

Unfortunately, due to a silly bug in my calculator code, the % differences
in the benchmark results were slightly off from their real values, so I
have just retroactively adjusted them.
In fact, the Symfony demo app on PHP 8.1 is ~23% faster than on PHP 7.4,
while Laravel runs ~21.5% faster on PHP 8.1 than on PHP 7.4. In my opinion,
this performance increase
is still very remarkable, so I'm not disappointed at all that the
improvement is slightly less dramatic than I previously suggested. In any
case, sorry for the misleading news!

I wish a happy PHP 8.1 release party to everyone:
Máté

[PHP-DEV] [RFC] [Discussion] Readonly Classes

[Máté Kocsis]

Hi Internals,

I'd like to propose adding support for readonly classes:
https://wiki.php.net/rfc/readonly_classes

The implementation heavily builds on the semantics of the already accepted
readonly properties RFC (https://wiki.php.net/rfc/readonly_properties_v2),
basically the only thing I had to implement in the PR are the following:
- Any declared property of a readonly class is implicitly treated as
readonly
- Creation of dynamic properties is forbidden

Readonly classes would make it easier to declare immutable(-alike) classes,
especially if a class
contains a lot of properties. As a side-effect, one could already opt out
from dynamic properties
completely, which won't really be possible for quite some time, even if
https://wiki.php.net/rfc/deprecate_dynamic_properties passes.

Regards:
Máté

Re: [PHP-DEV] Automatic performance benchmarking: PHP 8.1 is ~30% faster than PHP 7.4

[Máté Kocsis]

Thanks all for the answers!

> AWS is meant to have credits for open source projects:
https://aws.amazon.com/blogs/opensource/aws-promotional-credits-open-source-projects/

Nice! An AWS support representative suggested me that I should apply for
AWS Activate (https://aws.amazon.com/activate/), which clearly doesn't seem
the best program for my purpose, so I'll have a try instead with the
promotional credits. Thanks for the idea!

> The link to the results in the readme points to
https://kocsismate.github.io/php-version-benchmarks/index.html which is
empty. Maybe this is known.

Thanks for the heads up, I forgot that this URL is linked when I posted the
benchmark. This is the page where I'd like to display the visualization I
mentioned
in my previous message. I'll need some time until I finish it, but until
then, the current results can be accessed in an markdown format at
https://github.com/kocsismate/php-version-benchmarks/blob/main/docs/results/2021_11_11_09_20_1_aws_arm64_c6g_4xlarge/result.md

> For my sins, I can do some frontend stuff... are we talking CSS, SVG, and
maybe a bit of the old JavaScript? If so, feel free to contact me off-list,
> I probably can't do anything in the next week or two, but I'm
optimistically (naively) hoping I can clear some of my todo list :-)

Thanks for the offer, I'll DM you! :) What I already have locally is a
very-very basic chart of the most recent benchmark results. I currently use
chart.js (https://www.chartjs.org/)
and a fork of semantic-ui (https://fomantic-ui.com/), only because I am
familiar with these libraries, so I'm not very attached to them, if there
are libraries more suitable for the job.

> Overall against having it on the ML: I set up a filter to ignore those
pesky emails by Intel. As much as I appreciate their effort, they were
really noisy, and not really useful to the larger group.

I appreciate your feedback! I've just realized that there's probably as
many people looking forward to new benchmark results as who are not
interested in them at all, so I won't
propose sending a regular mail about the results to the internals list.

Regards:
Máté

Deleu  ezt írta (időpont: 2021. nov. 13., Szo, 13:40):

>
>
> On Sat, Nov 13, 2021 at 1:31 PM Craig Francis 
> wrote:
>
>> On Fri, 12 Nov 2021 at 10:19, Máté Kocsis  wrote:
>>
>> - Does anyone know how to get some sponsorship from AWS...
>>
>> Craig
>>
>
> You may fill out this form
> https://pages.awscloud.com/AWS-Credits-for-Open-Source-Projects to apply
> for 12 months credit. If I'm not mistaken it could be a recurring thing by
> re-applying every 12 months. It might be important to have a dedicated
> account only for the purpose of the benchmark.
>
> You can also read more about it here:
> https://aws.amazon.com/blogs/opensource/aws-promotional-credits-open-source-projects/
>
> --
> Marco Aurélio Deleu
>

[PHP-DEV] Automatic performance benchmarking: PHP 8.1 is ~30% faster than PHP 7.4

[Máté Kocsis]

Hello Internals,

I'm writing this email because lately, I've been working on an automatic
benchmarking framework for PHP, and I'd like to share some news regarding
it. The initial implementation
was sponsored by Craig Francis, and it was used for the evaluation of the
performance aspects of the is_literal() RFC. Since then, I fixed numerous
issues and implemented many new features, so with the very close advent of
PHP 8.1, the time has come to unveil it.

I'm sure that many of you still remember Intel's automatic benchmarks from
a couple of years ago (e.g. https://externals.io/message/89843#89843). I
loved these emails, so this project served as a great inspiration for me
to start working on something similar. I have to admit
though that I won't be able to replicate their extremely advanced setup.

My main goal was to develop a framework (
https://github.com/kocsismate/php-version-benchmarks) which is:
- fully automatic so that it can be easily run regularly, and the
benchmarks are reproducible
- it's possible to try it out locally via Docker, but it can be run in the
cloud (currently, only AWS is
supported), and the instance type is configurable
- supports different CPU platforms (X86-64 and ARM64), and advanced options
whether
turbo boost/hyper threading/deeper CPU C-states are enabled is configurable
- supports any version of PHP since PHP 7.4, including any git branches or
commits
- supports the most important PHP configurations, including whether
OPcache/JIT/preloading
is enabled
- supports multiple tests: currently, the micro benchmarks bundled with
php-src, as well as the
Symfony and the Laravel demo sites as "real-life" tests are included with
some customizability (number of warmups, iterations number, number of
requests)
- results are measured via using PHP-CGI which is a lightweight web server
with very a small overhead

The most current benchmark is available at
https://github.com/kocsismate/php-version-benchmarks/blob/main/docs/results/2021_11_11_09_20_1_aws_arm64_c6g_4xlarge/result.md
. I'm happy to share that the results suggest PHP 8.1.0 is around 28-32%
faster than PHP 7.4 in real-life tests, and a few percent faster when it
comes to micro benchmarks. The benchmark was performed on an ARM64 instance
because this platform provided much more stable results than X86-64-based
ones did, mainly due to their fixed CPU frequency.

Unfortunately, the benchmark is not yet suitable for detecting minor
performance changes between commits as the run-to-run variation of the
results is a bit too high; in some cases, it can be up to 1-2%. I hope that
this problem can be mitigated to an acceptable level in the future, but
most probably I'll need some help to achieve this. So any help is
appreciated!

Furthermore, I'd have some questions regarding this project:
- Would you welcome automatic emails on internals (or on a dedicated
mailing list), just like how
Intel did it? I'm not sure about the frequency, but in my opinion,
1-2/month would be a sensible one, given there is interest in it.
- Does anyone know how to get some sponsorship from AWS (or from some other
cloud
provider at last resort)? I already tried to apply for AWS Activate's free
credits, but my application was rejected due to different reasons
(normally, this program is available for startups). I have some hope that
running the benchmark on a dedicated instance would decrease variation of
the results, but enabling this feature is relatively expensive compared to
other costs, so I'd be very happy if I wouldn't have to sponsor all of
this, when I already dedicate a great deal of my time to the cause.
- As always, I'm happy to receive feedback and improvements (ideally along
with an implementation :) ). For example, currently I've started working on
some visualization and chart support, but frontend stuff is not my area of
expertise, so I'm progressing a bit slower than normally. Also, the
statistical foundations could be reviewed, and possibly improved.

Regards:
Máté

Re: [PHP-DEV] [RFC] Wall clock time based execution time

[Máté Kocsis]

Hi Deleu,

What a coincidence! I've just added my PR to the "PHP 8.2" milestone on
GitHub. That means, I'll definitely want to continue the work on this
proposal
as soon as I finish my previous tasks.

The reason that I delayed this RFC is that it lacked feedback and answers
to my latest questions. Also, Nikita's comment which you linked, made me
less
confident, so I didn't want to push the proposal any further until
the details were clarified. In the meantime, I started a few other RFC-s
which took all my time.
With all that said, I plan to ping the original discussion thread in the
following weeks/months.

Regards,
Máté

Re: [PHP-DEV] [RFC] [VOTE] is_literal

[Máté Kocsis]

Hi Nikita,

I performed a few other benchmarks in order to provide a little bit more
insights into the performance aspect of the RFC. My latest measurement is
using the same setup as the previous
one, although I made a few smaller changes in the process (e.g. changing
the order of the tests). So at the end, I got the following results:

- Laravel demo app: +0.1%
- Symfony demo app: +0.43%
- bench.php: +0.4%
- custom concat bench: +3.89%

The results are comparing the "literals" branch with the commit in master
on which Joe's work is based. As far as I remember, the simpler variant of
is_literals had a 0.1% slowdown
in case of Symfony, and 2.09% in case of the custom concat benchmark.
Personally, I think even the current numbers are acceptable, especially
because PHP 8.1 is going to be
roughly 30% faster than PHP 8.0 according to my benchmark in case of the
Symfony demo app. Laravel's result is very similar, but I don't remember
about the exact numbers.

Regards:
Máté

Re: [PHP-DEV] [RFC] [Vote] Adding return types to internal methods

[Máté Kocsis]

> This feels a bit different than the class case, in that implementing
> internal interfaces is common, while extending internal classes is mostly
> an artifact of us not adding enough "final"s in the early days.
>

I have to admit that this aspect of the feature wasn't accurately covered
by RFC, sorry for that. And I agree that interface changes will imply a bit
more effort to migrate; however, I believe this is for the good.
For example, until a few weeks ago, probably only very few people knew what
a custom SessionHandlerInterface::read() implementation should really
return in case of an error: neither our stubs, nor the manual
had it correctly. I "accidentally" learned about the issue (that false
should be returned in case of an error) because a test failed when I was
implementing this RFC.
Afterwards, I fixed the return type in the stubs as well as in the
documentation:
https://github.com/php/php-src/pull/6884/files#diff-c6492b1d4bee1c7c3eca01e4b30af39a438ba823efa220fe1d0e5868bd58586eR74

With all that said, I am ok to have some exceptions if it turns out that a
return type declaration is too disruptive for the ecosystem, but I'm
hopeful that the new return types - even for interfaces - will be beneficial
overall.

Regards,
Máté

Re: [PHP-DEV] [RFC] [Vote] Final constants

[Máté Kocsis]

Hi Internals,

I've just closed the vote, and the RFC is accepted with 28 votes in favor
and 4 against.

Regards:
Máté

Máté Kocsis  ezt írta (időpont: 2021. máj. 24., H,
10:14):

> Hi George,
>
> Being able to override interface constants by default might seem like a
> regression, but I believe making interface constant behavior consistent with
> class constants is worth this, and also, I don't see any other good way to
> fix the weird inheritance behavior Nikita highlighted than to do what the
> RFC
> currently proposes.
>
> Regards:
> Máté
>
>