Re: [PHP-DEV] [RFC] [Discussion] #[\Deprecated] attribute again v1.3

[Nicolas Grekas]

Hi Benjamin,

My PR for #[\Deprecated] attribute was in hibernation for a long while now
> and after some off-list discussion a few weeks ago I have decided to
> revisit it and asked Tim to help me out with the work.
>
> Tim has cleaned up the PR quite a bit and also worked in additional
> features such as #[Deprecated] support in stub generation.
>
> While there are still some small todos, at this point we want to restart
> the discussion about the RFC for inclusion in 8.4:
>
> RFC: https://wiki.php.net/rfc/deprecated_attribute
> PR: https://github.com/php/php-src/pull/11293
> Old discussion: https://externals.io/message/112554#112554
>
> Let me know about your questions and feedback.
>

Thanks for the RFC.

While I'd like to be able to replace as many annotations as possible with
attributes, the devil is in the details and here, I'm not sold about the
details:

   - I concur with others about the "since" property being missing.
   - We'd also need a "package" property so that it's trivial to know which
   composer package is deprecating.
   - The attribute class should not be final, so that packages could
   subclass, e.g. to define the "since" or "package" property once for all -
   or to define a custom constructor, etc.
   - We should be able to add the attribute to a class.

Yes, the "package" + "since" info can be put in the message itself, but
having a structured way to declare them is critical to 1. be sure that
authors don't forget to give that info and 2. build tools around that.

You're saying that these are not useful because the engine wouldn't make
anything useful out of e.g. "since".
That's true, although I'd suggest using them to prefix the final message.
If that's the policy around attributes for the engine, then I'm wondering
if the attribute shouldn't live elsewhere. Or if we shouldn't discuss this
policy.

I also anticipate a problem with the adoption period of the attribute: in
order to be sure that a call to a deprecated method will trigger a
deprecation, authors will have to 1. add the attribute and 2. still call
trigger_error when running on PHP < 8.next. That's a lot of boilerplate.

Personally, I'm not convinced that there should be any runtime side-effects
to the attribute. I'd prefer having it be just reported by reflection.

Nicolas

Re: [PHP-DEV][DISCUSSION] Fix mb_trim inaccurate $character

[Nicolas Grekas]

Hi

Le jeu. 4 avr. 2024 à 07:41, youkidearitai  a
écrit :

> 2024年4月4日(木) 6:30 Tim Düsterhus :
> >
> > Hi
> >
> > On 4/3/24 10:02, youkidearitai wrote:
> > > Therefore, I think require an RFC, I have written a draft an RFC that
> > > fixes these issues.
> > > https://wiki.php.net/rfc/mb_trim_change_characters
> >
> > I don't think this (widening the type and changing the default value to
> > obtain the *intended* behavior) requires an RFC. It's a bugfix, a bugfix
> > with a slightly larger observable impact than other bugfixes.
> >
> > Best regards
> > Tim Düsterhus
>
> Hi
> Thank you very much for comment.
>
> I would like to discuss this RFC with all of you, as we have not yet
> decided on the correct revision policy.
> I created the RFC for that purpose.
>
> Still waiting for your comments!
> Thank you.
>
>
I also don't think this requires an RFC. This is a minor design issue that
has only one possible solution, so there is little to discuss.

Cheers,
Nicolas

Re: [PHP-DEV] [RFC][Vote announcement] Property hooks

[Nicolas Grekas]

Hi Ilija, Larry,

Heads-up: Larry and I would like to start the vote of the property
> hooks RFC tomorrow:
> https://wiki.php.net/rfc/property-hooks
>

I would make 3 changes to the RFC if I were in charge :

   - I'd make $this->propName inside a hook access the parent property via
   its hooks if any. I'd do this because that'd provide
   encapsulation-by-default for inheritance. Aka a parent class could
   reasonably expect its child classes to get through its hooks. (I'd be fine
   with only allowing the "=" operator on this.)
   - I'd keep the special $field value to access the backing value. It's a
   clear way to signify that accessing the raw value is possible only in
   hooks, never in other methods.
   - And I'd consider forbidding any method calls in hooks except static
   methods. This would allow factorising functional logic without opening for
   side-effects on the object outside of hooks themselves.

I'm sharing this in case it rings a bell somewhere, but I can work with the
peculiarities of the current RFC and I must admit I didn't spend as much
time as you on the topic so my proposals might fall short, while yours to
actually work. Thanks for that :)

I'm in favor of the RFC.

Cheers,
Nicolas

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

[Nicolas Grekas]

Hi Gina, Máté

Máté Kocsis and myself would like to propose deprecating implicitly
> nullable parameter types.
>
> The RFC is available on the wiki at the following address:
> https://wiki.php.net/rfc/deprecate-implicitly-nullable-types
>

Thanks for the RFC.

I have the same concerns as Larry but I don't have any ideas on how to make
the migration more seamless, so personally I'm fine with deprecating in 8.4
and dropping in 9.0.

For the record, Symfony was using the reverse rule (removing the nullable
flag when the default was null already). This was done to reduce the
overall visual debt™, but the arguments in favor of your proposal are sound.

I ran php-cs-fixer on the codebase, you can see the patch here:
https://github.com/symfony/symfony/pull/53612

TL;DR, this is a +/- 2000 lines change that's quite easy to do.
I think the RFC is missing some impact analysis BTW, just to be sure voters
can look at some data to make a best informed decision.

There's one catch-22, which is that I had to manually fix non-optional
arguments with default values (as a reminder, they're deprecated too):
https://github.com/symfony/symfony/pull/53612/commits/fb9fa26102512090550f9137751f81677c06d5f9
Fixing them can be automated too but this is still a separate code
transformation. I think this should be mentioned in the RFC.

The impact on the ecosystem is going to be significant for sure, especially
because of the previous CS of Symfony. But at least the transition has
already started since we merged this in our oldest maintained version.

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] Add http_(get|clear)_last_request_headers() function

[Nicolas Grekas]

Hi Gina,

I would like to propose an RFC to add the functions
> http_get_last_request_headers() and http_clear_last_request_headers() to
> PHP to replace the magic variable $http_response_header.
>
> Link: https://wiki.php.net/rfc/http-last-response-headers
>


Sorry I missed this RFC.

In Symfony's HttpClient, we use stream_get_meta_data($h)['wrapper_data'] to
access headers, so there is already a way to get them using nothing but
local scope.

This makes me wonder why, in "rejected ideas", you wrote this?

> One suggested idea was to provide those headers via a by-reference entry
> to the stream context.


Maybe we don't need anything but to promote what works already (this
"wrapper_data" entry)?

Nicolas

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

[Nicolas Grekas]

Hello Mate,

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

Congrats for this result :)

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

- for curl - that's already supported by CurlHandle so this is seamless
- for DBA, because dba_list()  works by
resourceid
- for resources created by stream_socket_server() and stream_socket_accept()

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

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

Cheers,
Nicolas

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

[Nicolas Grekas]

Le sam. 20 janv. 2024 à 13:42, Máté Kocsis  a
écrit :

> > 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.
>

I've used it a few year ago in a stream filtering experiment:
https://github.com/nicolas-grekas/Patchwork-PHP-Parser/blob/master/class/Patchwork/PPP/AbstractStreamProcessor.php

I don't remember the purpose of the property though. This would need more
investigation.

Nicolas

Re: [PHP-DEV] [VOTE] [RFC] Final-by-default anonymous classes

[Nicolas Grekas]

Hi Daniil,

I've opened voting for the final-by-default anonymous classes RFC:
> https://wiki.php.net/rfc/final_by_default_anonymous_classes
>

I voted against the proposal because as I mentioned in the previous thread
on the same topic, this is a backward compatibility break that lacks ground
but will have impact.

Note that I voted even though I think the vote itself might be "illegal"
per our policies (neither did 6 months pass, nor does the proposal make
substantial changes to the previous one, to some definition of
"substantial"). If the vote itself isn't allowed per this policy, then my
vote is void of course.
https://wiki.php.net/rfc/voting#resurrecting_rejected_proposals

Sorry for being a bit legalist on the topic but principles matter. We have
policies and this vote is on the edge of two of them. That's a red flag for
me.

Nicolas

Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute

[Nicolas Grekas]

>
>
>>> śr., 3 sty 2024 o 08:12 Nicolas Grekas 
>>> napisał(a):
>>>
>>>> Hi Max,
>>>>
>>>> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
>>>> > functionality is already available for internal classes - userspace
>>>> should
>>>> > benefit from it, too.
>>>> >
>>>> > The RFC: https://wiki.php.net/rfc/not_serializable
>>>> > Proposed implementation: https://github.com/php/php-src/pull/12788
>>>> >
>>>> > Please let me know what you think.
>>>> >
>>>>
>>>> Regarding the inheritance-related behavior ("The non-serializable flag
>>>> is
>>>> inherited by descendants"), this is very unlike any other attributes,
>>>> and
>>>> this actively prevents writing a child class that'd make a parent
>>>> serializable if it wants to.
>>>>
>>>> To me, if this is really the behavior we want, then the attribute
>>>> should be
>>>> replaced by a maker interface.
>>>> Then, a simple "instanceof NotSerializable" would be enough instead of
>>>> adding yet another method to ReflectionClass.
>>>>
>>>
>>> This should be possible without ReflectionClass, see
>>> https://3v4l.org/N3fmO
>>>
>>
>> Sure.
>>
>> My main point is : why use an attribute? this should be an interface to
>> me. All semantics match an interface.
>>
>>
>>> From the serialization libraries you can find similar attributes
>>>
>>
>> Those a very different, because they tackle the problem from the
>> *external* angle: the attributes there describe how a system *external* to
>> the class itself should best serialize an object. There, attributes make
>> sense, because they enrich the description of the class without forcibly
>> telling what to do with the object
>>
>> But in the RFC, we're talking about the object deciding itself how it
>> should be (not) serialized. This is enforced and thus belongs to the
>> typesystem - not to an attribute.
>>
>
> But then what should implement the NotSerializable interface?
> If you want to ignore a string-typed property there would be no option to
> mark it with a NotSerializable interface
> Consider "baz" property in this example:
>
> class Foo {
> protected int $bar = 1;
> #[NotSerializable]
> protected string $baz = 2;
> }
>


The attribute is #[Attribute(Attribute::TARGET_CLASS] so I'm not sure why
you consider this as it's not mentioned in the RFC (and I'm not sure it
would make sense).

Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute

[Nicolas Grekas]

> Hi Nicolas,
>
> śr., 3 sty 2024 o 08:12 Nicolas Grekas 
> napisał(a):
>
>> Hi Max,
>>
>> Hi, I'd like to propose a new attribute, #[NotSerializable]. This
>> > functionality is already available for internal classes - userspace
>> should
>> > benefit from it, too.
>> >
>> > The RFC: https://wiki.php.net/rfc/not_serializable
>> > Proposed implementation: https://github.com/php/php-src/pull/12788
>> >
>> > Please let me know what you think.
>> >
>>
>> Regarding the inheritance-related behavior ("The non-serializable flag is
>> inherited by descendants"), this is very unlike any other attributes, and
>> this actively prevents writing a child class that'd make a parent
>> serializable if it wants to.
>>
>> To me, if this is really the behavior we want, then the attribute should
>> be
>> replaced by a maker interface.
>> Then, a simple "instanceof NotSerializable" would be enough instead of
>> adding yet another method to ReflectionClass.
>>
>
> This should be possible without ReflectionClass, see
> https://3v4l.org/N3fmO
>

Sure.

My main point is : why use an attribute? this should be an interface to me.
All semantics match an interface.


> From the serialization libraries you can find similar attributes
>

Those a very different, because they tackle the problem from the *external*
angle: the attributes there describe how a system *external* to the class
itself should best serialize an object. There, attributes make sense,
because they enrich the description of the class without forcibly telling
what to do with the object

But in the RFC, we're talking about the object deciding itself how it
should be (not) serialized. This is enforced and thus belongs to the
typesystem - not to an attribute.

IMHO

Cheers,
Nicolas

Re: [PHP-DEV] [RFC][Discussion] NotSerializable attribute

[Nicolas Grekas]

Hi Max,

Hi, I'd like to propose a new attribute, #[NotSerializable]. This
> functionality is already available for internal classes - userspace should
> benefit from it, too.
>
> The RFC: https://wiki.php.net/rfc/not_serializable
> Proposed implementation: https://github.com/php/php-src/pull/12788
>
> Please let me know what you think.
>

Regarding the inheritance-related behavior ("The non-serializable flag is
inherited by descendants"), this is very unlike any other attributes, and
this actively prevents writing a child class that'd make a parent
serializable if it wants to.

To me, if this is really the behavior we want, then the attribute should be
replaced by a maker interface.
Then, a simple "instanceof NotSerializable" would be enough instead of
adding yet another method to ReflectionClass.

Cheers,
Nicolas

Re: [PHP-DEV] [VOTE] [RFC] Final anonymous classes

[Nicolas Grekas]

Hello Nikita,


> > I've just opened voting for the final anonymous classes RFC @
> > https://wiki.php.net/rfc/final_anonymous_classes.
> >
> > Voting started now, and will run until December 18th 2023, 00:00 GMT.
>
> For the record, I've voted against this proposal because I believe it
> should have gone with option 2, that is to *always* make anonymous classes
> final.
>
> It makes very little sense to me that everyone needs to explicitly mark
> their anonymous classes as final just because there is a class_alias
> loophole that could, in theory, have been used to extend anonymous classes
> in the past. Especially given that there is no evidence of this "feature"
> being used in the wild (or if there is such evidence, it was not presented
> in the proposal).
>

You might have missed my message in the discussion thread,

see https://externals.io/message/121685#121690

There is such evidence (not in the RFC though).

Nicolas

Re: [PHP-DEV] Adding a donate link to the PHP website

[Nicolas Grekas]

Hi Ben

> On Nov 30, 2023, at 02:45, Andreas Heigl  wrote:
> >
> > On 30.11.23 09:39, James Titcumb wrote:
> >> On Thu, 30 Nov 2023 at 07:28, Andreas Heigl  andr...@heigl.org>> wrote:
> > [...snip...]
> >>I suppose that is actually nothing that an RFC can do as I imagine
> that
> >>everyone from the PHP Group needs to support this and even an RFC
> >>wouldn't legally be able to change anything in regards to that.
> >> Surely, everyone who has contributed (i.e. has voting karma) has the
> opportunity to vote, and therefore, if they choose not to vote, that is
> surely their choice. I don't know the ins and outs of it, but I'd have
> thought an RFC, well advertised, with plenty of time to ensure as many
> people can vote who have rights to.
> >
> > What I meant by that is that the members of "The PHP Group" are
> currently the copyright holders. From a legal point of view no RFC can
> change that. The only way to change that would be for the PHP Group to
> transfer their copyright to someone else. What an RFC *can* do though is
> *propose* that the PHP Group transfers their copyright to the PHP
> Foundation.
> >
> > Though I'm lo lawyer, so ¯\_(ツ)_/¯
>
>
> I have spoken at length with a lawyer about this, and the TL;DR version is
> that every contributor owns the copyright on their specific contributions,
> if the contributions are copyrightable. Some contributions (typo fixes,
> white space changes, etc.) aren’t copyrightable, but anything more
> significant that is the contributor’s own work belongs to them.
>
> In other words, even though the license statement says the copyright
> belongs to The PHP Group[^1] or Zend Technologies Ltd.[^2], technically,
> these copyrights only apply to the specific code contributed by these
> organizations or contributed by people for these organizations (through
> work-for-hire or by legally transferring the copyright to them).
>
> Contributing to an open source project is NOT an implicit transfer of your
> copyright to the project. To do this, every contributor needs to sign a CLA
> that specifies they are transferring their copyright to The PHP Group.
>
> What is implied, however—and I’m told this is how most courts in the US
> and outside would view it—is assignment of license. When someone
> contributes to an open source project, they own the copyright on their
> contributions, but unless they specify a different license that covers
> their contributions, it’s implied that they are granting use of their
> contributions under the same license as the project. In this way, the
> contributor can’t later demand to have their copyrighted code removed; it’s
> under the terms of the same license, which can't be revoked.
>
> Once a copyright statement is placed on a source file, there’s a bunch of
> legal reasons why you cannot change or remove that copyright statement. In
> general, you should keep all copyright statements added to a source file
> and never change one that already exists on a source file. Just look at the
> file header on any WebKit[^3] source file. WebKit even specifies that you
> add a copyright notice to each file where you make “significant”
> changes.[^4]
>
> With this in mind, it would be more proper to update the general copyright
> notice to something like this:
>
> Copyright (c) 2023 and later, The PHP Foundation and contributors. All
> rights reserved.
> Copyright (c) 1999-2023, The PHP Group and contributors. All rights
> reserved.
>
> Since no reassignment of copyright is taking place, we don’t run into any
> major legal issues, and this tells a true and accurate story. The PHP Group
> were stewards of the project until 2023, at which point the community
> recognized The PHP Foundation as the new stewards of the project, and
> through all of this time (since 1999), the various copyrights have been
> owned by their respective owners (i.e., contributors).
>
> If you look at the file headers on ICU source code, you can see that
> Unicode, Inc. made a similar change in 2016.[^5]
>
> All this said… I am in favor of making this change.
>
> I also have a lot more to say on this, as I’ve been considering opening up
> an RFC on just this topic. I had intended to reach out to Zend first
> (through Matthew Weier O’Phinney), but I haven’t done that yet, so this is
> the first time anyone from Zend has seen these ideas. My apologies. :-)
>
> The PHP source code is interesting in that it is covered by two licenses:
> the PHP License[^6] and the Zend Engine License.[^7] This is an historical
> artifact of the early days of PHP when it was conceivable that other
> companies might develop their own engines for PHP, but we know how this
> story ends: for all intents and purposes, the Zend Engine is PHP and PHP is
> the Zend Engine. Yes, I’m aware of alternatives (with very limited
> adoption), and no, they don’t matter for this discussion.
>
> Both the PHP License and Zend Engine License are based on the BSD 4-clause
> “Original” license,[^8] 

Re: [PHP-DEV] Re: [RFC][Discussion] Harmonise "untyped" and "typed" properties

[Nicolas Grekas]

Hi Rowan,

Le jeu. 23 nov. 2023 à 08:56, Rowan Tommins  a
écrit :

> On 23 November 2023 01:37:06 GMT, Claude Pache 
> wrote:
> >What you describe in the last sentence is what was initially designed and
> implemented by the RFC: https://wiki.php.net/rfc/typed_properties_v2
> (section Overloaded Properties).
> >
> >However, it was later changed to the current semantics (unset() needed in
> order to trigger __get()) in https://github.com/php/php-src/pull/4974
>
>
> Good find. So not only is it not specified this way in the RFC, it
> actually made it into a live release, then someone complained and we rushed
> out a more complicated version "to avoid WTF". That's really unfortunate.
>
> I'm not at all convinced by the argument in the linked bug report -
> whether you get an error or an unexpected call to __get, the solution is to
> assign a valid value to the property. And making the behaviour different
> after unset() just hides the user's problem, which is that they didn't
> expect to *ever* have a call to __get for that property.
>
> But I guess I'm 4 years too late to make that case
>

Sorry this comes as a surprise to you but you're rewriting history here.
The current behavior, the one that was fixed in that commit, matches how
PHP behaved before typed properties, so this commit brought consistency.

About the behavior, it's been in use for many years to build lazy proxies.
I know two major use cases that leverage this powerful capability: Doctrine
entities and Symfony lazy services. There are more as any code that
leverages ocramius/proxy-manager relies on this.

About the vocabulary, the source tells us that "uninitialized" properties
that are unset() become "undefined". I know that's not super accurate since
a typed property is always defined semantically, but that's nonetheless the
flag that is used in the source. Maybe this could help with the RFC.

Cheers,
Nicolas

[PHP-DEV] Re: [RFC] Pass Scope to Magic Accessors

[Nicolas Grekas]

Hi all,

Ilija and I would like to start a discussion about the following RFC:
> https://wiki.php.net/rfc/pass_scope_to_magic_accessors
>
> When using magic methods to access actual properties, respecting their
> declared visibility is often desired. Yet, accessing the calling scope to
> emulate the visibility restrictions is unreasonably difficult at the
> moment. This RFC proposes to pass the calling scope to magic accessors to
> make it trivial to get it.
>

Just a quick heads up on this thread to let you know that I'm withdrawing
the RFC.
I'm working with Arnaud Le Blanc on a native implementation of lazy objects
and I don't need fast access to the calling scope for any other use case.
If anyone has, feel free to follow up, but on my side I'm going to focus on
that new soon-to-be RFC for lazy objects. Stay tuned :)

Even if I'm withdrawing, I'm sending a Big Thank You to Ilija for writing
the prototype implementation.
Also thank you to everybody who took some time to review the proposal!

Cheers,
Nicolas

Re: [PHP-DEV] Re: [RFC][Discussion] Harmonise "untyped" and "typed" properties

[Nicolas Grekas]

Hi Rowan, Larry,

Thanks for the RFC.

I think there is an inaccuracy that needs to be fixed in the after-unset
state : as noted later in the RFC, magic accessors are called after an
unset($this->typedProps). This means the state cannot be described as
identical ("uninitialized') before and after unset() in the first table in
the RFC. Isn't there some vocabulary in the source that we can use  to
describe those states more accurately?

My initial gut reaction is that both ReflectionParameter and
> ReflectionProperty should treat "omitted" as "mixed", and just evaluate
> their type to "mixed".  It is in practice a distinction without meaning, or
> will be after this RFC.  That said, I also have an itch telling me that
> there are a few small-but-important edge cases where you would care about
> the difference between mixed and none in reflection, and that I'd be the
> one to run into them, but I cannot think of what they would be.
>


I think this needs to be clarified. I can't think of when this difference
could matter. I never wrote any code that could give any meaning to mixed
vs untyped properties, and many here know that I can write unusual PHP
code. I actually always wondered why we have this difference and I
therefore welcome the RFC.

Maybe this can be evaluated up to the point where we realize that the
change could go into 8.4?
I'd be happy to run a patched PHP on some codebases I maintain to see how
it goes.

Nicolas

Re: [PHP-DEV] [RFC] [Discussion] Final anonymous classes

[Nicolas Grekas]

Hi Daniil and thanks for the RFC.

I would like to submit an RFC and PR to add support for final anonymous
> classes, /or/ make all anonymous classes final by default, with or
> without the possibility to make them non-final.
>
> Here's the URL of the RFC:
> https://wiki.php.net/rfc/final_anonymous_classes
>
> I'd more more than glad to receive some feedback!
>

I've read the RFC proposing to make anonymous classes final by default, and
as defender of backward compatibility, I have concerns. The RFC cites
potential performance gains and a cleaner codebase as justifications.
However, the performance improvement seems limited to a narrow use case –
anonymous classes aren't that common in everyday PHP coding. Without
concrete data demonstrating significant performance benefits, this argument
doesn't seem strong enough to warrant a BC break.

Regarding the argument for cleaner code, while I acknowledge the appeal of
streamlined design, we must remember that breaking backward compatibility
isn't the right tool for correcting past decisions. PHP has a rich history
of handling such transitions gracefully, typically introducing changes with
a deprecation notice in a minor version before a major shift. This approach
has served our community well, maintaining stability and predictability.

Is this change truly worth such a departure from our norms? I believe not.
Making anonymous classes final by default goes against the principle of
least surprise, a core philosophy in PHP where regular classes aren't final
by default.

While I'm open to Proposal 1, which introduces final anonymous classes
without breaking BC, Proposals 2 and 3 are a different story. They seem to
suggest we can break backward compatibility on a whim, but our release
process (https://wiki.php.net/rfc/releaseprocess) clearly advises against
such moves in minor releases. Overriding this policy should require a more
substantial rationale.

As an example, in my work with Symfony, I've utilized class_alias to extend
anonymous classes within a test suite (
https://github.com/symfony/symfony/blob/c9e7809f045a1366afe2c2643bae15751ae7b500/src/Symfony/Component/VarExporter/Tests/LazyProxyTraitTest.php#L294).
While the RFC suggests alternative methods to achieve something similar,
the essence of preserving backward compatibility is about ensuring
stability and reliability without imposing the burden of code modification
on userland.

In summary, I advocate for the RFC to focus on the non-BC-breaking option.
Let's maintain our commitment to stability and gradual evolution in PHP.

Cheers,
Nicolas

Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

[Nicolas Grekas]

I just opened the vote for the "Increasing the default BCrypt cost" RFC.
> The RFC contains a two votes, one primary vote that requires a 2/3
> majority to pass and a secondary vote deciding on the new costs with a
> simple majority. Voting runs 2 weeks until 2023-10-05 17:45 UTC.
>
> Please find the following resources for your references:
>
> RFC Text: https://wiki.php.net/rfc/bcrypt_cost_2023
> Discussion Thread: https://externals.io/message/121004
> Feedback by a Hashcat team member on Fediverse:
> https://phpc.social/@tychotithonus@infosec.exchange/111025157601179075
>


Hi Tim,

For the record, I voted for 11 because I think it's nicer to end users (I
guess many don't know they could have a potential DoS vector via password
submissions), and also because it's going to be easy to raise again in
8.5/9.0.

I was wondering if you considered also raising the Argon2 default cost? Has
this been discussed?

Thanks for the RFC

Nicolas

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

[Nicolas Grekas]

> 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.
>

Even when catching the exception, there is no way to access the cloned
instance, since it's not referenced anywhere yet, isn't it? I thought we
could get it by calling $exception->getTrace(), but even this doesn't
contain the cloned object as far as I tested correctly.



> 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?
>

It's not only solving the problem of wasteful deep cloning, but also
solving the problem of validating while cloning (the one Alexandru
highlighted.)
So yes, I think it's worth it. Also because to me shipping a solution with
known shortcomings is likely going to turn into technical debt for the
community and even for internals in the future.

I saw your message about postponing this to 8.4. I think that's fair.
Complex topic :)

Nicolas

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

[Nicolas Grekas]

>
> 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.
>

Thanks Mate for moving all this forward.

I wish we could have voted to keep rand() and getrandmax() and just change
them to use the CSPRNG, like we do for  array_rand() & co. but I guess this
should have been proposed earlier. Might be worth a small follow up RFC to
not disrupt the current one.

Cheers,
Nicolas

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

[Nicolas Grekas]

> Then, among both options, we need to select the one with the best future
> > proofness, and that's definitely the OOP one to me, because it comes with
> > more guarantees (type checks).
>
>
> So are you saying we should deprecate the function-based version
> completely, and not provide any new names at all? I think I would prefer
> that to the confusing set of aliases the current RFC text proposes.
>

I was not but I would be fine with this solution also :)


> I think I would support any of these options:
>
> 1) Do nothing. I don't see the current situation as being that big a
> problem.
>

The problem is the overloaded signature, which Mate explained in the RFC.


> 2) Deprecate the function-based signature, provide no alternative but to
> switch to the interface-based one. Potentially non-trivial upgrade for
> those affected, but leaves us with a single clear function.
>

Would work for me! The OOP way can be implemented since more than 10 years.


> 3) Create new names for both signatures, treating neither of them as
> more "default" than the other, so that users have a clear choice between
> the two. Deprecate the existing function completely.
>

Deprecating in the same version that provides the alternative would create
a lot of friction (even if in this case, it's possible to polyfill, which
makes this a bit less rough). So to me, this path requires providing an
alias without deprecating the OOP version first, and later on deprecating
the current name. THIS would create confusion to me, because ppl would have
two names to choose from, and many won't take the time to figure out the
difference.

Mate, WDYT of 2)?

Nicolas

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

[Nicolas Grekas]

Le lun. 19 juin 2023 à 22:33, Rowan Tommins  a
écrit :

> On 19/06/2023 21:17, Nicolas Grekas wrote:
> > I think we must account for a bit of history/legacy on the topic.
> > I think session_set_save_handler(SessionHandlerInterface) is the best
> BC/FC
> > path we can provide.
>
>
> Can you elaborate? The SessionHandlerInterface is the *newer* of the two
> current signatures, so what does making it the preferred signature (with
> users of the other having to change their code) have to do with
> "history/legacy"?


Sure : SessionHandlerInterface is around 5.4. It's been there since long
enough to not care about this aspect when considering both signatures. In
my experience, supporting 3 major versions is way enough: previous major,
current major and next major. Which means 7+8+9 in this case. It's enough
because an app that runs on < 5.4 doesn't need to prepare to move to 9. It
first has many more steps to do.

Then, among both options, we need to select the one with the best future
proofness, and that's definitely the OOP one to me, because it comes with
more guarantees (type checks).

By doing so, we allow apps that are still on 7 but are also actively
planning to upgrade to 8+ to make the future-proof choice of choosing
SessionHandlerInterface.

Nicolas

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

[Nicolas Grekas]

Le mer. 14 juin 2023 à 23:51, Máté Kocsis  a écrit :

> >
> > 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.
>


I think we must account for a bit of history/legacy on the topic.
I think session_set_save_handler(SessionHandlerInterface) is the best BC/FC
path we can provide.
And I don't think we should alias this function to a new name because that
would require users to make a choice, and will lead to confusion, because
in this case, the choice is purely cosmetic, but people will still have to
get it.

Nicolas

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

[Nicolas Grekas]

> > > I'm going to nitpick on the newly suggested names and argument order
> for
> > > the
> > > DatePeriod factory methods — althoughI do agree that they need to get
> > > created:
> > >
> > > createFromInterval(DateTimeInterface $start, DateInterval $interval,
> > > DateTimeInterface $end, int $options = 0)
> > > → createWithRange(DateTimeInterface $begin, DateTimeInterface $end,
> > > DateTimeInterface $int, int $options = 0)
> > >
> > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval,
> > > int $recurrences, int $options = 0)
> > > → createWithRecurrences(DateInterval $begin, int $recurrences,
> > > DateInterval $interval, int $options = 0)
> > >
> > > We also should fix the argument names. Either $start/$finish, or
> > > $begin/$end. I
> > > prefer the latter.
> > >
> > > createFromIso8601(string $specification, int $options = 0)
> > > -> createFromISO8601String
> > >
> > > I am open to bike shedding about this :-)
> > >
> >
> > 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 still has an overloaded third argument — DateTimeInterface|int.
>
Where you trying to suggest to change the current existing __construct()
> to:
>
> public function __construct(DateTimeInterface $start, DateInterval
> $interval, DateTimeInterface $end, int $options = 0)
>
> and then add these two new factory methods?
>
> createFromRecurrences(DateTimeInterface $start, DateInterval $interval,
> int $recurrences, int $options = 0)
>
> createFromISO8601String(string $specification, int $options = 0)
>

I really meant DateTimeInterface|int, not an overloaded signature but a
union type.

My concern is about providing the best path forward.
Both deprecating and providing the alternative in the same version creates
a lot of friction for end users.
After a quick chat with Mate, I think we should do this union type *in
addition* to adding the new static constructors.
Then, later on, if future us think it's worth it, we might want to consider
deprecating passing an integer.
This would provide a path for BC/FC I would fully support.

Nicolas

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

[Nicolas Grekas]

>
> > 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 going to nitpick on the newly suggested names and argument order for
> the
> DatePeriod factory methods — althoughI do agree that they need to get
> created:
>
> createFromInterval(DateTimeInterface $start, DateInterval $interval,
> DateTimeInterface $end, int $options = 0)
> → createWithRange(DateTimeInterface $begin, DateTimeInterface $end,
> DateTimeInterface $int, int $options = 0)
>
> createFromRecurrences(DateTimeInterface $start, DateInterval $interval,
> int $recurrences, int $options = 0)
> → createWithRecurrences(DateInterval $begin, int $recurrences,
> DateInterval $interval, int $options = 0)
>
> We also should fix the argument names. Either $start/$finish, or
> $begin/$end. I
> prefer the latter.
>
> createFromIso8601(string $specification, int $options = 0)
> -> createFromISO8601String
>
> I am open to bike shedding about this :-)
>

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.

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 like everything else in the RFC. I'm going to go with the "short path" of
deprecating things because the migration paths look smooth enough, thanks
for taking the time to explain them all (I just have a concern with the
DatePeriod migration path, which would be fixed with my suggestion bove.)

Thanks!
Nicolas

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

[Nicolas Grekas]

Le ven. 9 juin 2023 à 02:11, Larry Garfield  a
écrit :

> On Thu, Jun 8, 2023, at 5:39 PM, Stephen Reay wrote:
>
> > Is there a specific reason `clone with (foo: $bar);` can’t simply pass
> > the arguments given to with(), to the __clone() magic method?
> >
> > It leaves the developer free to use the passed argument(s) or deep
> > clone existing properties or a mix depending on what’s passed, and
> > seems like it has the least “magic” or unknown behaviour in terms of
> > when things happen.
> >
> > Just a thought.
>
> I experimented with that a few years back.  The results showed it is
> actually pretty awful in practice.
>
> https://peakd.com/hive-168588/@crell/object-properties-part-2-examples
>


Thanks for the link.

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)

Rewriting the example in your blog post Larry, the Request class would then
look like below.
And I think I like it.

WDYT? Mate?

class Request implements RequestInterface
{
public readonly array $headers = [];
public readonly UriInterface $uri;
public readonly string $version = '1.1';
public readonly string $method = 'GET';

public function __clone()
{
$this->uri = clone $this->uri;
}

public function __clone_with(array $properties)
{
if (isset($properties['version']) && !in_array($this->version, ['1.1', '1.0',
'2.0'])) {
throw new InvalidArgumentException($k);
}

$headers = $this->headers;

if (isset($properties['uri'])) {
$headers['host'] = $this->uri->host;
} else {
$this->uri = clone $this->uri;
}

$this->headers = $headers;
}

public function getHeader($name): string
{
return $this->headers[strtolower($name)] ?? '';
}

// Still needed because of the $preserveHost = true option.
public function withUri(UriInterface $uri, bool $preserveHost = false):
static
{
$headers = $this->headers;

// If headers were itself a pseudo-immutable object, this would be even
uglier.
if ($preserveHost && isset($headers['host'])) {
$headers['host'] = $uri->host;
}

return clone $this with (uri: $uri, headers: $headers);
}

public function withHeader(string $name, string $value): static
{
$headers = $this->headers;
$headers[strtolower($name)] = $value;

return clone $this with (headers: $headers);
}
}

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

[Nicolas Grekas]

> On Tue, May 30, 2023, at 10:04 PM, Alexandru Pătrănescu wrote:
> > > On Tue, May 30, 2023, 19:39 Larry Garfield 
> > wrote:
> > >
> > >> On Mon, May 29, 2023, at 11:22 PM, Máté Kocsis wrote:
> > >> > 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()?
> > >>
> > >> Not a specific one off hand.  It's more a conceptual question.  `with`
> > has
> > >> more contextual awareness than __clone(), so it should have "first
> > crack"
> > >> at the operation, so that if necessary it can make changes that
> > __clone()
> > >> can then respond to.  The inverse doesn't make sense.
> > >>
> > >> The only reason for `with` to come after would be to allow `with` to
> > >> "override" or "undo" something that __clone() did.  Generally
> speaking,
> > if
> > >> you have to undo something you just did, you shouldn't have done it in
> > the
> > >> first place, so that's a less compelling combination.
> > >>
> > >> This one isn't a deal breaker, but we should be sure to think it
> through
> > >> as it's kinda hard to reverse later.
> > >>
> > >
> > > To me so far also it was natural to assume that __clone is first and
> only
> > > after that the rest of the operations.
> > > But `with` operations, be it properties assignment or even a closure,
> > would
> > > run in the context of the caller of clone and sometimes this might be
> run
> > > not from a method of the cloned class.
> > >
> > > An example:
> > > There is a class that represents persons of a fictive country/planet.
> > > Each person has many properties but has also a first name and a last
> name
> > > and there is a rule: the two names must not start with the same letter.
> > > Both names cannot be changed as they are defined readonly.
> > > Creation of new persons can be done using new for new random properties
> > or
> > > using clone to preserve existing properties. But in both cases the
> first
> > > name and last name are randomly chosen.
> > > If we want to control the last name value during clone that would be
> > > possible using the `with` operation but the logic to allocate a first
> > name
> > > will only happen in `__clone()`method.
> > >
> > > To be able to achieve this we must have __clone last, as there we have
> > the
> > > internal validations, operations and also access to private/protected
> > > members that are not accesible from where clone is being called.
> > >
> > > Regards,
> > > Alex
> >
> > I... could not understand that in the slightest.  Can you show it in
> code?
> >
> >
>
> Sorry for that. Here you go: https://3v4l.org/JIBoI/rfc#vgit.master
> If __clone would be first, there is no way to enforce the rule that a
> person cannot have their first name starting with the same letter as last
> name.
>

I'm not sure that's what __clone should be used for.
This looks like a perfect use case for property hooks, isn't it?

On my side, I would find it unexpected that __clone is called after because
that would break cloning expectations:
Imagine you have a __clone that does some deep cloning (a much more typical
scenario for this construct),
Let's say __clone() does $this->foo = clone $this->foo;

Imagine now that you do: clone $obj with (foo: $bar)
I'd expect $obj->foo === $bar after this. But if __clone is called after,
that won't be true, and I don't see how that could be "fixed" if we swap
the order. Would you?

Nicolas

Re: [PHP-DEV] [RFC] Property hooks, nee accessors

[Nicolas Grekas]

Ilija Tovilo and I would like to offer another RFC for your consideration.
> It's been a while in coming, and we've evolved the design quite a bit just
> in the last week so if you saw an earlier draft of it in the past few
> months, I would encourage you to read it over again to make sure we're all
> on the same page.  I'm actually pretty happy with where it ended up, even
> if it's not the original design.  This approach eliminates several
> hard-to-implement edge cases while still providing a lot of functionality
> in one package.
>
> https://wiki.php.net/rfc/property-hooks
>

Congrats for the RFC for property hooks, I like it all, including
abbreviated forms, $value as default name, etc.

Some notes:

   - does set($value) { mean set(mixed $value) { or set(TypeFromProp
   $value) { ? I understand it would be the latter but it might be nice to
   clarify (unless I missed it)
   - function setFullName => ucfirst is not found in the "set" version
   - could parent::$x::set($x) be replaced by parent::$x = $x ? Same for
   the get variant? Why not if not?
   - readonly: could this be supported by materializing them, using the
   storage for memoization after first access? that'd be quite powerful
   - I'm not sure we need ReflectionProperty::getHook('set')  when we can
   do getHooks()['set'] ?? null
   - What does ReflectionMethod::getName return on a hook? And getClosure?
   - Should we add ReflectionProperty::IS_VIRTUAL ?
   - I'm a bit surprised by the possibility of adding attributes to hooks.
   I'm not sure I see the use case and I think this might be confusing in
   addition to attributes on the property itself.
   - For serialization: exclude them from var_export() and serialize(),
   which don't need virtual props to rebuild the state (you mean
   json_encode(), not JsonSerializable on that list), call get for
   var_dump/print_r when it's defined (but ignore exceptions). But (array) /
   get_mangled_object_vars() / get_object_vars() shouldn't call any "get".

I have a bigger concern: the take on references contradicts with the intro
about BC breaks: turning a materialized property into virtual one would
break BC as far as refs are concerned. One idea to fix that: add a ref
hook, that must return a reference. If not implemented => Error when trying
to get-by-ref. This could also help solve the $foo->array[] case (including
the asym-visiblity issue).

Nicolas

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

[Nicolas Grekas]

> > > What about using a real closure to define the scope we need for
> cloning?
> > > That closure would take the cloned instance as argument to allow
> > > manipulating it at will.
> >
> > I believe someone mentioned that one previously in the thread.
>
>
> Yes, Nicolas mentioned me.
> I wanted to get back myself to discussing this topic more as well and find
> a better solution but didn't get yet time for it.
>
>
> >   The problem is that the closure would run in the scope of the object,
> > not the scope of the caller.  That means if called outside the object, it
> > would allow modifying private or protected properties.  The itemized list
> > of values (whether an array or named-args style) would allow the engine
> to
> > enforce access restrictions, which is a desireable feature.
> >
>
> As far as I can see, Nicolas was able to find a solution to this problem
> and so far I like it:
> The closure is running in the current scope where it is defined and not in
> the scope of the cloned object. The cloned object is passed as the single
> parameter to the closure.
>

Absolutely. This would be a plain boring closure with all its current
visibility semantics.
Using a closure to run some code nested in a transaction is already quite a
common practice.
E.g.this is a common way to define the computation logic for a cache
storage:
$cache->get($cacheKey, $callback)

Or for a database transaction:
$db->transaction(function() { ... });

The cloning logic we want to run fits this style, so this is quite natural
once we realize that:
$clone = clone($this, $callback);

The only thing we would need to settle on is the interface of the $callback.

The suggested clone signature for a class T would be:
> - clone(T $object, callable(T $clone)): T; // calling clone as a function
> - clone T $object with callable(T $clone): T; // calling clone as a
> language construct


100% this. My preference goes for #1, to keep things as boring as possible.
Just to make it clear, I would document the closure with the void return
type:
clone(T $object, callable(T $clone)*:void*): T; // calling clone as a
function


> Alternatively, we can have also:
> - clone T, callable(T $clone); // without "with" keyword
>

This would be ambiguous, eg foo(clone T, callable(T $clone)) is that a
function call with two arguments, or?


> And improve it to allow even multiple closures to be executed:
> - clone(T $object, ...callable(T $clone)): T;
> - clone T $object, ...callable(T $clone): T;
>

I wouldn't allow this. Calling many closures is what the main closure can
do in its body, no need for more fancy things.

Nicolas

Re: [PHP-DEV] [Discussion] Callable types via Interfaces

[Nicolas Grekas]

Hi all,

https://wiki.php.net/rfc/allow_casting_closures_into_single-method_interface_implementations
>
> https://wiki.php.net/rfc/allow-closures-to-declare-interfaces-they-implement
> https://wiki.php.net/rfc/structural-typing-for-closures


Thanks Larry for the nice introduction to those ideas.

Personally, I feel like going with adding Closure::castTo() might provide
the most immediate benefit. I expanded the rationale on the corresponding
RFC and added more examples. I'd appreciate it if all of you reading could
have another look to see if that helps to better understand the proposal.

strtr(...)->castTo(TranslatableInterface::class) is one example of RFC #1
function ($message, $parameters) implements TranslatableInterface is RFC #2

Both RFCs nicely combine together to cover many cases of typed callabled.

Then RFC#3 is a bit more adventurous (according to our understanding) but
still desirable as it's essentially about allowing the engine to
tentatively call castTo() from RFC#1 when a closure is passed as argument
while an interface is expected.

We're now wondering if we should start spending time on prototype
implementations for #1 and/or #2, and #3 in this order. Should we consider
a preliminary vote on the topic?

Nicolas

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

[Nicolas Grekas]

Hi again,

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
>

As mentioned in another thread, I'd like to make an alternative proposal
for the syntax.
Alex talked about it already but I think it deserves more attention.

What about using a real closure to define the scope we need for cloning?
That closure would take the cloned instance as argument to allow
manipulating it at will.

Internally, the engine would "just" call that closure just after calling
__clone() if it's defined.

This could look like this:
$c = clone $a with $closure;

or maybe we could skip introducing a new keyword and go for something that
looks like a function call:
$c = clone($a, $closure);

I've adapted your examples from the RFC using the latter and here is how
they could look like:

The “wither” method copy-pasted from Diactoros:
class Response implements ResponseInterface {
public readonly int $statusCode;
public readonly string $reasonPhrase;
// ...
public function withStatus($code, $reasonPhrase = ''): Response
{
return clone($this, fn ($clone) => [
$clone->statusCode = $code,
$clone->reasonPhrase = $reasonPhrase,
]);
}
// ...
}

The property name expressions:
class Foo {
private $a;
private $b;
private $c;
/**
* @param array $properties
*/
public function withProperties(array $properties) {
return clone($this, function ($clone) use ($properties) {
foreach ($properties as $name => $value) {
$clone->$name = $value;
}
});
}
}

Linking the cloned instance to the original one:
class LinkedObject {
public function __construct(
private readonly LinkedObject $next,
private readonly int $number
) {
$this->number = 1;
}
public function next(): LinkedObject {
return clone($this, fn ($clone) => [
$clone->number = $clone->number + 1,
$this->next = $clone,
]);
}
}

All these look pretty neat to me, and they come with no new syntax but a
simple call-like clone() statement.

Scope semantics remain the same as usual, so we already know how to
interpret that aspect.

Does that make sense to you?

Nicolas

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

[Nicolas Grekas]

Hi Mate,

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
>

Thanks for working on this, we definitely need improvements on the topic.
Thanks also for mentioning my proposal and for the comparison analysis,
that's really helpful.

Quoting from the RFC:
> One cannot control whether $this should really be cloned: e.g. if a
property should only be modified based on certain conditions (e.g.
validation), the object would potentially be cloned in vain, resulting in a
performance loss.

Returning a clone or the same instance depending e.g. on some validation is
usually an abstraction leak. It's a leak because it allows knowing internal
implementation details by comparing the identity of the resulting objects.
Since the PHP community is leaning towards more strictness and better
abstractions, I think this point is actually a win for my proposal: it'd
naturally close the loophole in wither-based abstractions. See
https://3v4l.org/02IVc#v8.2.5 if what I mean is unclear.

> Sometimes one also needs access to the original instance, but doing so
would only be possible via workarounds (e.g. by introducing $that or a
similar construct).

Here is how we would achieve this under my proposal (adapting from your
example):
class LinkedObject {
/*...*/

public function next(): static {
$clone = $this->prepareNext();
$clone->next = $this;

return $clone;
}

private clone function prepareNext(): static {
$this->number++;
unset($this->next);

return $this;
}
}

As you can see, this works by *not* using the clone keyword on the public
method. And this makes me realize that what you see as an advantage ("it
could be made part of the interface contract") might actually be a
drawback, by forcing a coupling between a contract and an implementation.
That's what you mean also with your last sentence on your analyses.

And that kills my proposal, RIP :)
But I have another proposal I'm sending separately.

Nicolas

Re: [PHP-DEV] Final anonymous classes

[Nicolas Grekas]

Hi all,

> > I've submitted https://github.com/php/php-src/pull/11126 to add support
> for final anonymous classes, though as noted by iluuu1994, it would
> probably make more sense to just make all anonymous classes final by
> default, what do you think?
> >
> > Extending an anonymous class is indeed possible (https://3v4l.org/pDFTL),
> but it is a hack as best. If someone wants a non-final class, could they
> not write a non-anonymous one? As a bonus, they wouldn’t need to
> instantiate the class before referencing it.
>
> Indeed. The argument was that, if you need to give the anonymous class
> a dedicated name through an alias to extend it, you might as well
> declare a named class in the first place.
>
> In case somebody finds benefit in making anonymous classes open, it
> seems more sensible to make them opt into openness, rather than
> applying this behavior to all anonymous classes that are used as final
> 99.9% of the time. Although I really don't think that is necessary.
>

Fun fact: I wrote test code that extends anonymous classes recently.
The typical test case is like this:

$obj = new class() {
public array $foo;
};
$proxy = $this->createLazyGhost($obj::class, fn () => null);


And createLazyGhost() generates a proxy class and returns an instance of
it, see
https://github.com/symfony/symfony/blob/1f8c5928a1445378eacbaf5b7e1636fdfa8610ed/src/Symfony/Component/VarExporter/Tests/LazyGhostTraitTest.php#L415-L438

Admittedly this is for convenience in writing test cases, but convenience
is what many RFCs are about :)

But anyway, there is a related idea I've had in my mind about anonymous
classes that I'd like to throw in that conversation:

Because they conceptually don't create a new type, I wonder if (final)
anonymous classes could be allowed to extend final classes? In order to not
allow hacking around and still create a type with class_alias(), we should
forbid aliasing final anonymous classes IMHO. Then we could have this
discussion about allowing finally anonymous classes to extend final
classes. That'd be really useful in many situations where "final" is
preventing end users from achieving what they want.

Nicolas

Re: [PHP-DEV] Brainstorming idea: inline syntax for lexical (captured) variables

[Nicolas Grekas]

Hi Rowan, hi all!

Le ven. 17 mars 2023 à 15:51, Larry Garfield  a
écrit :

> On Thu, Mar 16, 2023, at 6:05 PM, Rowan Tommins wrote:
> > On 16/03/2023 22:14, Larry Garfield wrote:
> >> Wouldn't the functionality described boil down to essentially just
> materializing into a few extra lines in the constructor?  At least to my
> ignorant non-engine brain it seems straightforward to have this:
> >>
> >> $a = 1;
> >> $b = 2;
> >> $c = 3;
> >>
> >> $o = new class ($a, $b) use ($c) {
> >>public function __construct(private int $a, private int $b) {}
> >>public function something() {}
> >> }
> >>
> >> Desugar to this:
> >>
> >> $c = class ($a, $b) use ($c) {
> >>private $c;
> >>public function __construct(private int $a, private int $b) {
> >>  $this->c = 3;  // By value only, so this should be fine?
> >>}
> >>public function something() {}
> >> }
> >
> >
> > Not quite - as Ilija pointed out, the class definition gets compiled
> > once, but the capture needs to happen for every instance, with
> > (potentially) different values of $c. In other words, $c needs to be
> > injected as a constructor argument, not a constant in the class
> definition.
> >
> > That's still fine, in principle - you can compile to this:
> >
> > $o = class ($a, $b, $c) {
> >public function __construct(private int $a, private int $b, private
> $c) {
> >}
> >public function something() {}
> > }
> >
> > Or once constructor promotion is de-sugared as well, this:
> >
> > $o = class ($a, $b, $c) {
> >private int $a;
> >private int $b;
> >private $c;
> >
> >public function __construct($a, $b, $c) {
> >   $this->a = $a; // from constructor promotion
> >   $this->b = $b; // from constructor promotion
> >   $this->c = $c; // from use() statement
> >   // other lines from body of constructor go here
> >}
> >public function something() {}
> > }
> >
> >
> > It just introduces a lot of extra cases to handle:
> >
> > * If there's no constructor, create one
> > * If there is a constructor with other arguments, merge the argument
> > lists; since there will then be an explicit argument list to "new
> > class()", merge those lists as well
> > * Maybe different handling if those other arguments are already using
> > constructor promotion, as in this example
> > * If there are existing lines in the constructor body, combine those
> > with the auto-generated assignments
> >
> >
> > Which is why I'm thinking a first implementation would be reasonable
> > which just took this approach:
> >
> > * "new class" can either have an argument list or a use() statement, not
> > both
> > * the use() statement generates a constructor at the top of the class
> > * if the class body already contains a constructor, the compiler will
> > complain that you have two methods named "__construct"
>
> Ah, fair enough.  I'd want to see a better error message than that (which
> would be confusing for people who don't know what they're looking for), but
> otherwise that's a reasonable first iteration.  Especially as I can't
> recall when I last had an anon class constructor that was doing anything
> other than manual closures. :-)
>


I created this draft RFC to help move things forward:
https://wiki.php.net/rfc/syntax-to-capture-variables-when-declaring-anonymous-classes

Please let me know your early thoughts and I'd be happy to move it to
"under discussion".
I'd also need someone for the implementation as I doubt I'll be able to
write it myself in a reasonable time!

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] New core autoloading mechanism with support for function autoloading

[Nicolas Grekas]

Hello George,

Dan and I would like to propose a new core autoloading mechanism that fixes
> some minor design issues with the current class autoloading mechanism and
> introduce a brand-new function autoloading mechanism:
> https://wiki.php.net/rfc/core-autoloading
>
> The existing SPL autoloading functions would become aliases to the new Core
> ones and will continue to work without any migrations needing to be
> performed.
>

Thanks to you and Dan for the RFC.

I have two main questions about this:
1. What's the performance hit?
2. If the perf hit is measurable, is function autoloading worth it? aka
what problem does it address?

About the perf hit, I see that you put a lot of thought into minimizing it
and that's sound. The way I see this feature be spread to userland is via
composer of course. I foresee that composer will add a way for packages to
declare function autoloading. This means that every application that uses
composer will be impacted by the perf overhead eventually (aka the case of
"programs that do not have a function autoloader registered" mentioned in
the RFC won't happen in practice.)

In e.g. https://github.com/php/php-src/pull/6627#issuecomment-773922448,
Dmitry explains that adding inheritance cache to PHP provided a 8% perf
benefit when running a demo app. I would like to see a similar benchmark
with function autoloading enabled. https://github.com/phpbenchmarks/
provides several demo apps so it could be a nice playground for running
this.

Then, depending on the resulting numbers, I anticipate several possible
outcomes:
- if the impact is really low, then all is fine and we can keep things as
is;
- otherwise, I'm also interested in this idea to scope function autoloading
per namespace, so that the engine can filter ahead of calling any userland
autoloaders;
- if the overhead is still significant, could another idea be to load
functions per namespace, where the engine would ensure to call function
autoloaders only once per namespace? Even with the current proposal, I can
easily anticipate that a common practice for package authors could be to
put all functions in one file, and register a function autoloader that
would require that file for any of the functions in it. Loading per
namespace would fit this loading strategy. But before exploring this idea,
let's get some numbers about the performance of the previous approaches.

Then comes my second main question: what does this solve?

To me, class autoloading is a performance feature. If we didn't care about
performance, we would be fine with eagerly loading a bunch of classes as
e.g. Java does. But because we run PHP mainly in short-lived
request/response loops, loading just the few classes we need to serve a
specific request provides the best performance. In recent versions of PHP,
we may question this practice, since we now have a crazy good opcache. Of
course, I'm not suggesting removing autoloading for classes. But if we do
have that crazy good opcache, what do we need function autoloading for? The
current way for package authors to declare classes is via composer "files
" entry. One could argue in
the past that this adds needless "require" on the hotpath. But if they cost
nothing thanks to recent opcache improvements, we don't need to care, do
we? At least, we need to compare both approaches, because if the main
argument for adding function autoloading is lazy-loading of function
implementations, then it must be faster than eager-loading. This circles
back to my previous question, so we'd really need a thorough perf analysis
IMHO.

Nicolas

Re: [PHP-DEV] Brainstorming idea: inline syntax for lexical (captured) variables

[Nicolas Grekas]

> > To overcome the issues spotted in the thread, what about doing some sort
> > of CPP instead of autocapture?
> >
> > new class (...$arguments) use ($outer) extends Foo {
> > public function getIt() {
> > return $this->outer;
> > }
> > }
> >
> > This would be the equivalent of this:
> >
> > new class ($outer, ...$arguments) extends Foo {
> > public function __construct(public mixed $outer, ...$arguments) {
> > parent::__construct(...$arguments);
> > }
> >  public function getIt() {
> >  return $this->outer;
> > }
> > }
> >
>
>
> I was actually just thinking about exactly that approach, and wondering it
> would be possible to do it entirely as an AST rewrite.
>
> My only uncertainty so far is what to do with an actual constructor in the
> class, like this:
>
> new class($custom) use ($captured) {
> public function __construct($custom) {
>  // Duplicate definition error?
>  // Silently renamed and called from the generated constructor?
>  // Merged into the body after the generated lines?
> }
> }
>
> Forbidding it wouldn't be the worst restriction, but if there was some
> per-instance setup logic needed, not being able to write a constructor body
> might be a pain.
>

We could define the "use" as declaring + setting the properties before the
constructor is called, if any.
But I'm also fine making both constructs conflict: when there is a
constructor, the boilerplate saved by the "use" becomes really low.
No strong opinion either. There could be other factors to consider.



> > And we could also allow this for better type expressivity:
> > new class (...$arguments) use (private int $outer) extends Foo {
> > // ...
> > }
> >
>
>
> I was going to suggest an "as" clause, similar to traits, which would also
> allow naming the property differently from the source variable:
>
> $foo = 42;
> $name = 'Bob';
> $class = new class use ($foo as private int $counter, $name as readonly
> string) {}
>

I like that!

Re: [PHP-DEV] Brainstorming idea: inline syntax for lexical (captured) variables

[Nicolas Grekas]

Hi Rowan,

I have been pondering for a while how to improve the anonymous class
> syntax to allow "capturing" of values from the outer scope, and came up
> with the idea of a special variable marker for "lexically captured
> variable" - instead of $foo, you would write $!foo or $^foo (I quite
> like the "upwardness" of $^).
>

To overcome the issues spotted in the thread, what about doing some sort of
CPP instead of autocapture?

new class (...$arguments) use ($outer) extends Foo {
public function getIt() {
return $this->outer;
}
}

This would be the equivalent of this:

new class ($outer, ...$arguments) extends Foo {
public function __construct(public mixed $outer, ...$arguments) {
parent::__construct(...$arguments);
}
 public function getIt() {
 return $this->outer;
}
}

And we could also allow this for better type expressivity:
new class (...$arguments) use (private int $outer) extends Foo {
// ...
}

Nicolas

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

[Nicolas Grekas]

Hi Matthew,


> 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
> >
>
> I missed something when reviewing previously.
>
> Under the Proposal 1 section is the following verbiage:
>
> > readonly classes can declare neither static, nor untyped properties, no
> matter if the declaration is done directly in the class or indirectly via a
> trait (https://github.com/php/php-src/issues/9285). Under this RFC, their
> non-readonly child classes would support them as any other child class
> does.
>
> However, the example demonstrates neither static nor untyped properties. As
> such, it's hard to understand what pattern you are trying to enable here.
> Could you provide an example of a child class that uses static and/or
> untyped properties, please? Basically trying to understand what this would
> enable, and why.
>

We've added a few examples on the RFC that should help clarify.
Here is the link to the diff:

https://wiki.php.net/rfc/readonly_amendments?do=diff%5B0%5D=1674550446%5B1%5D=1674635113=sidebyside

And the link to the updated RFC of course:
https://wiki.php.net/rfc/readonly_amendments

Nicolas

Re: [PHP-DEV] [RFC] Pass Scope to Magic Accessors

[Nicolas Grekas]

> On 1/19/23 18:43, Ilija Tovilo wrote:
> > You're right, we'll try to improve the wording and provide a handful of
> > examples. Essentially, $callingScope should contain the class name of
> > the calling scope, so the place where the magic method was called. When
> > the calling scope is not a class (global scope, function, static
> > closure, etc.) null is passed instead.
>
> Thank you, this is more clear now.
>
> > given property. This is not a problem when the property is supposed to
> > be public but becomes difficult when trying to restrict access to it. It
>
> If I understand the proposal correctly, then any access restriction
> would effectively be a Gentleman's agreement only, because nothing is
> preventing me from calling '$someObj->__get('someProp',
> $someObj::class);', no?
>

Yep. That's already the case: using a rebound close, we can fake any scope
already so this is not new capability.



> Honestly so far I fail to see the cost/benefit ratio to be acceptable
> for the added complexity here. Not just the implementation complexity
> within the language itself, but also because this will need to be
> documented within the PHP manual and integrated within tooling, such as
> static analyzers.
>
> Is there some real-world example where I would employ this type of
> access control within the magic methods and where "not documenting the
> property to not expose it to the IDE autocompletion" would not be
> sufficient, but this easily circumventable check would be?
>
> My only use case of __get() so far was making properties visible to the
> public, while restricting write access, so this is not a problem I
> needed to solve before.
>

Legit concerns. I'm going to prepare a more extensive use case so the
motivations of RFC become more obvious.

I'll get back on this thread when ready. Stay tuned :)

Nicolas

[PHP-DEV] [RFC] Pass Scope to Magic Accessors

[Nicolas Grekas]

Hi internals,

Ilija and I would like to start a discussion about the following RFC:
https://wiki.php.net/rfc/pass_scope_to_magic_accessors

When using magic methods to access actual properties, respecting their
declared visibility is often desired. Yet, accessing the calling scope to
emulate the visibility restrictions is unreasonably difficult at the
moment. This RFC proposes to pass the calling scope to magic accessors to
make it trivial to get it.

Kind regards,
Nicolas

Re: [PHP-DEV] [RFC] Add SameSite cookie attribute parameter

[Nicolas Grekas]

Hi George,

Hello Internals,
>
> I would like to start the discussion about the Add SameSite cookie
> attribute parameter RFC:
> https://wiki.php.net/rfc/same-site-parameter
>
> This proposes to add an optional same site parameter to the setrawcooki(),
> setcookie() and session_set_cookie_params() that takes a a value a new
> SameSite enum:
>
> enum SameSite {
> case None;
> case Lax;
> case Strict;}
>

There's quite some activity on the HTTP cookies side.
I read about SameParty and Partitioned attributes recently, see:
- https://developer.chrome.com/docs/privacy-sandbox/chips/
- https://github.com/cfredric/sameparty

Maybe we should have a plan that works for these too?

Nicolas

Re: [PHP-DEV] [RFC][Vote] Asymmetric Visibility

[Nicolas Grekas]

Le mar. 10 janv. 2023 à 00:00, Larry Garfield  a
écrit :

> On Mon, Jan 9, 2023, at 10:00 AM, Nicolas Grekas wrote:
> >> I am hereby opening the vote on the Asymmetric Visibility RFC:
> >>
> >> https://wiki.php.net/rfc/asymmetric-visibility
> >>
> >> Voting will be open for 2 weeks.
> >>
> >> While we would certainly prefer if everyone voted in favor, for those
> who
> >> vote against the authors kindly request that you indicate why, using the
> >> second poll question or comment section afterward.  Thank you.
> >>
> >
> > Hi Larry, Ilija,
> >
> > Thanks for opening the vote. I played a bit with the code and I didn't
> spot
> > any unexpected behavior, nice work.
> >
> > I'm undecided for now, on two points:
> > 1. the syntax: looking at https://wiki.php.net/rfc/property-hooks, I
> wonder
> > if using public string $foo {private set;} wouldn't be more appropriate.
> > Otherwise, it might be strange to have public private(set) string $foo
> {set
> > ($value) { /* the setter */};} if hooks are the way forward. Sure we
> could
> > compact that but that would introduce two very different syntax whether a
> > setter is defined or not. WDYT?
>
> As discussed, putting the visibility on the right has a number of problems.
>
> 1. If we assume that hooks in something similar to the current unreleased
> RFC happen (which I hope they do), then putting the visibility in the hook
> area creates additional problems.
>
> For one, hooks will require disabling getting a reference to a property.
> That is not negotiable, as getting a reference to a property would let you
> bypass the hooks.  Aviz doesn't require that, though.  If the only thing
> you're doing is private-set, then using the hooks syntax for aviz means
> either introducing yet another access marker, like "private raw" to
> indicate that you don't want to disable references (I don't want to have to
> explain that to people), or just sucking it up and saying that aviz
> disables references.
>
> But disabling references also makes `$obj->arrayProp[] = 4` break
> entirely.  So that would make arrays completely incompatible with
> asymmetric visibility, when there's no actual reason that has to be the
> case.
>
> So in short, we end up with three options:
>
> A) The current proposed syntax.
> B) Having both "{ private set }" and "{private raw}", and having to
> explain WTH that even means.
> C) Arrays are incompatible with asymmetric visibility entirely, period.
>
> Of those, A is, I would argue, the clearly better option.
>
> 2. Some nerdy engine details, courtesy of Ilija.  Basically, given the way
> we're looking at doing hooks, mixing the hook and a-viz syntax would make
> it *more* complicated, not less.
>
> We're hoping to completely avoid generated accessors ({ get; set; } with
> no additional behavior) completely. Essentially, all they do is disable
> indirect modification (i.e. modifying arrays with []) and references
> (assigning a reference or extracting a reference). The reason they have to
> do that is that both of those things allow mutating the value of the
> property in a way where we cannot call the accessors, either because the
> engine isn't aware of it or because there's no simple way to "feed" the
> modified value into the setter. There are two primary reasons generated
> modifiers existed in the previous draft: 1. To avoid a BC break when adding
> a hook in the future and 2. to allow sub-classes to override the property
> while staying compatible with the parent.
>
> As described here (
> https://gist.github.com/iluuu1994/4ca8cc52186f6039f4b90a0e27e59180#use-case-5-arrays-properties)
> we've come to the conclusion that this issue of incompatibility is almost
> exclusively restricted to array properties. However, there are other
> reasons why publicly exposing an array property with write access isn't a
> good idea, as outlined in the gist. In other words, this compatibility
> layer provided by { get; set; } was provided for something that shouldn't
> be a publicly writable property in the first place. We're now hoping to
> completely avoid the { get; set; } syntax which also makes public
> private(set) a better fit, instead of offering a fake { get; private set; }
> syntax that is either limiting (i.e. not working properly for arrays) or
> semantically inconsistent (not preventing indirect modification and
> references).
>
> 3. More stylistically, it means visibility may end up split in two
> different locations.  That's harder for people to read.
>
> Especially if we add hooks later, it means they could be several lines
> a

Re: [PHP-DEV] [RFC][Vote] Asymmetric Visibility

[Nicolas Grekas]

> I am hereby opening the vote on the Asymmetric Visibility RFC:
>
> https://wiki.php.net/rfc/asymmetric-visibility
>
> Voting will be open for 2 weeks.
>
> While we would certainly prefer if everyone voted in favor, for those who
> vote against the authors kindly request that you indicate why, using the
> second poll question or comment section afterward.  Thank you.
>

Hi Larry, Ilija,

Thanks for opening the vote. I played a bit with the code and I didn't spot
any unexpected behavior, nice work.

I'm undecided for now, on two points:
1. the syntax: looking at https://wiki.php.net/rfc/property-hooks, I wonder
if using public string $foo {private set;} wouldn't be more appropriate.
Otherwise, it might be strange to have public private(set) string $foo {set
($value) { /* the setter */};} if hooks are the way forward. Sure we could
compact that but that would introduce two very different syntax whether a
setter is defined or not. WDYT?
2. the feature itself: I feel like this new syntax doesn't open new
capabilities over using methods, so this might "just" add complexity in the
end, adding alternative ways to do something already doable (I'm making the
argument for hooks also BTW). Would you be able to clarify why we need
this? (The only new capability that feels appealing to me is the "init"
modifier, but this is not what this RFC is about.)

About my point 2., did you consider a syntax to map a getter+setter to
properties? That might fill the need for asym visibility + hooks in one go?
I'm seeing quite often libraries that do this sort of mapping based on
conventions. This would bake the practice into the language:

public string $foo {get: getFoo, set: setFoo} or something like that, with
method visibility being added on top of the property visibility.

Sorry to chime in that late, I'm really undecided and discussing those
topics would help.

Thanks,
Nicolas

Re: [PHP-DEV] Re: [RFC][Dynamic class constant fetch]

[Nicolas Grekas]

Hi Ilija,

> I'd like to propose a simple RFC to introduce looking up class
> > constants by name. We have dedicated syntax for basically all other
> > language constructs. This RFC aims to get rid of this seemingly
> > arbitrary limitation.
> >
> > https://wiki.php.net/rfc/dynamic_class_constant_fetch
>
> Heads up: There hasn't been much discussion on this RFC in the last
> couple of weeks, thus I'd like to start the voting phase in the next
> couple of days.
>

Thanks for the reminder about this topic.

I'm wondering about this:

> This RFC proposes no change in the interaction between class constant
fetches and the null-coalescing operator ??. That is, Foo::{$bar} ?? null;
will throw an Error if the given constant does not exist. It would be
possible to suppress this error as is done for other types of member
accesses. However, it's not clear whether this is desirable, especially for
explicit class constant fetches. This change can be made in the future with
no backwards compatibility break.

Since an argument in favor of the RFC is to replace a function call by
native syntax, wouldn't it be desired to allow the same for checking the
existence of a const? I'm thinking about defined(), which the ?? operator
would nicely replace IMHO.

Are there technical reasons to not do it in this RFC? If not, could this be
considered?

Thanks,
Nicolas

Re: [PHP-DEV] [RFC] Asymmetric Visibility, with readonly

[Nicolas Grekas]

Le ven. 2 déc. 2022 à 12:32, Ilija Tovilo  a écrit :

> Hi Stephen
>
> > So here’s my last attempt:
> >
> > Please change this behaviour in your rfc.
> >
> > You are explicitly making it mutually exclusive with readonly now, so
> that’s not a bc break - if/when it becomes compatible with readonly the
> authors of that rfc can either keep the limitation as it exists **for
> readonly properties** (I’m not an expert but I don’t believe this would be
> a BC break either), or decide to drop the limitation completely (deliberate
> BC break)
> >
> > Keeping the limitation now on non-readonly properties makes no sense,
> and will be confusing to user land developers.
>
> This behavior is already not limited to readonly, it works the same
> for normal typed properties.
> https://3v4l.org/WpBp4
>
> The set-visibility is not relevant here: When unset has been called,
> __set will be called (and __get too). The set-visibility is only
> relevant when unset hasn't been called, as that will influence whether
> the property is written to or an error is thrown. The RFC is actually
> wrong saying this is specifically due to readonly properties. This
> behavior has been here for a while.
>
> See the relevant commit:
>
> https://github.com/php/php-src/commit/f1848a4b3f807d21415c5a334b461d240b2a83af
>
> > Assigning to an uninitialized typed property will no longer trigger
> > a call to __set(). However, calls to __set() are still triggered if
> > the property is explicitly unset().
>
> > This gives us both the behavior people generally expect, and still
> > allows ORMs to do lazy initialization by unsetting properties.
>
> > For PHP 8, we should fine a way to forbid unsetting of declared
> > properties entirely, and provide a different way to achieve lazy
> > initialization.
>
> I'm not sure if anybody actually makes use of this trick. It might be
> worth removing this weird behavior by offering an alternative. In any
> case, this is out of scope for this RFC.
>

Yes, there are critical features built on this, especially Ocramius'
ProxyManager and more recently Symfony's VarExporter, both to provide lazy
initialization. This is an important capability provided by the engine. It
should be preserved (in the current form or a better one possibly of
course.)

Nicolas

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

[Nicolas Grekas]

The code that Marco (Pivetta) shared was supposed to illustrate how
readonly classes can be useful to enforce some invariants on child classes.
Yet, here is another implementation that does use a readonly child class,
but still provides the behavior that was supposedly prevented by the
keyword:

readonly class MutableCounter extends ImmutableCounter {
private stdClass $count;
public function __construct(int $count) {
$this->count = (object) ['count' => $count];
}
public function add1(): self { return new self(++$this->count->count); }
public function value(): int { return $this->count->count; }
}


This counterexample shows that readonly classes do not provide any extra
safeguards.

As it stands, the readonly keyword on classes has two consequences:
1. it forces writing dummy boilerplate to work around the current
limitation while failing to provide any real guarantees
2. it gives false expectations - the exact one that we're discussions about
here (no, readonly classes don't help enforce any aspects of LSP)

1. is why I call the current restriction arbitrary, and 2. might be
dangerous since it would make people build on non-existing grounds.

If we stay with the current way, we'll just have another WTF in the
language IMHO. One that will make it harder to master PHP.


Le dim. 27 nov. 2022 à 17:51, Larry Garfield  a
écrit :

> On Sat, Nov 26, 2022, at 6:35 PM, Jordan LeDoux wrote:
> > On Sat, Nov 26, 2022 at 3:40 PM Deleu  wrote:
> >
> >>
> >> As I think more about this, there's nothing about the current RFC in
> this
> >> code sample. What's breaking LSP here is the child class doing state
> >> modification, not PHP.  To further expand that rationale, PHP allows us
> to
> >> create child classes. Whether that class will be LSP-safe or not is up
> to
> >> us, not up to PHP.
> >>
> >> However, the point still stands. Allowing child classes to break
> readonly
> >> will make it easier to build code that breaks LSP. The question then
> >> becomes: why is this being proposed and is it worth it?
> >>
> >
> > I cannot help but feel that the way `readonly` is being treated is going
> to
> > end up one of those things that is regretted. "Readonly does not imply
> > immutability". The fact that very nearly *every* single person who has
> not
> > worked on the RFCs has at some point been confused by this however should
> > be very telling.
> >
> > This comes from two *different* avenues that compound with each other to
> > *both* make this design head-scratching to me.
> >
> > First, in virtually all other technical contexts where the term
> "readonly"
> > is used, it means that the information/data cannot be altered. That is
> not
> > the case with readonly. In PHP, in this implementation, it is not
> > "readonly" in the sense that it is used everywhere else for computing, it
> > is "assign once".
> >
> > Second, the English words "read only", particularly to native speakers,
> > make this behavior very counterintuitive and confusing. I won't belabor
> > that point further.
> >
> > What "read only" really is, is "constructor initialize only". It honestly
> > has nothing to do with "read" as it's implemented.
>
> Not quite.  It really is just write-once.  The idea that you can only do
> that in the constructor is not in the language; that's been invented by
> over-eager static analysis tools.  (Everyone should disable that check.)
>
> > I guess I worry that this RFC makes `readonly` even more of a minefield
> for
> > PHP developers, increasing the mental load of using it in code while
> *even
> > further* watering down the benefits it may provide. It's already designed
> > in a somewhat counterintuitive way that I feel will be almost completely
> > replaced in actual code in the wild by "immutable" if PHP ever gets that.
>
> Working on asymmetric visibility, I have come to agree.  Nikita proposed
> readonly as a "junior version" of asymmetric visibility, to cover the most
> common use case without introducing more complexity.  At the time, he was
> confident that it wouldn't preclude expanding to asymmetric visibility in
> the future.  Well... I can say with confidence at this point that is not
> correct, and the design of readonly is causing issues for asymmetric
> visibility, and for cloning, to the point that (based on feedback in the
> other thread) we're likely going to for now forbid readonly and a-viz on
> the same property.
>
> At this point, I think I view readonly as a cautionary tale about the
> costs of doing "quick and easy" design over something more robust, because
> the quick-and-easy creates problems down the line that a more thoughtful,
> holistic view would have avoided.
>

To me, the best outcome of this discussion should be to retire readonly
*classes*. They were thought as a quick way to not repeat the readonly
keyword on every property, but they in fact introduce this behavior +
expectations related to child classes and these collide. From a conceptual
ground, I 

Re: [PHP-DEV] [VOTE] Improve unserialize() error handling

[Nicolas Grekas]

> > The other failing tests involve "unserialize_callback_func" to gracefully
> > detect class-not-found, and they are all plain broken by the RFC.
> >
> > I created this patch/PR to show the changes that would be required on
> > Symfony to work around the BC break:
> > https://github.com/symfony/symfony/pull/47882
> >
> > Note to readers: in this whole discussion, Symfony is just an example of
> > affected code. In the end, Symfony will adapt to the RFC if it passes.
> The
> > point being made is that PHP scripts should not have to be patched to run
> > on newer minor versions of PHP. That's what "keeping BC" means.
>
> Scripts will not need to be changed, unless they are already relying on
> observed behavior instead of the documented behavior.

When I use Symfony's 'PhpSerializer' and perform the following call:
>
> > $serializer->decode([
> > 'body' => '{"message": "bar"}',
> > ]);
>
> Then I can expect the input to be gracefully rejected with a
> MessageDecodingFailedException. This behavior is tested in the
> 'testDecodingFailsWithBadFormat' test.
>
> But *under no circumstances* must I perform the following method call:
>
> > $serializer->decode([
> > 'body' => 'O:8:"DateTime":0:{}',
> > ]);
>
> Because this payload is invalid and thus must not exist in practice.
>

That's the current behavior, yes. It allows graceful errors when one
misconfigures their workers and sends jsons to a worker that expects
php-ser. Under no circumstances do we need to guard against adversary
payloads. We could handle the situation you describe of course, but that'd
be just dead code in practice. ¯\_(ツ)_/¯



> > Breaking BC in the name of such invalid payloads doesn't make an argument
> > in favor of the RFC.
>
> I demonstrated how to adjust 'PhpSerializer' to improve the behavior for
> what I consider a reasonable use case without breaking any existing
> tests in step3b.diff and step3c.diff. At the same time the fixed version
> will automatically handle the change this RFC proposes. I even offered
> to adjust the RFC implementation to preserve the original Exception
> message and code.
>
> >> Failed asserting that exception message 'Could not decode message using
> >> PHP serialization: O:13:"ReceivedSt0mp":0:{}.' matches '/class
> >> "ReceivedSt0mp" not found/'.
> >>
> >> Okay, now the Exception message changed. Personally I do not consider
> >> this a BC break: I believe Exception messages are meant for human
> >> consumption, not for programs. Otherwise fixing a typo in the message
> >> would be a BC break. If the code wants to learn about the cause, it
> >> should either use the '$code' or different types of Exception should be
> >> thrown to clarify the cause by entering a different catch() block.
> >>
> >
> > Yes, the specific error message should be part of the BC promise. This
> > allows building test suites that can assert the message in a stable way.
>
> I'm not talking about test suites here. I believe makes sense to verify
> the error message to ensure a specific error message is emitted to the
> human observer in the error log.
>

Breaking test suites is the BC issue I wanted to highlight.



> I was talking about code that does something like this, which I consider
> to be inherently unsafe:
>
> try { … }
> catch (SomeException $e) {
>if ($e->getMessage() === 'Foobar') doSomething();
>else doSomethingElse();
> }
>
> As a library author I want to be able to provide the best possible
> Exception message to ease debugging for the user. This is not possible
> if I am locked into a bad choice forever.
>
> > This is also why we don't change the output of
> var_dump/print_r/var_export:
> > they're written now, in the same of BC, for the best of stability. (I've
> > barely read any PHP code where exception's code is used in any useful
> BTW -
> > that can't be a solution.)
>
> PDO is an example where the $code is useful, because it exposes the
> standardized SQL error code. Similarly a HTTP client could expose the
> status code within $code.
>
> For other libraries simply using a different Exception class within the
> same hierarchy is often sufficient to differentiate between different
> errors, because programmatically I often don't care *why exactly* an
> operation failed.
>
> > More importantly, the type of thrown exceptions should undoubtedly be
> part
> > of the BC promise.
>
> I agree.
>
> > Wrapping exceptions inside UnserializationFailedException breaks existing
> > catch/instanceof checks (see my PR above.)
> >
>
> unserialize() only promises you that a Throwable might be thrown:
>
>
> https://www.php.net/manual/en/function.unserialize.php#refsect1-function.unserialize-errors


Breaking BC at will unless it's properly documented looks like a terrible
policy. It'd make developing in PHP really risky. Especially here:
expecting that an exception would bubble down is just what everybody would
expect from unserialize(), like everything else in 

Re: [PHP-DEV] [VOTE] Improve unserialize() error handling

[Nicolas Grekas]

Hi Tim,

Thanks for taking the time to have a closer look. Unfortunately, you picked
the one failing test where there could be a discussion about whether the
original code needs improvement or not.

The other failing tests involve "unserialize_callback_func" to gracefully
detect class-not-found, and they are all plain broken by the RFC.

I created this patch/PR to show the changes that would be required on
Symfony to work around the BC break:
https://github.com/symfony/symfony/pull/47882

Note to readers: in this whole discussion, Symfony is just an example of
affected code. In the end, Symfony will adapt to the RFC if it passes. The
point being made is that PHP scripts should not have to be patched to run
on newer minor versions of PHP. That's what "keeping BC" means.


Resending the message without attachments, because the mailing list
> rejected the original due to exceeding 3 bytes:
>
> > ezmlm-reject: fatal: Sorry, I don't accept messages larger than 3
> bytes (#5.2.3)
>
> You can find the attachments at:
> https://gist.github.com/TimWolla/376dd162f7684daef38f76a07254871c
>
> Original message below:
>
> --
>
> On 10/15/22 11:06, Nicolas Grekas wrote:
> >> I'm not surprised by the “no” on the first vote based on the previous
> >> discussion. I am surprised however that you also disagree with raising
> >> the E_NOTICE to E_WARNING for consistency.
> >>
> >> I don't plan on repeating all the previous discussion points with you,
> >> but as you mention the Symfony tests specifically: Please find the patch
> >> attached. Do you believe the expectations in this new test would a
> >> reasonable expectation by a Symfony user?
> >>
> >
> > Since the beginning my point is not that the RFC doesn't have merits.
> It's
> > that the proposed approach breaks BC in a way that will affect the
> > community significantly. We have policies that say we should avoid BC
> > breaks and they should apply here.
> >
> > To anyone wondering about the reality of the BC break if you're not
> > convinced there is one because the original code is broken anyway (which
> is
> > your main argument Tim IIUC): please have a look at the failures I
> reported
>
> Yes, you understood this correctly: I consider the existing
> implementation to be broken. Unfortunately you did not answer my
> question whether the attached patch would be a reasonable expectation by
> a Symfony user.
>
> Let's presume it is: If I use the 'PhpSerializer' then I expect to
> receive exactly the 'MessageDecodingFailedException' whenever the
> message fails to decode. When decoding an erroneous DateTime (or an
> erroneous SplDoublyLinkedList or whatever) a different Exception will be
> thrown and not caught, thus the contract by the 'PhpSerializer' is
> violated.
>
> Let's fix this using test driven development. We first add the failing
> test cases. This is step1.diff (patches are in
> https://gist.github.com/TimWolla/376dd162f7684daef38f76a07254871c). When
> running the tests we get the following output:
>

I didn't answer the question because we ruled out such payloads in the
discussion thread: constructed payloads like the ones involving DateTime do
not exist in practice. Or if they do, it means the source of the payload is
not trusted, and then we are in much bigger trouble (security issues for
which the RFC can't do anything.)

Breaking BC in the name of such invalid payloads doesn't make an argument
in favor of the RFC.


> Failed asserting that exception message 'Could not decode message using
> PHP serialization: O:13:"ReceivedSt0mp":0:{}.' matches '/class
> "ReceivedSt0mp" not found/'.
>
> Okay, now the Exception message changed. Personally I do not consider
> this a BC break: I believe Exception messages are meant for human
> consumption, not for programs. Otherwise fixing a typo in the message
> would be a BC break. If the code wants to learn about the cause, it
> should either use the '$code' or different types of Exception should be
> thrown to clarify the cause by entering a different catch() block.
>

Yes, the specific error message should be part of the BC promise. This
allows building test suites that can assert the message in a stable way.
This is also why we don't change the output of var_dump/print_r/var_export:
they're written now, in the same of BC, for the best of stability. (I've
barely read any PHP code where exception's code is used in any useful BTW -
that can't be a solution.)

More importantly, the type of thrown exceptions should undoubtedly be part
of the BC promise.
Wrapping exceptions inside UnserializationFailedException breaks existing
catch/instanceof checks (see my PR above.)



> All gr

Re: [PHP-DEV] [VOTE] Improve unserialize() error handling

[Nicolas Grekas]

> > Not sure why I didn't think about it before but I just ran the test suite
> > of Symfony after applying the patch proposed in the RFC to change the way
> > exceptions are handled by unserialize.
> >
> > This change breaks the test suite of 5 separate components. I created
> this
> > gist to list all the failures:
> > https://gist.github.com/nicolas-grekas/3da652a51669baa40c99bd20e4a1b4dd
> >
> > Maybe I wasn't convincing enough during the discussion period, but that
> > doesn't change the fact that the proposed behavior in the RFC is a very
> > clear BC break that will affect userland significantly.
> >
> > I'm therefore voting NO on the proposal.
> >
>
> I'm not surprised by the “no” on the first vote based on the previous
> discussion. I am surprised however that you also disagree with raising
> the E_NOTICE to E_WARNING for consistency.
>
> I don't plan on repeating all the previous discussion points with you,
> but as you mention the Symfony tests specifically: Please find the patch
> attached. Do you believe the expectations in this new test would a
> reasonable expectation by a Symfony user?
>

Since the beginning my point is not that the RFC doesn't have merits. It's
that the proposed approach breaks BC in a way that will affect the
community significantly. We have policies that say we should avoid BC
breaks and they should apply here.

To anyone wondering about the reality of the BC break if you're not
convinced there is one because the original code is broken anyway (which is
your main argument Tim IIUC): please have a look at the failures I reported
above and wonder about the changes the RFC would impose. I cannot think
about one that would not imply some sort of "if the version of PHP is >=
8.3 then A, else B". This is the fact that highlights the BC break.

Nicolas

Re: [PHP-DEV] [VOTE] Improve unserialize() error handling

[Nicolas Grekas]

Hi Tim,

as announced on Wednesday [1] I've now opened the vote for:
>
> "Improve unserialize() error handling" [2]
>
> The RFC contains three votes, each of which requires a 2/3 majority. Two
> of the votes are for 8.x (8.3), one for 9.0.
>
> Voting will run 2 weeks until:
>
> 2022-10-28 at 14:00 UTC
>
> 
>
> Please find the below resources for your reference:
>
> RFC: https://wiki.php.net/rfc/improve_unserialize_error_handling
> PoC implementation:
> - https://github.com/php/php-src/pull/9425
> - https://github.com/php/php-src/pull/9629
>
> Discussion Thread: https://externals.io/message/118566
> Related Thread: https://externals.io/message/118311
>
> 
>
> [1] https://externals.io/message/118566#118807
> [2] https://wiki.php.net/rfc/improve_unserialize_error_handling
>

Not sure why I didn't think about it before but I just ran the test suite
of Symfony after applying the patch proposed in the RFC to change the way
exceptions are handled by unserialize.

This change breaks the test suite of 5 separate components. I created this
gist to list all the failures:
https://gist.github.com/nicolas-grekas/3da652a51669baa40c99bd20e4a1b4dd

Maybe I wasn't convincing enough during the discussion period, but that
doesn't change the fact that the proposed behavior in the RFC is a very
clear BC break that will affect userland significantly.

I'm therefore voting NO on the proposal.

Cheers,
Nicolas

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

[Nicolas Grekas]

Hi Tim,

I'm not quoting the full text of the discussion because the php ML server
refused the message - too long it said.

> this gist:
> > https://gist.github.com/nicolas-grekas/5dd3169f94ed3b4576152605330824fe
> >
> >
>
> [...] (see previous messages on the thread for this missing part)
>
> >> I'm asking, because the examples fail to explain the "why". Why is the
> >> code written like this? I fail to see how catching "Exception", but not
> >> catching "Error" during unserialization is ever going to be useful.
> >>
> >
> > It's useful because these two roots mean very different things.
>
> I understand the distinction between Error and Exception in general.
>
> However I've specifically asked about the distinction in the context of
> unserialization. An Exception during unserialization is not more or less
> a programmer error compared to an Error. You generally put in some
> opaque string into unserialize and you hopefully get out some useful
> value. If the string is broken you either get an E_FOO or some random
> Throwable you can't control. You can't magically fix the input string
> and you can't really prevent the errors happening either.
>
> As pointed out in the list above: DateTimeImmutable will already throw
> an Error, whereas other classes will throw an Exception. That does not
> mean that an error during unserialization of DateTimeImmutable is worse,
> because the choice is pretty arbitrary.
>
> The documentation of unserialize()
> (https://www.php.net/manual/en/function.unserialize.php) also indicates
> that:
>
>  > Objects may throw Throwables in their unserialization handlers.
>
> Further indicating that you are not guaranteed to only get Exception or
> only get Error out of it.
>
> >
> >> Likewise I don't see how catching ErrorException specifically is useful
> >> when '__unserialize()' might throw arbitrary Throwables.
> >
> >
> > It's useful when all you need is eg to care about the exceptions thrown
> by
> > a custom error handler, and want to ignore the other ones (because other
> > exceptions aren't part of the current domain layer.)
> >
>
> Basically see response to previous quote.
>
> >> Quoting myself, because the examples didn't answer the question: "I am
> >> unable to think of a situation where the input data is well-defined
> >> enough to reliably throw a specific type of exception, but not
> >> well-defined enough to successfully unserialize".
> >>
> >> And don't get me wrong: I'm not attempting to say that it is appropriate
> >> to break logic that I personally consider wrong without justification. I
> >> believe the benefits of having the unified Exception outweigh the
> >> drawbacks of introducing a behavioral change in code that is already
> >> subtly broken depending on the exact input value passed to
> unserialize().
> >>
> >
> > The thoughts you describe might make sense as a rule of thumb principle
> > when you write your code. It might even be upgraded to a best practice.
> > Generally turning an Error to an Exception is not by my book, but I'm not
> > arguing about this. My point is : /!\ BC break ahead, do not cross /!\
> >
> > If you look at the gist I linked before, wrapping all throwables would
> > break most if not all the cases I listed. That's bad numbers, and I don't
> > think this is specific in any way to the Symfony codebase.
> >
>
> Based on my analysis and understanding of the linked examples, the only
> one that *might* break (I don't fully understand it), is the one in
> src/Symfony/Component/VarExporter/Internal/Registry.php. The others will
> either no change or change to be more correct.
>
> >
> >> This is similar to the attribute syntax breaking hash comments that
> >> start with a square bracket without any space in the middle. Or the '@@'
> >> attribute syntax that was also proposed. It would have broken code
> >> containing duplicated error suppression operators.
> >>
> >> Similarly to the attribute breakage, it's also reasonably easy to find
> >> possibly affected logic by grepping for 'unserialize('.
> >>
> >
> > Sorry to disagree, that's a very different sort of break. One that
> changes
> > the behavior of perfectly fine apps.
> >
>
> Disagreeing is fine. Without disagreement we would not need the RFC
> process. However I disagree on the "perfectly fine" part. I believe that
> some of the Symfony examples are already subtly broken (the
> DateTimeImmutable part).
>

As 

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

[Nicolas Grekas]

>
> > 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.
>> >
>>
>> What's your take about 8.2? As I demonstrated, readonly classes are broken
>> because of this propagation to child classes.
>>
>
> s/broken/working as expected
>

broken. see thread

>

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

[Nicolas Grekas]

Hi all,


> 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.
>

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'm looking forward to your work on cloning! Note that "clone-with" or
similar is a separate issue from the current inability to clone readonly
properties within the __clone method. It might be easier to tackle them
independently, from a discussion pov.

Cheers,
Nicolas

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

[Nicolas Grekas]

Hi Tim,

I'm a bit busy with conferences these days...

On 9/12/22 21:46, Nicolas Grekas wrote:>> unserialize() is a generic
> function that will also call arbitrary
> >> callbacks. You already have no guarantees whatsoever about the kind of
> >> exception that is thrown from it. I am unable to think of a situation
> >> where the input data is well-defined enough to reliably throw a specific
> >> type of exception, but not well-defined enough to successfully
> unserialize.
> >>
> >> So on the contrary wrapping any Exceptions with a well-defined Exception
> >> allows you to rely on a specific and stable Exception to catch in the
> >> first place.
> >>
> >> As you specifically mention catch(Throwable), I'd like to point out that
> >> this will continue to work, because UnserializationFailedException
> >> implements Throwable.
> >>
> >
> > You might have misunderstood my point so let me give two examples:
> >
> > Example 1.
> > set_error_handler(fn ($t, $m) => throw new ErrorException($m));
> > try {
> > $ser = serialize($value);
> > catch (ErrorException $e) {
> >// deal with $e
> > finally {
> >restore_error_handler();
> > }
> >
> > Example 2.
> > try {
> > $ser = @serialize($value);
> > catch (Exception $e) {
> >// deal with $e but don't catch Error on purpose, to let them bubble
> down
> > }
> >
> > Changing serialize to wrap any throwables into an
> > UnserializationFailedException would break both examples. That's what I
> > mean when I say this breaks BC. As any BC-break, this might cause big
> > troubles for the community and this should be avoided. The release
> process
> > policy I mentioned above is a wise document.
> >
>
> First I'd like to note that these examples use 'serialize()' (which is
> not touched by my RFC, because serialize rarely fails). I consider this
> a typo and will answer as if you used 'unserialize()'. If that was not a
> typo, then this might explain the confusion.
>

It was a typo! I meant unserialize() yes.


> Are those two examples based on real-world use cases or did you craft
> them specifically to point out how the proposal would introduce a
> behavioral change?
>

They were inspired by what I found in the Symfony codebase.

I just conducted a quick lookup at this code base: most call to
unserialize() are not checked at all, but I hghlighted the checked ones in
this gist:
https://gist.github.com/nicolas-grekas/5dd3169f94ed3b4576152605330824fe



> I'm asking, because the examples fail to explain the "why". Why is the
> code written like this? I fail to see how catching "Exception", but not
> catching "Error" during unserialization is ever going to be useful.
>

It's useful because these two roots mean very different things.

That's why Error has been introduced in the first place: instances of Error
are not just exceptions, they're really programming mistakes. As such the
best practice to me is to *not* catch them in domain layers, and let them
bubble down to a generic Error catch layer instead.



> Likewise I don't see how catching ErrorException specifically is useful
> when '__unserialize()' might throw arbitrary Throwables.


It's useful when all you need is eg to care about the exceptions thrown by
a custom error handler, and want to ignore the other ones (because other
exceptions aren't part of the current domain layer.)


Specifically
> see the list in this email: https://externals.io/message/118311. Code
> outside of the function that actually calls unserialize() is not going
> to be prepared to usefully handle unserialization failure. This
> effectively means that the Exception will bubble up to the global
> Exception handler (or the Exception handler middleware), resulting in an
> aborted request.
>

Yep, this is fine in many cases.



> Quoting myself, because the examples didn't answer the question: "I am
> unable to think of a situation where the input data is well-defined
> enough to reliably throw a specific type of exception, but not
> well-defined enough to successfully unserialize".
>
> And don't get me wrong: I'm not attempting to say that it is appropriate
> to break logic that I personally consider wrong without justification. I
> believe the benefits of having the unified Exception outweigh the
> drawbacks of introducing a behavioral change in code that is already
> subtly broken depending on the exact input value passed to unserialize().
>

The thoughts you describe might make sense as a rule of thumb principle
when you write your code. It might even be upgraded to a best practice.
Generally turnin

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

[Nicolas Grekas]

> >> Personally I'm undecided at the moment whether or not it should be
> >> allowed.  I'm sympathetic to the "it's easier to disallow and allow
> later
> >> than vice versa" argument, but still not sure where I stand.  The above
> at
> >> least gives a concrete example of where the problem would be.
> >>
> >
> > If we want the safety you describe, we might want a stronger version of
> it.
> > Let's name it immutable. An immutable property/class would be like a
> > readonly property with the additional restriction that only an immutable
> > value could be assigned to it (scalars + array + immutable classes.) But
> > that's another topic.
> >
> > Your example made me doubt for a moment, but without any convincing
> > purpose, this should help decide that child classes shouldn't have to be
> > readonly.
> >
> > Nicolas
>
> Another question I think is pertinent to ask: What kinds of objects
> generally would be proxied in this way, and are those the kinds of objects
> where a readonly object would make sense?
>
> Off hand, I'd expect this kind of proxying to be used mainly on services,
> whereas readonly classes would be mostly value objects.  So the problem
> space may be smaller than it initially appears.
>

It's very difficult to anticipate this. Here is a counter argument:
About services: the best ones are of the functional sort; they don't hold
any states and their dependencies are also pure functional units. Aka
they're immutable and they're good candidates for readonly classes.
About entities: e.g. Doctrine proxies them all.
So no, I wouldn't say that services are the most common things to proxy,
and that entities are the most common things to use readonly on...

I hope this discussion illustrated why we should remove propagating
readonly to child classes: it serves no purpose and it breaks a universal
design pattern. There is no rationale that supports the current behavior.

Mate, WDYT?

Cheers,

Nicolas

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

[Nicolas Grekas]

> On 9/8/22 19:06, Nicolas Grekas wrote:
> >  From what I understand, there are two things in the RFC:
> >
> > 1. a proposal to wrap any throwables thrown during unserialization
> > inside an UnserializationFailedException
>
> Correct.
>
> > 2. a discussion to figure out a way to turn these notices+warnings
> into
> > exceptions.
>
> Partly correct:
>
> The RFC primarily proposes unifying the existing non-Exception errors
> (which are currently split between E_WARNING and E_NOTICE with no
> apparent pattern). To *what* exactly (E_WARNING or some Exception) they
> are unified is up for discussion/vote.
>

Thanks for clarifying. Our accepted release process policy
<https://wiki.php.net/rfc/releaseprocess> says "Backward compatibility must
be respected with the same major releases". I think it's critical to stick
to this policy as much as possible and also to plan for the smoothest
upgrade path when preparing the next major (deprecations FTW.)
That's what I have in mind when I review the RFC and where I don't see the
match yet. I know we're only at the beginning of the discussion and we're
still looking for what would work best.



> > About 1., I read that you think "this is not considered an issue", but to
> > me, changing the kind of exceptions that a piece of code might throw is a
> > non negligible BC break. There needs to be serious justification for it.
> I
> > tried looking for one, but I'm not convinced there is one: replacing the
> > existing catch(Throwable) by catch(UnserializationFailedException) won't
> > provide much added value to userland, if any. And "reliably include more
> > than just the call unserialize() in the try" is not worth the BC break
> IMHO.
>
> unserialize() is a generic function that will also call arbitrary
> callbacks. You already have no guarantees whatsoever about the kind of
> exception that is thrown from it. I am unable to think of a situation
> where the input data is well-defined enough to reliably throw a specific
> type of exception, but not well-defined enough to successfully unserialize.
>
> So on the contrary wrapping any Exceptions with a well-defined Exception
> allows you to rely on a specific and stable Exception to catch in the
> first place.
>
> As you specifically mention catch(Throwable), I'd like to point out that
> this will continue to work, because UnserializationFailedException
> implements Throwable.
>

You might have misunderstood my point so let me give two examples:

Example 1.
set_error_handler(fn ($t, $m) => throw new ErrorException($m));
try {
   $ser = serialize($value);
catch (ErrorException $e) {
  // deal with $e
finally {
  restore_error_handler();
}

Example 2.
try {
   $ser = @serialize($value);
catch (Exception $e) {
  // deal with $e but don't catch Error on purpose, to let them bubble down
}

Changing serialize to wrap any throwables into an
UnserializationFailedException would break both examples. That's what I
mean when I say this breaks BC. As any BC-break, this might cause big
troubles for the community and this should be avoided. The release process
policy I mentioned above is a wise document.



> > About 2., unserialize() accepts a second argument to give it options. Did
> > you consider adding a 'throw_on_error' option to opt-in into the behavior
> > you're looking for? I think that would be a nice replacement for the
> > current logics based on set_error_handler(). If we want to make PHP 9
> throw
> > by default, we could then decide to deprecate *not* passing this option.
>
> I did not consider this when writing the RFC. I now considered it, and I
> do not believe adding a flag to control this a good thing.
>
> 1. "No one" is going to set that flag, because it requires additional
> effort. I strongly believe that the easiest solution should also the
> correct solution for the majority of use cases. The flag fails this
> "test", because the correct solution should be "don't fail silently".
>

Unless we deprecate calling the method without the argument as I suggested.
I agree, it might not be ideal to ask the community to rewrite every call
to serialize($v) to serialize($v, ['throw_on_error' => true/false]) but
that's still way better than a BC break.
Maybe there's another way that doesn't break BC. I'm looking forward to one.

2. If you actually want to set that option in e.g. a library, then you
> break compatibility with older PHP versions for no real gain. If you go
> all the way and remember to add that extra flag, then you can also write
> an unserialize wrapper that does the set_error_handler dance I've shown
> in the introduction of the RFC. Similar effort to adding the flag, but
> better c

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

[Nicolas Grekas]

Hello Larry,


> 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?
>
> I don't have a real-world example, but a general pattern.  The advantage
> of a readonly object (either conventionally like PSR-7 or enforced by the
> `readonly` keyword) is that you can store a reference to it without having
> to worry about it changing out from under you.  That means you can code on
> the assumption that any data derived from that object is also not subject
> to change.
>
> If you accept a readonly object of type Foo, you would write code around
> it on that assumption.
>
> If you're then passed an object of type Bar extends Foo, which has an
> extra non-readonly property, that assumption would now be broken.  Whether
> that counts as a Liskov violation is... a bit squishy, I think.
>
> Where that might be a problem is a case like this:
>
> readonly class Person {
>   public function __construct(public string $first, public string $last) {}
>
>   public function fullName() {
> return "$this->first $this->last";
>   }
> }
>
> class FancyPerson extends Person {
>   public string $middle = '';
>
>   public function setMiddle(string $mid) {
> $this->middle = $mid;
>   }
>
>   public function fullName() {
> return "$this->first $this-middle $this->last";
>   }
> }
>
> class Display {
>   public function __construct(protected Person $person) {}
>
>   public function hello() {
> return "Hello " . $this->person->fullName();
>   }
> }
>
> Now, Display assumes Person is readonly, and thus fullName() is
> idempotent, and thus Display::hello() is idempotent.  But if you pass a
> FancyPerson to Display, then fullName() is not safely idempotent (it may
> change out from under you) and thus hello() is no longer idempotent.
>
> Whether or not that problem actually exists in practice is another
> question.  And to be fair, the same risk exists for conventionally-readonly
> classes today (PSR-7, PSR-13, etc.), unless they're made final.  Arguably
> the safety signaling of a lexically readonly class is stronger than for a
> conventionally readonly class, so one would expect that to not be broken.
> But that's the case where a non-readonly child of a readonly parent can
> cause problems.
>

Thanks for constructing this example, that's good food for thoughts.
Unfortunately, The code following readonly child class shows that this
safety you describe doesn't exist:

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";
  }
}


> Personally I'm undecided at the moment whether or not it should be
> allowed.  I'm sympathetic to the "it's easier to disallow and allow later
> than vice versa" argument, but still not sure where I stand.  The above at
> least gives a concrete example of where the problem would be.
>

If we want the safety you describe, we might want a stronger version of it.
Let's name it immutable. An immutable property/class would be like a
readonly property with the additional restriction that only an immutable
value could be assigned to it (scalars + array + immutable classes.) But
that's another topic.

Your example made me doubt for a moment, but without any convincing
purpose, this should help decide that child classes shouldn't have to be
readonly.

Nicolas

Re: [PHP-DEV] RFC [Discussion]: Improve unserialize() error handling

[Nicolas Grekas]

Hi Tim,

Thanks for the RFC.

I've now written up an RFC as a follow-up for the "What type of
> Exception to use for unserialize() failure?" thread [1]:
>
> 
>
> RFC: Improve unserialize() error handling
> https://wiki.php.net/rfc/improve_unserialize_error_handling
>
> Proof of concept implementation is in:
>
> https://github.com/php/php-src/pull/9425
>
> Discussion period for that RFC is officially opened up.
>
> 
>
> The primary point of discussion in the previous mailing list thread and
> in the PR comments is whether unserialize() should continue to emit
> E_WARNING or whether that should consistently be changed to an
> Exception. As of now I plan to explicitly vote on this and the RFC
> contains some opinions on that matter.
>

>From what I understand, there are two things in the RFC:

   1. a proposal to wrap any throwables thrown during unserialization
   inside an UnserializationFailedException
   2. a discussion to figure out a way to turn these notices+warnings into
   exceptions.

About 1., I read that you think "this is not considered an issue", but to
me, changing the kind of exceptions that a piece of code might throw is a
non negligible BC break. There needs to be serious justification for it. I
tried looking for one, but I'm not convinced there is one: replacing the
existing catch(Throwable) by catch(UnserializationFailedException) won't
provide much added value to userland, if any. And "reliably include more
than just the call unserialize() in the try" is not worth the BC break IMHO.

About 2., unserialize() accepts a second argument to give it options. Did
you consider adding a 'throw_on_error' option to opt-in into the behavior
you're looking for? I think that would be a nice replacement for the
current logics based on set_error_handler(). If we want to make PHP 9 throw
by default, we could then decide to deprecate *not* passing this option.

Lastly I'd like to add a 3. to your proposal, because there is one more
thing that makes unserialization annoying: the unserialize_callback_func

ini setting. It would be great to be able to pass a 'callback_func' option
to unserialize to provide it, instead of calling ini_set() as we have to
quite often right now.

Would that make sense to you?

Kind regards,
Nicolas

[PHP-DEV] Re: Issues with readonly classes

[Nicolas Grekas]

Hi Mate

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.
>

Thanks for the precision, works for me As I mentioned, there are ways
around anyway (storing in another class or in a static variable.)


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?
>

Not only can I not find where removing this invariant would be a problem, I
also cannot think of where this invariant would be useful. I don't see what
I can build on top of it...



> 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.
>

During cloning only, yes, and we don't need any new syntax to allow this
(we might need one to make it easier to write wither APIs, but that's
another topic.)
That's exactly what I'm trying to achieve in
https://github.com/php/php-src/pull/9403. For the record, I tried 3
approaches to manage the transient state that is required to flag readonly
properties as once-writeable during cloning. I tried using zval.u1 but I
realized this is a read-only area right?, then tried using zval.u2.extra
but this is not a bitfield, then zval.u2.property_guard but without luck. I
didn't see any blocker either in the engine, so the only reason why I
failed is because my C-related skills are too weak and I'd need help to
figure out a proper patch.

Nicolas

>

Re: [PHP-DEV] Issues with readonly classes

[Nicolas Grekas]

>Hi Marco,
> >
> >IMO good as-is: can be relaxed later (8.3 or later), if anybody believes
> >> it's a show-stopper.
> >>
> >Readonly classes don't really need to be lazy.
> >>
> >
> >"Break things and move slow" doesn't feel right to me for the language.
>
> It's a new keyword that you have to opt into. So it's not breaking
> anything. It just means you might not be able to use it for your use case
> yet.
>
> (and before you say that third party code might add it, you can just say
> you don't support it yet).
>

It's not like it wouldn't be supported *yet*. It won't be supported *by
design of the language*.

Readonly classes introduce a new concept that isn't backed by any language
theory I've read about.
Of course it doesn't break until you use it, but conceptually, it breaks a
universal design pattern: inheritance-based proxy decorators.
What I'm asking for is: what is the justification for this limitation? If
there is none, we should drop it. Preemptively breaking universal concepts
(decoration) by introducing a never-seen concept might be innovation for
sure. Or it might be a mistake. E.g. do we know any other language that
provides something like readonly classes? How do they deal with inheritance?
Can anyone answer those questions?

If we come to the conclusion that there is no backing theory to support
propagating the readonly flag to child classes, then we might decide if we
want to fix this in 8.2 or 8.3. I know I would prefer acting in 8.2, but
that's a separate discussion.

Nicolas

Re: [PHP-DEV] Issues with readonly classes

[Nicolas Grekas]

Hi Marco,

IMO good as-is: can be relaxed later (8.3 or later), if anybody believes
> it's a show-stopper.
>
Readonly classes don't really need to be lazy.
>

"Break things and move slow" doesn't feel right to me for the language.
Maybe you don't see the need for lazy readonly classes, but that's
nonetheless inconsistent.

Note that lazy-loading is just an example. Any kind of inheritance-based
decorator will be broken, as far as cloning is concerned. This should be
discussed to me.

Nicolas

[PHP-DEV] Issues with readonly classes

[Nicolas Grekas]

Hello everybody, hi Mate

I played with readonly classes recently, to add support for them to
lazy-loading proxy generation and I struggled upon what I see as serious
issues.

As noted in the RFC, and because the readonly flag must be set on child
classes, those cannot:

   1. Declare a new non-readonly property;
   2. Have static properties;
   3. Have dynamic properties.

I figured out 1. and 2. not by reading the RFC but because I needed those
capabilities in the generated inheritance proxies.

1. is the most serious one: being able to declare a non-readonly property
is the only way to implement a cloneable proxy (to be allowed to do
$this->realInstance = clone $this->realInstance; in __clone()). I think
there are two ways to solve the use case I have: A. fix the
non-cloneability of readonly props or B. allow child classes to add
non-readonly properties. Because to me this is a serious restriction, A or
B should ideally be in 8.2. I think A is a must-have so that might be the
thing to do. 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.)

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.

For 3. I just noted the limitation to be exhaustive, but I'm really fine if
dynamic properties remain forbidden on child classes. It might be seen as
inconsistent with other classes, but dynamic properties don't have any
purpose anyway since there is always an alternative to them.

I understand there are good intentions behind these limitations (making PHP
a stricter language). But here, I feel like this is detrimental to the
language, by making it a minefield of kinda arbitrary dos and don'ts. At
least that's what I feel here.

Does that make sense to you? Should we remove the limitations I mention?

Nicolas
PS: About allowing readonly props to be reinitialized during cloning, I
tried https://github.com/php/php-src/pull/9403 but I failed as I can't make
it work. I'd appreciate any help on the topic. I'd even be happy to close
if someone else would like to work on the topic. Could that be you?

Re: [PHP-DEV] [RFC] Asymmetric visibility

[Nicolas Grekas]

> Ilija Tovilo and I are happy to present the first new RFC for PHP 8.3:
> Asymmetric Visibility.
>
> https://wiki.php.net/rfc/asymmetric-visibility
>
> Details are in the RFC, but it's largely a copy of Swift's support for the
> same.
>

Thank you Larry and Ilija for this RFC.

After reading it, I'm wondering about inheritance rules. What would be the
rules when redeclaring an existing asymmetric property in a child class?
The RFC should mention them I believe.

But I'm also wondering about the use case where asymmetry would be useful
in practice now that we have readonly?
I voted against readonly at the time because it didn't address cloning, and
also because it deprives end-users vs code authors IMHO (alike "final"). I
would have very much prefered asymmetric visibility back then. But now that
we are here, what are the use cases? If it's cloning only, then I would
very much prefer finding a way to fix it rather than providing two subtly
different ways to achieve the same thing.

Eg can't we allow __clone methods to bypass the readonly restrictions? The
engine already handles "guards" when using magic property accessors. Can't
we use something similar for clone?

And if yes, what would remain of the motivations for the asymmetric RFC?

Cheers,
Nicolas

Re: [PHP-DEV] [RFC][Vote] New Curl URL API

[Nicolas Grekas]

> Hi internals,
>
> I opened voting for the new Curl URL API as part of PHP8.2.
>
> All recent discussions show that we are not even close to getting a
> consensus on how the new CurlUrl OO API should be done. After changing my
> mind 300 times in the last day, I decided to only propose the procedural
> implementation that stays consistent with other functions of the ext/curl
> to target 8.2.


I fully support this approach: php-src is not the proper layer to build an
OOP API to me. Each time it's been done, it proved too rigid for userland
and ppl wrote wrappers around.

I think php-src should continue to provide thin layers on top of C
libraries so that userland is then empowered to build and evolve OOP APIs
at will.

Nicolas

Re: [PHP-DEV] [RFC][Vote] Auto-capture closures

[Nicolas Grekas]

Hi there,


Greetings, Internalians.
>
> The vote for auto-capture closures is now open.  It will run until 15 July.
>
> https://wiki.php.net/rfc/auto-capture-closure
>

Thanks for the RFC, I voted yes as I think the optimized auto-capturing
logic is sound.

Having "function()" still be a first-class citizen is also important to me
as there are cases where one might prefer the explicit nature of capturing,
so I join Rowan on this "rhetoric" topic.

I'm just wondering if there could be a way to enable the auto-capture
optimization for arrow functions. Having three kinds of rules for capturing
feels too much, especially when there are that subtles for short closures.
Maybe a deprecation could be possible for affected arrow functions?

Nicolas

Re: [PHP-DEV] [RFC][Under discussion] Fetch properties in const expressions

[Nicolas Grekas]

> > Hi everyone
> >
> > I'd like to start a discussion on a simple RFC to allow fetching
> > properties in constant expressions.
> > https://wiki.php.net/rfc/fetch_property_in_const_expressions
> >
> > The RFC proposes adding support for fetching properties in constant
> > expressions using the -> operator. I'm looking forward to your
> > feedback.
> >
> > Regards,
> > Ilija
>
> The enum-in-attribute use case came up recently in Symfony.  I'm in favor
> and the RFC looks good to me.
>

Genuine question:

In the thread about auto-implementing Stringable for string backed enums,
there is extensive argumentation to explain why ppl that use enums this way
are wrong. You mentioned writing a blog post about these reasons Larry, and
Benjamin (/cc) also expressed a similar opinion.

So my question is: don't these arguments apply here also? Shouldn't we
reject this RFC and tell ppl to use consts instead? If not, why?

Nicolas

Re: [PHP-DEV] [RFC] [Under Discussion] Auto-implement Stringable for string backed enums

[Nicolas Grekas]

Hi Guilliam,


> > There are also cases where using "->value" is just not possible. I
> mention
> > attributes in the RFC,
>
> which also mentions
> https://wiki.php.net/rfc/fetch_property_in_const_expressions (but with
> "For people that use non-strict mode, this extra “->value” is
> boilerplate that they'd better remove")
>

Absolutely.

Providing a nice and expressive syntax is critical. We added e.g. short
closures, constructor property promotion, etc. all because we care to not
write ugly code.

With the "Fetch properties of enums in const expressions" RFC, we soon
might be able to reference values in enums with this syntax:
#[MyAttr(MyEnum::Case->value)]

I understand why we're considering this and this might be only me, but I
find this ugly.
The same happens when coding: take_string($enum->value)

This ugliness is part of the problem I'd like to solve. Rowan's proposals
about sets could solve this in a very nice way , but we're not there yet. I
can wait though.



> Symfony YAML has a `!php/const X` feature, which also works when X is
> an Enum::CASE; how about a `!php/enum_value` feature?
>

I submitted something similar today at
https://github.com/symfony/symfony/pull/46771

Otherwise, I also like Rowan's suggestion of implementing "internal
> cast handlers", so that non-strict users could call e.g.
>

I had a look at gmp for example: cast handlers don't work when calling a
function.
They do work when explicit casting and when doing loose comparisons, but
they don't when calling functions (or returning from one.) I don't know the
underlying reason for that behavior but it looks consistent. Unfortunately
that idea looks like a dead end.

In any case, several people requested that it should require to be
> *opted-in* explicitly; but then [for solutions other than "allowing
> user-land to implement Stringable"] we probably also need a way to
> test whether a BackedEnum [instance] is "coercible"?
>

I don't have a solution that meets all the requirements that ppl expressed
so far.
If I could get back in time, I would enable that missing casting rule I
described for gmp in non-strict mode, and I would implement it on backed
enums from the start.

I'm now considering withdrawing the RFC because I don't see a way forward
that could be consensual enough.
If other ppl share my concerns and have a proposal, please let us know.

Nicolas

Re: [PHP-DEV] [RFC] [Under Discussion] Auto-implement Stringable for string backed enums

[Nicolas Grekas]

> domain SymfonyPermission: string;
> domain AcmePermission: string { 'admin' | 'user' | 'bot' };
> [...]
> Domains can also be considered sets, which you could compare directly,
> and maybe even calculate intersections, unions, etc:
>
> The actual values would be ordinary strings, and type constraints would
> just be checking the value passed against the domain:
>
> Crucially, this solves the described problem of a library accepting an
> infinite (or perhaps just very wide) set of values, and a consuming app
> wanting to constrain that set within its own code.
>
> It's one disadvantage is the typo-proofing and look up availability that
> constants give, but you could always combine the two.
>

Thanks for this idea Rowan, that's really interesting.
I would go one step further and require naming the values in the set.
Borrowing from the syntax of enums, we could have:

set AcmePermission: string {
  case ADMIN = 'admin';
  case USER = 'user';
  case BOT = 'bot';
}

Then AcmePermission::ADMIN would === 'admin' and, as you said,
AcmePermission could be used as a type on functions:

function (AcmePermission $perm): AcmePermission
{
$perm instanceof AcmePermission; // true

return $perm;
}

In order to make this work, we would need to be able to autoload the type
AcmePermission.This could be done by the type-checking logic: when checking
$var against AcmePermission and the type is undefined, if $var is an int or
a string, call the autoloader for 'AcmePermission'.

There are other things to consider, like possible changes to reflection,
whether such a set also declares a class like enums do, etc.But overall, I
really like it and I agree with you: this would provide a really good
solution for the problem ppl try to use enums for.

I don't know how we could move forward on that idea. By starting another
thread?

Nicolas

Re: [PHP-DEV] [RFC] [Under Discussion] Constants in traits

[Nicolas Grekas]

> >
> > I'd like to start a discussion on an RFC to allow defining constants in
> traits.
> > https://wiki.php.net/rfc/constants_in_traits
> >
> > I'm looking forward to your feedback, including corrections on English
> wordings.
> >
> > Thanks!
> >
> > --
> > Shinji Igarashi
>
> I am initially lukewarm.  One thing not addressed in the RFC that should
> be: Why were constants left out of traits previously, and what has changed
> to make them make sense to include now?  (I don't recall, honestly, so I
> have no strong feelings one way or the other yet.)
>

That!

I'm also wondering why the default value of a const (and a property) could
not be changed by the class importing the trait? This sometimes hits me and
the original RFC doesn't explain why this is needed.

I added Stefan Marr in the loop in case he still checks this email address
and would like to shed some light on those questions. WDYT Stefan?

And I'd also be happy to see an implementation of this before voting, to be
sure this is something that can be achieved without too many troubles. Do
you think this is possible?

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] [Under Discussion] Auto-implement Stringable for string backed enums

[Nicolas Grekas]

Hi Benjamin and Derick,

I'm replying to both of you because I see some things in common in your
comments.

>https://wiki.php.net/rfc/auto-implement_stringable_for_string_backed_enums


I would prefer if this was an explicit opt-in to have a __toString on a
> backed enum. Maybe a special trait for enums that has the implementation,
> so that a custom __toString cannot be implemented or a new syntax "enum Foo
> : Stringable".
>

We can make this opt-in by simply allowing user-land to implement
Stringable.
This is a different solution to the problem the RFC tries to solve and I
would also personally be fine with.
I would then *not* try to limit which implementation of it is allowed by
the engine. Instead, I would give all powers to user-land to decide on
their own what makes sense for their use case. As a general principle, I
believe that empowering user-land is always a win vs trying to "save them
from themselves", as if we knew better than them what they want to achieve,
and especially how.


> My concern is that auto implementing __toString, will lead to decreasing
> type safety of enums in weak typing mode, since they get auto-casted to
> string when passed to a function accepting strings. This effectively adds
> more type juggling cases.
>

I don't share this concern: if an API accepts a string and the engine can
provide a string, let it do so. There is nothing inherently dangerous in
doing so. But we don't need to agree on that if the proposal above makes
sense to everybody :)


> The example in the RFC about attributes accepting strings or Enums can be
> solved by union types on the side of the library developers, it doesn't
> need to be magically implemented by the engine.
>

I extensively explain in the RFC why this should not be on the side of lib
authors, but on the side of end-users. Please double check and let me know
your thoughts.


> I don't consider "use strict mode" a good argument to avoid this problem,
> because that has other downsides such as overcasting.
>

I'm 100% aligned with that.

2. It is not implemented for all enum types, only for string backed ones.
> This change would now make different classes of enums, without them being
> differently inherited classes.
>

This would also be solved by allowing user-land to implement Stringable on
*all* kind of enums. Would that make sense to you?


> 3. The main use case seems to be to prevent having to type ->value, which
> would otherwise indicate reliably whether an enum is used as string
> argument.
>

Yep, that's a "strict mode" approach and this RFC mostly applies to
non-strict mode.
There are also cases where using "->value" is just not possible. I mention
attributes in the RFC, but we also have a case in Symfony where defining
service definitions in yaml doesn't work with enums because there is no way
to express the "->value" part.

If that's the consensus, I'm fine updating the RFC to turn the vote into
whether "allowing user-land to implement Stringable on any kind of enums"
is desired or not.

Nicolas

[PHP-DEV] [RFC] [Under Discussion] Auto-implement Stringable for string backed enums

[Nicolas Grekas]

Hi everyone!

I'd like to open a discussion on this RFC, to auto-implement Stringable for
string-backed enums:
https://wiki.php.net/rfc/auto-implement_stringable_for_string_backed_enums

I'm looking forward to your feedback,

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement

[Nicolas Grekas]

> An RFC has been created to fix an issue in Random Extension 5.x.
>
> https://wiki.php.net/rfc/random_extension_improvement
>
> Voting on this RFC will begin in two weeks (2022-07-02), in time for the
> PHP 8.2 Feature Freeze. (Vote finished in 2022-07-16, Feature Freeze is
> 2022-07-19)
>
> In the unlikely event that the Random Extension 5.x RFC is rejected, this
> RFC will become invalid regardless of the outcome of the vote.
>

Hi, thanks for the update, that makes sense to me.

I'm wondering: does Random\SerializableEngine extend Random\Engine? Can
this be mentioned in the RFC? If not, what about making it this way? Having
this interface only contain __(un)serialize would look strange to me, aka
too broad for the name and the purpose.

I'm also wondering: is CombinedLCG worth it? I must admit I don't know when
I should use it instead of MT19937.

Since the names are all super opaque to many of us, documentation should be
very clear about the use case of each implementation... (if can reduce the
number of implementations, that's even better :) )

Cheers,
Nicolas

Re: [PHP-DEV] Add ReflectionFunctionAbstract::isAnonymous()

[Nicolas Grekas]

Le ven. 22 oct. 2021 à 15:41, Dylan K. Taylor  a écrit :

>
>  On Fri, 22 Oct 2021 14:19:23 +0100 Claude Pache <
> claude.pa...@gmail.com> wrote 
>
>  >
>  > Le 21 oct. 2021 à 01:12, Dylan K. Taylor  a écrit :
>  >
>  > Hi all,
>  >
>  > Given the addition of Closure::fromCallable() and the upcoming
> first-class callable syntax in 8.1, it seems slightly problematic that
> there's no simple way to tell by reflection if a Closure refers to an
> anonymous function or not. ReflectionFunctionAbstract::isClosure() (perhaps
> somewhat misleadingly) returns whether the closure is literally a \Closure
> instance, so it's not useful for this purpose.
>  >
>  > The only way to do this currently (that I know about) is to check if
> the name of the function contains "{closure}", which is a bit unpleasant
> and depends on undocumented behaviour.
>  >
>  > I'm proposing the addition of
> ReflectionFunctionAbstract::isAnonymous(), which would fill this use case,
> and may be able to offer an implementation.
>  >
>  > Thanks,
>  > Dylan Taylor.
>  >
>  >
>  > --
>  > PHP Internals - PHP Runtime Development Mailing List
>  > To unsubscribe, visit: https://www.php.net/unsub.php
>  >
>  >
>  >
>  > Hi,
>  >
>  > Per the manual [1], Closure::fromCallable() “creates and returns a new
> anonymous function”. I guess that this might not match your notion of
> “anonymous function”?
>  >
>  > Therefore, I am asking for clarification: What practical distinction do
> you make between ”an instance of Closure” and “an anonymous function”, and
> why does this distinction matter?
>  >
>  > [1]: https://www.php.net/manual/en/closure.fromcallable.php
>  >
>  > —Claude
>  >
>
> Hi Claude,
> Sorry for the double email, my previous reply got bounced from the mailing
> list because I replied from the wrong address.
>
> An anonymous function would be an unnamed function, e.g. arrow function or
> function(){}.
>
> I guess the documentation for Closure::fromCallable() ought to be updated,
> because Closure::fromCallable('namedFunc') isn't really anonymous, it's a
> \Closure object that refers to a named function. A \Closure may refer to a
> named or an anonymous function, and currently there's no way to tell the
> difference without hacks.
>
> The distinction is important for reflection cases, such as the one Aaron
> mentioned, and also like generating a pretty name for closures in a project
> I maintain [1].
>
> It's a fringe use case for sure, but considering we already have
> ReflectionClass->isAnonymous(), I think it makes sense.
>
> [1]:
> https://github.com/pmmp/PocketMine-MP/blob/986b4e0651d665c72ec011542f95b4bd9529c6a8/src/pocketmine/utils/Utils.php#L144
>
>
>

For the record, I submitted a PR adding ReflectionFunction::isAnonymous()
at
https://github.com/php/php-src/pull/8499

Cheers,
Nicolas

Re: [PHP-DEV] Add leading backslash to enum and class names in var_export

[Nicolas Grekas]

Le jeu. 31 mars 2022 à 17:50, Larry Garfield  a
écrit :

> On Thu, Mar 31, 2022, at 10:45 AM, Ilija Tovilo wrote:
> > Hi everyone
> >
> > We've had two bug reports for var_export not working for enums. The
> > reason for that is that var_export does not add a leading backslash to
> > names of enums. This will cause them to be resolved as
> > `\CurrentNamespace\EnumNamespace\EnumName` instead of just
> > `\EnumNamespace\EnumName`.
> >
> > enum A {
> > case B;
> > }
> >
> > echo var_export(A::B, true), "\n";
> >> A::B
> >
> >
> > This is a problem for things like Doctrines ProxyGenerator that embeds
> > the result of var_export in classes with namespaces
> > (https://github.com/doctrine/common/pull/955). The Doctrine team
> > resolved this by adding a `use` of the enum at the top of the file but
> > that really shouldn't be necessary.
> >
> > The same issue already exists for classes.
> >
> > class C {}
> >
> > echo var_export(new C(), true), "\n";
> >> C::__set_state(array(
> >> ))
> >
> > https://bugs.php.net/bug.php?id=64554
> >
> > Marco Pivetta created a PR that adds leading backslash to all class or
> > enum names of var_export. Adding a backslash will make the code work
> > both inside or outside namespaces.
> > https://github.com/php/php-src/pull/8233
> >
> > To avoid disruption, I'm proposing to merge this into PHP 8.2 only.
> > Unless anybody has any objections to this change I will merge it in a
> > few weeks.
> >
> > Ilija
>
> What would be the disruption on 8.1?  It wouldn't break the existing `use`
> trick, and I doubt anyone is leveraging this deliberately.
>
>
It could break fixtures that ppl might use to compare the two outputs of
var_export().

Re: [PHP-DEV] [RFC] [VOTE] Sealed Classes

[Nicolas Grekas]

> > to me this closes extensibility in a hard way.
>
> This is not necessarily true, we have `final` in PHP which does exactly
> that, but `sealed` can still allow for extensibility, just from a different
> point, e.g:
>

`final` has the same issue to me: it's a feature of the language that would
be better enforced by a static analyzer instead of a runtime check.

This would work for me:
#[Sealed(A::class, B::class, ...)]
interface Foo {...}
Then, nothing in php-core but a rule in SA tools to check that the
semantics of the attribute are respected (or a custom exception to the rule
when a user really wants to ignore the attribute for a specific class,
being on their own then).

Note that for `final` this is already common practice - using the `@final`
annotation to express the intent that a class is final but still allow e.g.
to generate a lazy proxy or a mock class (or any other need authors cannot
anticipate).



> Considering the `Option`/`Some`/`None` example above, given `Option`, now
> you are sure that it's either an instance of `Some` or `None`, where
> previously, a third type could exist.
>
> Authors previously got around this issue by adding methods on `Option`
> such as `isSome()`/`isNone()`/`isSuccess()`/`isFailure()` .. etc
>

Or "instanceof Option|Some|None", which achieves something very similar to
me. This relates to the paragraph about composite types in the RFC: most of
the problems you describe for them can be solved in practice by introducing
an abstract (@internal) class.

Please note that I'm making these criticisms in a constructive mindset. I
do appreciate that there is an RFC about sealed classes and that we can
discuss if and how we want them in PHP. Right now, I'm not convinced we
should add them and I'm trying to articulate why, that's all :)

Nicolas

Re: [PHP-DEV] [RFC] [VOTE] Sealed Classes

[Nicolas Grekas]

Le jeu. 17 mars 2022 à 04:54, Saif Eddin Gmati  a
écrit :

> Hello Internals,
>
> As per my last email in the previous thread, i have started the vote for
> sealed classes feature.
>
> The vote will run for 2 weeks until March 31st 2022.
>
> Discussion: https://externals.io/message/117173
>
> Draft Discussion: https://externals.io/message/114116
>
> RFC: https://wiki.php.net/rfc/sealed_classes
>

Hello Saif,

Thanks for the RFC.

I voted "no" because to me this closes extensibility in a hard way. If
users are fine ignoring an "@internal" annotation, or using reflection to
access private symbols, then I think that's fine: their problem; they know
why they need to do so - not authors. Allowing authors to forcibly remove
that capability from users is going too deep into removing power from users.

Said another way, I don't think this solves any problem that authors face
in practice. As such I don't think this is worth the added language
complexity + removal of power.

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] Undefined Variable Error Promotion

[Nicolas Grekas]

Le mer. 23 févr. 2022 à 17:59, Christoph M. Becker  a
écrit :

> On 23.02.2022 at 16:29, Nicolas Grekas wrote:
>
> > Le mar. 22 févr. 2022 à 14:56, Marco Pivetta  a
> écrit :
> >
> >> On Tue, Feb 22, 2022 at 2:53 PM Nicolas Grekas <
> >> nicolas.grekas+...@gmail.com> wrote:
> >>
> >>> But this makes me think: we should trigger a deprecation just before
> all
> >>> corresponding warnings!
> >>
> >> Please, no more deprecation warnings, make it stop 
> >> Yes, undefined variables are a problem, but I just spent 6 months in
> >> `vendor/` code for 8.0->8.1 stuff, and it doesn't bring anything but
> >> frustration: this stuff is statically introspectible, and even more
> >> side-effects are just more trouble.
> >
> > I'm not going to be affected by this RFC at all, and neither are you,
> since
> > we use throwing error handlers. But ppl that do rely on code bases that
> > have undefined vars "by design" will be. I would bet that the number of
> ppl
> > in that affected group and that also use a static analyser is very very
> > small. This means that static analysers are not a pragmatic solution
> here.
>
> That.
>
> > Ppl that don't use static analysers deserve a prior notice. There is a
> > dedicated reporting mechanism in place and we should use it IMHO. With
> new
> > deprecations added to PHP 8.1, the ecosystem realized that the tooling
> > needed to improve - and it did (phpunit, Laravel, etc.). We can and
> should
> > add new runtime deprecations when planning a BC break.
> >
> > Please consider adding this deprecation Mark (and others.)
>
> Do you mean E_DEPRECATED in addition to E_WARNING, or instead?  I
> wouldn't be happy either way.
>

I mean in addition yes, deprecation before warning.

I considered grouping them as E_DEPRECATED|E_WARNING but that'd break many
userland error handlers I fear.

Re: [PHP-DEV] [RFC] Undefined Variable Error Promotion

[Nicolas Grekas]

Le mar. 22 févr. 2022 à 14:56, Marco Pivetta  a écrit :

> On Tue, Feb 22, 2022 at 2:53 PM Nicolas Grekas <
> nicolas.grekas+...@gmail.com> wrote:
>
>>
>> But this makes me think: we should trigger a deprecation just before all
>> corresponding warnings!
>>
>
> Please, no more deprecation warnings, make it stop 
> Yes, undefined variables are a problem, but I just spent 6 months in
> `vendor/` code for 8.0->8.1 stuff, and it doesn't bring anything but
> frustration: this stuff is statically introspectible, and even more
> side-effects are just more trouble.
>

I'm not going to be affected by this RFC at all, and neither are you, since
we use throwing error handlers. But ppl that do rely on code bases that
have undefined vars "by design" will be. I would bet that the number of ppl
in that affected group and that also use a static analyser is very very
small. This means that static analysers are not a pragmatic solution here.

Ppl that don't use static analysers deserve a prior notice. There is a
dedicated reporting mechanism in place and we should use it IMHO. With new
deprecations added to PHP 8.1, the ecosystem realized that the tooling
needed to improve - and it did (phpunit, Laravel, etc.). We can and should
add new runtime deprecations when planning a BC break.

Please consider adding this deprecation Mark (and others.)

Nicolas

>

Re: [PHP-DEV] [RFC] Undefined Variable Error Promotion

[Nicolas Grekas]

Le mar. 22 févr. 2022 à 12:07, Mark Randall  a écrit :

> On 22/02/2022 09:15, Nicolas Grekas wrote:
> > I very much call for an implementation to be provided before starting any
> > vote on the topic btw.
> The RFC states on its first line that accessing an undefined variable
> emits an E_WARNING. If it does not currently emit an E_WARNING then it
> is out of scope of this RFC.
>

This last sentence could be worth copy/pasting into the RFC :)
Thanks for the recent edits, they help clarify to me.


> It is not sensible to provide a full implementation for something that
> will require extensive engine and test changes for something several
> years into the future, during which many other changes will be made.
>

> A sensible time for this would be after the branch is cut for 8.4.
>

I understand if the target is to do everything in 9.0.
But this makes me think: we should trigger a deprecation just before all
corresponding warnings!
This would signal the change to 8.x users and help them spot where a change
is needed.
This should happen before the warning to that error handlers that turn the
warning into an exception also get a note about the soon-to-be change.

WDYT?

Nicolas

Re: [PHP-DEV] [RFC] Undefined Variable Error Promotion

[Nicolas Grekas]

Le ven. 18 févr. 2022 à 12:24, Rowan Tommins  a
écrit :

> On 17/02/2022 23:28, Mark Randall wrote:
> > I present:
> >
> > https://wiki.php.net/rfc/undefined_variable_error_promotion
>
>
> It would be good to have a "Scope" or "Unaffected Functionality" section
> here, because there are a number of closely related things which were
> also raised from Notice to Warning in 8.0:
>
> - undefined array keys
> - undefined object properties
> - array access on a non-array
> - property access on a non-object
>
> I think it is sensible to discuss those separately, but it would be good
> to make that clear.
>
>
> Similarly, it would be good to have more discussion of what "accessing"
> means, as the current examples are quite narrow, only showing direct use
> and the ++ operator. Other functionality potentially affected:
>
> - passing the variable to a function, presumably excluding by-reference
> parameters which don't currently warn
> - all the combined assignment operators -
> https://www.php.net/manual/en/language.operators.assignment.php
> - the array append operator ($a[] = 42;) does NOT currently give an
> undefined variable Warning
> - variable variables, e.g. "$b = 'a'; echo $$b;"
> - the compact() pseudo-function
>
> There's probably others that I've missed.
>

I 100% agree with that. An "Unaffected Functionality" section would be much
welcome.

I would add to the list: isset($foo) when $foo is first seen.
(To me this should not throw anything, like for uninitialized properties.)

If the RFC is about promoting the current warnings to Error without adding
any other Error at places that currently don't throw any warnings, then it
could be useful to say so. And if the RFC is going to propose to throw
Error at places that are currently warning-less, the list should be
explicit.

I very much call for an implementation to be provided before starting any
vote on the topic btw.
This is typically the kind of topic where getting into the actual code
might help spot edge cases.

Thanks for the RFC btw!

Nicolas

Re: [PHP-DEV] [VOTE] Locale-independent case conversion

[Nicolas Grekas]

Le jeu. 25 nov. 2021 à 12:23, Tim Starling  a
écrit :

> On 25/11/21 8:58 pm, Nicolas Grekas wrote:
>
>
> The RFC says:
> > because they also use isdigit() and isspace(),
>
> Does that mean "too much work needed"? I would totally understand that of
> course but I hope someone could do these last miles.
>
> Yes.
>
> > and because they are intended for natural language processing
>
> I definitely do not agree with this argument and it should be removed from
> the RFC to me as it might add confusion in the future.
>
> Done.
>

Great, thanks!

Re: [PHP-DEV] [VOTE] Locale-independent case conversion

[Nicolas Grekas]

Le jeu. 25 nov. 2021 à 11:34, Christoph M. Becker  a
écrit :

> On 25.11.2021 at 10:58, Nicolas Grekas wrote:
>
> > Le jeu. 25 nov. 2021 à 10:47, Tim Starling  a
> > écrit :
> >
> >> and because they are intended for natural language processing
> >
> > I definitely do not agree with this argument and it should be removed
> from
> > the RFC to me as it might add confusion in the future.
>
> Yeah, the PHP manual says[1]:
>
> | This function implements a comparison algorithm that orders
> | alphanumeric strings in the way a human being would, this is described
> | as a "natural ordering".
>
> [1] <https://www.php.net/manual/en/function.strnatcmp.php>
>

Yep, yet "natural language processing" means processing sentences we write
as humans, e.g. processing this very message. natcase sorting functions are
not done for that. They're useful to sort items in a list - typically file
names - in a way that makes sense to humans. This is very different from
"natural language processing". Having "natsort" vary by locale doesn't make
more sense than having "sort()" vary by locale. That's my point. The
argument doesn't stand against implementing locale-insensitivity for these
functions and I think the RFC shouldn't make it (the argument.)

Nicolas

Re: [PHP-DEV] [VOTE] Locale-independent case conversion

[Nicolas Grekas]

Le jeu. 25 nov. 2021 à 10:47, Tim Starling  a
écrit :

> On 25/11/21 7:55 pm, Nicolas Grekas wrote:
>
>
> I voted yes because I want to see this happen but I raised a point in
> https://externals.io/message/116141#116259 and didn't get an answer:
>
> Despite their name, I never used natcase functions for natural language
>> processing. I use them eg to sort lists of files in a directory, to
>> account
>> for numbers mainly. But that's not what I would call natural language
>> processing. I'm not aware of anyone using them for that actually. I'm
>> wondering if it's a good idea to postpone migrating them to an
>> hypothetical
>> future as to me, the whole reasoning of the RFC applies to them.
>>
>
> I wish the strnatcasecmp() and natcasesort() function, but also the
> SORT_NATURAL flag were also covered by this RFC.
> Is that possible? Would it make sense?
>
>
> I'm not going to migrate those functions at this time. It's just a project
> scope decision.
>

Why?

The RFC says:
> because they also use isdigit() and isspace(),

Does that mean "too much work needed"? I would totally understand that of
course but I hope someone could do these last miles.

> and because they are intended for natural language processing

I definitely do not agree with this argument and it should be removed from
the RFC to me as it might add confusion in the future.

Nicolas

Re: [PHP-DEV] [VOTE] Locale-independent case conversion

[Nicolas Grekas]

Le jeu. 25 nov. 2021 à 06:05, Tim Starling  a
écrit :

> Voting is now open for my RFC on locale-independent case conversion.
>
> https://wiki.php.net/rfc/strtolower-ascii
>
> Voting will close in two weeks, on 2021-12-09.
>

Hi Tim,

I voted yes because I want to see this happen but I raised a point in
https://externals.io/message/116141#116259 and didn't get an answer:

Despite their name, I never used natcase functions for natural language
> processing. I use them eg to sort lists of files in a directory, to account
> for numbers mainly. But that's not what I would call natural language
> processing. I'm not aware of anyone using them for that actually. I'm
> wondering if it's a good idea to postpone migrating them to an hypothetical
> future as to me, the whole reasoning of the RFC applies to them.
>

I wish the strnatcasecmp() and natcasesort() function, but also the
SORT_NATURAL flag were also covered by this RFC.
Is that possible? Would it make sense?

Nicolas

Re: [PHP-DEV] [RFC] Deprecate dynamic properties

[Nicolas Grekas]

Hi Nikita, hi everybody,

Le mer. 25 août 2021 à 12:03, Nikita Popov  a écrit :

> Hi internals,
>
> I'd like to propose the deprecation of "dynamic properties", that is
> properties that have not been declared in the class (stdClass and
> __get/__set excluded, of course):
>
> https://wiki.php.net/rfc/deprecate_dynamic_properties
>
> This has been discussed in various forms in the past, e.g. in
> https://wiki.php.net/rfc/locked-classes as a class modifier and
> https://wiki.php.net/rfc/namespace_scoped_declares /
>
> https://github.com/nikic/php-rfcs/blob/language-evolution/rfcs/-language-evolution.md
> as a declare directive.
>
> This RFC takes the more direct route of deprecating this functionality
> entirely. I expect that this will have relatively little impact on modern
> code (e.g. in Symfony I could fix the vast majority of deprecation warnings
> with a three-line diff), but may have a big impact on legacy code that
> doesn't declare properties at all.
>

Thanks for the RFC, it makes sense to me and I support the move.

Since Symfony is mentioned in the RFC, I thought you might want to know
about this PR, that removes dynamic properties from the Symfony codebase:
https://github.com/symfony/symfony/pull/44037/files

What Nikita describes in the RFC is correct: declaring+unsetting the
"groups" property works.
There's just one more thing I had to do; I also had to replace two calls to
property_exists:

-if (!property_exists($this, 'groups')) {
+if (!isset(((array) $this)['groups'])) {

The rest are test cases where we've been lazily accepting fixtures with
undeclared properties. No big deal, and I'm happy the engine might soon
help us get a bit stricter in this regard.

I read that some think that this PR is not required because static
analysers (SA) can report when a dynamic property is used. Although that's
correct, I think it would be detrimental to PHP as a language if SA tools
(or any tools actually) were a requirement to code in the safe-n-modern
way. You should not have to install any complex safeguarding tooling
infrastructure to start coding; both for newcomers, but also for
no-so-new-comers.

About the discussion related to deprecations. I've yet to see a better
reporting system than the current one.
It's true that too many userland error handlers are throwing instead of
collecting/logging/skipping deprecations.
But these can be fixed (and many are being fixed these days, which is nice!)

Cheers,
Nicolas

Re: [PHP-DEV] [RFC] Locale-independent case conversion

[Nicolas Grekas]

Le lun. 11 oct. 2021 à 03:33, Tim Starling  a
écrit :

> On 4/10/21 9:08 pm, Nikita Popov wrote:
> >
> > Hi Tim,
> >
> > Thanks for creating this proposal, it looks great!
> >
> > I think this is a very beneficial change, and the amount of
> > incorrect locale-dependent calls we had just in php-src further
> > convinced me of this: We're generally aware of the problem, and we
> > still made this mistake. Many times.
> >
> > The only open question I have is regarding the ctype_* functions.
> > One might argue that these functions should be locale-independent as
> > well. Certainly, whenever I have used ctype_digit() I only intended
> > it to match [0-9]. It seems like some people try to use
> > ctype_alpha() in a locale-sensitive way
> > (
> https://stackoverflow.com/questions/19929965/php-setlocale-not-working-for-ctype-alpha-check
> > <
> https://stackoverflow.com/questions/19929965/php-setlocale-not-working-for-ctype-alpha-check
> >)
> > and then fail because it doesn't support UTF-8.
> >
> OK, I removed ctype_tolower() and ctype_toupper() from the RFC and the
> PR since they would be incompatible with a move towards a
> locale-independent ctype extension.
>
> The non-controversial parts of the PR were split and merged, so I
> rebased the PR and updated the RFC accordingly.
>
> Do you think the RFC is ready for voting now?
>
>
> > PS: Regarding escapeshellarg(), are you aware of the array command
> > support for proc_open() that was added in PHP 7.4? That does away
> > the need to escape arguments.
>
> It doesn't really help us. I recently wrote a new shell command
> execution system for MediaWiki called Shellbox. As part of that
> project, I reviewed how shell execution is used in the MediaWiki
> ecosystem. There are a lot of callers which are using shell features,
> for example redirecting inputs or outputs, or constructing pipelines.
> I didn't really want to break them all or reimplement those features
> without the shell. And we have security and containerization wrappers
> which depend on construction of a shell command string. So we need to
> be able to construct shell command strings safely.
>
> After studying locale sensitivity for this RFC, I decided to get rid
> of escapeshellarg() from MediaWiki. Instead we are doing our own shell
> escaping:
>
> https://gerrit.wikimedia.org/r/c/mediawiki/libs/Shellbox/+/722548
>
> I also made MediaWiki use a fixed locale, instead of being configurable.
>

Hi Tim,

thanks for the RFC and for the above pointers, I'm going to have a look at
Symfony Process to follow your lead!

About the RFC, I just have one note:

> I didn't include strnatcasecmp() and natcasesort() in this RFC, because
they also use isdigit() and isspace(), and because they are intended for
natural language processing. They could be migrated in future.

Despite their name, I never used *natcase* functions for natural language
processing. I use them eg to sort lists of files in a directory, to account
for numbers mainly. But that's not what I would call natural language
processing. I'm not aware of anyone using them for that actually. I'm
wondering if it's a good idea to postpone migrating them to an hypothetical
future as to me, the whole reasoning of the RFC applies to them.

Regards,
Nicolas

Re: [PHP-DEV] BC breaking changes in PHP 8.1

[Nicolas Grekas]

Le dim. 26 sept. 2021 à 14:42, Jordi Boggiano  a écrit :

>
> > I'm surprised that is_resource() returns false for resource objects,
> > does anyone knows why it wouldn't return true in such case ?
> >
> > This is a very weird behavior, I'd expect it to return true, moreover
> > this is the most annoying detail of this BC break, in my opinion.
> >
> > I have lots of code using is_resource() but I usually don't bother
> > checking for type (maybe I'm doing it wrong). But I'd also expect
> > get_resource_type() to be deprecated somehow and return
> > backward-compatible values for converted resources to objects.
>
> Right, having is_resource return true for resourcey objects would have
> prevented all resource-related errors I ran into with php8.0 migration I
> believe. That'd be a welcome improvement/fix for 8.1.
>

I'm not sure about this one: is_resource($foo) ? get_resource_type() is
pretty common. Also this would break some expectations with gettype($foo)

Nicolas

>

Re: [PHP-DEV] [RFC] $this return type

[Nicolas Grekas]

Hi Nikita

Hi internals,
>
> I'd like to pick up a loose thread from the future scope of the
> https://wiki.php.net/rfc/static_return_type RFC, the $this return type for
> fluent APIs:
>
> https://wiki.php.net/rfc/this_return_type
>
> I have some reservations about this (which basically come down to $this not
> being a proper "type", so should it be in the type system?) but I can see
> the practical usefulness, so I think it's worth discussing this.
>

Thank you so much for the RFC. I'm really looking forward to being able to
use $this as a return type.

> On a dataset of 2k composer packages, @return $this occurs 29k times,
while @return static is used 38k times

This is really good to know. It makes it pretty clear to me that being able
to declare $this as a return value would be valuable to many projects!

As far as my experience is concerned, I tried switching from the `@return
$this` annotation to `function (): static`, which is the closest
approximation currently supported by the engine, when I realized that this
would loosen the semantics of the corresponding interfaces and kinda force
ppl to deal with the return value while this doesn't make sense in
fluent-style APIs. I therefore kept the annotation in place and I'm now
looking forward to being able to write this declaration using native syntax
- with the added benefit of having it enforced by the engine.

About the syntax, I think the one proposed in the RFC is crystal clear.
I've been used to seeing the "$" symbol when reading `@return $this`, and
this just makes sense, past the first surprise. I also think that using
"this" (without the dollar) could too easily be confused with something
that means our "static" (esp. considering that TypeScript uses "this" in
place of our "static".)

I'm reading in responses that $this is not a type, but I fail to understand
how that makes it an argument against the RFC. Neither false is a type, and
null is both a type and a value. Yet we have them because they provide
clear added value. Having $this could benefit 29k times the top 2k
packages. It doesn't look like a niche use case!

As you might have understood, I support the RFC and would vote for it as is
personally!

Nicolas

Re: [PHP-DEV] Alias stdClass to DynamicObject?

[Nicolas Grekas]

> In the thread for deprecation of dynamic properties, Rowan suggested that
> we alias "stdClass" to "DynamicObject" in
> https://externals.io/message/115800#115802. I wanted to split this
> discussion off into a separate thread, as this can be decided
> independently.
>
> The rationale for this is that "stdClass" is something of a misnomer: The
> name makes it sound like this is a common base class, as used in a number
> of other languages. However, PHP does not have a common base class. The
> only way in which "stdClass" is "standard" is that it is the return type of
> casting an array to (object).
>
> The actual role of stdClass is to serve as a container for dynamic
> properties, thus the suggested DynamicObject name.
>
> What do people think about adding such an alias? Is this worthwhile?
>

Hi Nikita, Rowan,

I'm reading the discussion about the side of the alias. Can't we solve
these concerns by making DynamicObject extend stdClass instead of aliasing?
That wouldn't allow an stdClass object to get through the DynamicObject
typehint, but that shouldn't be an issue since no such code has yet been
written, isn't it?

Nicolas

[PHP-DEV] Re: [VOTE] Nullable intersection types

[Nicolas Grekas]

Hi everyone,

I'm happy to announce that the vote for nullable intersection types is now
> open:
> https://wiki.php.net/rfc/nullable_intersection_types
>
> It'll close in two weeks, on the 27th.
>

The vote is now closed with a total of 38 votes: 26 against and 12 in favor.

The RFC is declined.

Nicolas

Re: [PHP-DEV] Guidelines for RFC post feature-freeze

[Nicolas Grekas]

Le mer. 25 août 2021 à 19:32, Marco Pivetta  a écrit :

> Hey Nicolas,
>
>
> On Wed, Aug 25, 2021 at 7:30 PM Nicolas Grekas <
> nicolas.grekas+...@gmail.com> wrote:
>
>> I would welcome a new RFC to clarify what is allowed during the feature
>> freeze.
>>
>
> See https://en.wikipedia.org/wiki/Freeze_(software_engineering)
>

Thank you Marco,

Can you please let me know how that helps?

Re: [PHP-DEV] Guidelines for RFC post feature-freeze

[Nicolas Grekas]

Le mar. 24 août 2021 à 08:09, Pierre Joye  a écrit :

> Hi Marco,
>
> On Tue, Aug 24, 2021 at 3:49 AM Deleu  wrote:
> >
> > Hello everyone!
> >
> > We recently had the Nullable Intersection Types RFC process in an
> > unconventional way starting a new RFC post feature freeze. If memory
> serves
> > me right, another similar incident happened with the Attributes RFC which
> > had a syntax that could not be implemented without a secondary RFC [1]
> and
> > went through a secondary RFC which proposed a different syntax [2].
> >
> > [1] https://wiki.php.net/rfc/namespaced_names_as_token
> > [2] https://wiki.php.net/rfc/attributes_v2
> >
> > I would like to gather opinion on a potential Policy RFC that would
> define
> > some guidelines for such a process. As Nikita pointed out [3], the
> ability
> > to refine new features is both important for the developer and
> undocumented
> > for the PHP Project.
> >
> > In order to not be empty-handed, I started a gist that can be seen as the
> > starting point for this discussion, available at
> > https://gist.github.com/deleugpn/9d0e285f13f0b4fdcfc1d650b20c3105.
> >
> > Generally speaking, I'm first looking for feedback on whether this is
> > something that deserves attention and an RFC or is it so rare that it's
> > fine to leave it unchanged. If there is interest in moving forward, I
> would
> > then also be interested in suggestions on things that should be
> > included/excluded in the RFC.
>
> It is a very good text, thank you!
>
> It is also much needed, generally speaking. What I would add is about
> what is allowed to begin with. I would rather restrict to fixes only.
>
> The other issue, which is the one Nicolas suffered from, incomplete
> addition to begin with. Incomplete in the sense of, "We add feature A,
> but behaviors A1 and A2 are not supported and can be done later".
>
> Many additions went through while being incomplete. It was documented
> so in the RFC but it does not make it a good thing. Many of them are
> indeed much needed and related to features (some) PHP users have been
> waiting for. Are they critical enough for the PHP usage to allow them
> in while knowing it is not complete? For almost all recent RFCS
> related to syntax, arguments/return types or properties, I don't think
> it justifies being added while being incomplete. It is not critical
> enough to the larger user base. It makes migration paths harder as
> well.
>
> A library or framework (main users of most of these features) may or
> may not implement the given addition, requiring say 8.1, and yet again
> require 8.2 and redo the implementation to support (hopefully) the
> full features.
>
> This is a path I dislike, I may have a different view on the big
> picture, however I do think we rushed too many of these features too
> early. A vote does not solve this problem given the limited amount of
> votes we can see.
>

Thanks for writing this Pierre, I wholeheartedly agree with this. This was
the fundamental trigger to my RFC: trying to make intersection types closer
to general usefulness *in*my*opinion*! (I don't want to reopen the topic :)
)

I would welcome a new RFC to clarify what is allowed during the feature
freeze. As I wrote in another thread, my opinion is that anything that is
not yet released should be re-discussable until either beta or RC stage, at
least for simple changes (I wouldn't have submitted my RFC if the patch
wasn't trivial from a technical pov.)

WIth the current feature freeze schedule, I realize that there is little to
no room for userland to play with a feature-full binary before it's too
late to give feedback. I experienced this when I was objected that the RFC
was 4 months old already. I can't keep up with testing all RFCs. But if
there is a clear window where such feedback is welcomed, I would happily
use it. I think others would too.

Nicolas

Re: [PHP-DEV] Guidelines for RFC post feature-freeze

[Nicolas Grekas]

Le mar. 24 août 2021 à 21:09, Derick Rethans  a écrit :

> On 24 August 2021 19:53:57 BST, Deleu  wrote:
> >On Tue, Aug 24, 2021, 19:28 Derick Rethans  wrote:
> >
> >> On Mon, 23 Aug 2021, Deleu wrote:
> >>
> >> > We recently had the Nullable Intersection Types RFC process in an
> >> > unconventional way starting a new RFC post feature freeze. If memory
> >> > serves me right, another similar incident happened with the Attributes
> >> > RFC which had a syntax that could not be implemented without a
> >> > secondary RFC [1] and went through a secondary RFC which proposed a
> >> > different syntax [2].
> >>
> >> I find this comparison disingenuous.
> >>
> >
> >I want to state that I had no intention to compare the RFCs or even bring
> >their merits into discussion. What I intended to show is that we have 8.0
> >which had an RFC that would classify as Refinement RFC and 8.1 again
> having
> >another RFC that also classifies under the same category.
>
> That's where I disagree already. The nullable intersections RFC isn't a
> refinement, it's a new feature.
>

Hello Derick,

Can you please clarify what you want to express here? Your insistence in
repeating that statement makes me read this as: "the nullable intersections
RFC was not legal". If that's the case, I find that deeply disturbing,
because I need to be allowed to discuss not-yet-released features during
the freeze period. Whether an RFC should be considered as a new feature
should be the end of the discussion, not the abruptly-closing start. The
reason is that there is no precise definition of what "a feature" means.
Maybe it's obvious for you in this case, but others shouldn't be denied the
right to discuss the topic.

I think that we can reach a common agreement by working on the definition
of what we mean by "Refinement RFC".

Marco's gist defines them as "An RFC proposing changes, amendments,
adjustments to the language while refining an unreleased change that has
been approved." I'm sure we can improve it, but I mostly agree with this
statement. My RFC falls under this definition, and that should be enough to
end the debate around whether any particular post-feat-freeze RFCs are
legal. I think we should focus our efforts on improving this definition,
and move forward.

Nicolas

Re: [PHP-DEV] [VOTE] Nullable intersection types

[Nicolas Grekas]

Le lun. 23 août 2021 à 16:22, Larry Garfield  a
écrit :

> On Sun, Aug 22, 2021, at 5:42 PM, Patrick ALLAERT wrote:
>
> > Either extra time, or having a way to influence the schedule of the
> > releases.
> > For now, we work with a fixed schedule and don't know (at least: me) how
> > strict we must stick to it.
> >
> > The fixed schedule of the releases is what makes me more comfortable with
> > the idea of reverting a new feature rather than extending the scope of
> one.
>
> Speaking from the outside:
>
> Joe stated in an earlier email very clearly that because this was viewed
> as a "possible bug fix" it was given permission to run post-freeze.  There
> was no dispute in my mind at least whether the RFC was valid.
>
> That said, if voters are of the opinion that "the cure is worse than the
> disease" on this one (eg, because it didn't take into account larger
> questions around compound types that will have to come later, and so may
> make things more difficult in the future, or because the reflection API
> isn't fully thought through, etc.), or that it feels too rushed, that is
> entirely their right to vote no on it on those grounds.  That's basically a
> summary of my No vote: I'd rather have non-nullable intersections for now
> than something that is going to make life more difficult in the future for
> full mixed types.
>
>
> > > Based on the partial results of the vote, we could conclude that the
> > > consensus is to 1. not allow nullability and 2. force bracket around
> > > (X)|null. I can't read this as anything else than ppl voting not on
> the
> > > RFC, but on this metadiscussion about the feature freeze + some fear
> that
> > > we might put ourselves in a corner with the syntax.
> > >
> > > The RFC should be allowed to complete, it's gathering important data.
> > >>
> > >
> > > Because of what I just wrote about, I don't think we can rely on any
> data
> > > here. The discussion should be rebooted from scratch for me.
> Apparently, we
> > > need the full syntax for composite types to decide for the syntax for
> > > nullable intersection types. ¯\_(ツ)_/¯
> > >
> >
> > I also think that we can't rely on the data for the reason you mention
> even
> > if I intended to vote "no" on the feature, not on the targeted version of
> > PHP, I may be the exception there.
>
> That's not different than any other RFC.  We never differentiate "no on
> the concept" from "no on the implementation" from "no on some particular
> detail that's really really important."  That's data we almost never get,
> unless someone volunteers it on their own.  This RFC is no different in
> that regard.
>
> > > Now, I don't know what to do. The vote started and is not looking good
> for
> > > the proposal. Some will say that I didn't want to see it fail if I
> withdraw
> > > it. But honestly, with all those interferences, I'm not sure the
> conditions
> > > are met to have a fair vote.
>
> I have to disagree.  Given Joe's earlier message, the RFC was legal and
> allowed to proceed.  "I still think it's too late" is entirely someone's
> right to justify a No vote.  That's not interference, that's the voter's
> viewpoint and it should be respected.
>

No need to disagree as I was referring to exactly that: the legality of the
vote. This legality has been deeply challenged during the discussion period
and during the vote itself. That's what I'm referring to as interference.
Once the legality is shielded, of course ppl are free to vote what they
want. That wasn't the case at all for this RFC. I wish we'll find a way to
not experience this confusion anymore in the future.

Nicolas

Re: [PHP-DEV] [VOTE] Nullable intersection types

[Nicolas Grekas]

>
> On Mon, 16 Aug 2021 at 10:04, Nikita Popov  wrote:
>
>> On Mon, Aug 16, 2021 at 9:45 AM Joe Watkins  wrote:
>>
>>> Morning all,
>>>
>>> The initial RFC was clear that nullability was not supported, however
>>> that
>>> doesn't seem to be have widely understood.
>>>
>>> When I said we should move forward I did imagine that there was some
>>> consensus about the syntax we should use if we were to support
>>> nullability.
>>>
>>
>> Based on the vote, it looks like there's a pretty clear consensus on
>> (X)|null :)
>>
>>
>>> As this conversation has progressed it has become clear that we don't
>>> have
>>> that consensus, and many people are just not comfortable trying to build
>>> consensus this late in the cycle.
>>>
>>> The RFC is not passing currently so I don't think we actually need to do
>>> anything, except prepare to deploy the feature that was voted in, pure
>>> intersections.
>>>
>>> The RFC should be allowed to complete, it's gathering important data.
>>>
>>> In the end, I'm not as happy to make an exception as I was when the
>>> discussion started.
>>
>>
>> FWIW I think that if we granted an exception for this once, we shouldn't
>> go back on it. Maybe there should have been some discussion among RMs about
>> this, but I think your agreement was interpreted (at least by me and
>> presumably Nicolas) as it being fine to go forward with this RFC from a
>> release management perspective. Now that the vote is underway, people can
>> take the fact that this is targeting 8.1 into consideration in their choice
>> -- I suspect that a lot of the "no" votes here are specifically due to
>> that, not because they generally dislike support for nullable intersection
>> types.
>>
>> As a meta note, I think it's important that we're open to minor changes
>> to new features during the pre-release phase -- it's purpose is not just
>> implementation stability. In fact, we can fix implementation bugs anytime,
>> but usually can't do the same with design issues. (Of course, in this
>> particular case the proposed change is backwards-compatible, so there is no
>> strict requirement to make a change before the stable release.)
>>
>> Regards,
>> Nikita
>>
>
Morning Nikita, all,
>
> The exception was granted to ratify consensus that I thought we had - and
> that one of the two primary votes on the current RFC seems to support.
>
> However, the RFC we got was something that contained multiple primary
> votes - we must consider the first two votes primary, we don't want to
> choose syntax on a 50% majority.
>
> At the moment it looks like we don't have a consensus that adding
> nullability (at this late stage) is important.
>
> I too think it's important to be flexible in this stage of the cycle, but
> I look at the opposition to making this change now and that degrades my
> confidence that making the change this late even if it does pass is a good
> idea.
>
> Cheers
> Joe
>

What a mess! I feel so disappointed by the lack of clarity of the processes
here.

The RFC was granted a "go" by a release manager, but suddenly, in the
middle of the vote, another release manager raised an objection to this
grant!? This should have been discussed before between release managers.
Also, this "go" is now apparently revoked based on the partial results of
the vote. This is obviously interfering with the vote itself! I'm not
blaming any individuals, but some processes should clarify how this is
supposed to work. At least please coordinate together on such topics! 

The processes also lack clarity about what the feature freeze is supposed
to mean. We're having a metadiscussion about whether this RFC can target
8.1 or not, and many ppl stated that they would vote against it not because
they don't want nullability, but because they don't want it in 8.1, or
because they feel it's too late for 8.1. I can't blame them for that, but
what is "feature freeze" supposed to mean if we aren't allowed to discuss,
alter, or even revert not-yet-released features?!? This should be shielded
by some consensus on what is allowed during the feature freeze. On my side:
anything that is not yet released should be re-discussable until either
beta or RC stage. If we choose to go with beta, we should give more time
between the start of the feature freeze and the first beta, precisely to
fix what happened to this RFC, aka to give a window for the community (me
here) to play with the soon-to-be features and give feedback.

Based on the partial results of the vote, we could conclude that the
consensus is to 1. not allow nullability and 2. force bracket around
(X)|null. I can't read this as anything else than ppl voting not on the
RFC, but on this metadiscussion about the feature freeze + some fear that
we might put ourselves in a corner with the syntax.

The RFC should be allowed to complete, it's gathering important data.
>

Because of what I just wrote about, I don't think we can rely on any data
here. The discussion should be rebooted from scratch for me. 

[PHP-DEV] [VOTE] Nullable intersection types

[Nicolas Grekas]

Hi everyone,

I'm happy to announce that the vote for nullable intersection types is now
open:
https://wiki.php.net/rfc/nullable_intersection_types

It'll close in two weeks, on the 27th.

Cheers,
Nicolas

Re: [PHP-DEV] Re: [RFC] Nullable intersection types

[Nicolas Grekas]

> On 11/08/2021 13:09, Nicolas Grekas wrote:
> >
> > On 10/08/2021 13:39, Nicolas Grekas wrote:
> > > I will wait if I don't have the choice, but as many others
> > reported, the
> > > experience with 7.0 missing nullability was a pain.
> >
> > Apologies if you already did and I've forgotten, but could you please
> > expand on what "pain" you are referring to here?
> >
> >
> > I personally did not experience that pain because I just skipped 7.0,
> > since it wouldn't allow expressing the nullable return types I had to
> > express in the APIs I maintain.
>
>
> Sorry, I'm still confused what the "pain" is *other than* feeling
> obliged to wait until 7.1.
>

Maybe others that read this can share their experience on the topic?


> The larger issue is that when used as a type on arguments, adding the
> > nullability flag isn't possible without a BC breaking change.
>
>
> I can't imagine many people will force a parameter to be non-nullable
> just to use the shiny new syntax, then re-allow nulls in a subsequent
> version. More likely, they will leave that parameter un-checked until
> the full type can be specified.
>
> There is also a lot of code out there which is either:
>
> * in stand-alone applications, so backwards compatibility has no meaning
> * in private libraries with a handful of uses, so bringing uses in line
> is trivial
> * in public libraries, but marked "private" or "final", or documented as
> "internal use only", and therefore not subject to compatibility guarantees
>
>
> > But until it's too late, I'm willing to engage in this topic because I
> > think the current state is far from ideal for 8.1.
>
>
> There are always more features we could add, and there will always be a
> judgement call of what versions of PHP a code base should support.
>
> If it was generally agreed that there was only one right way to
> implement nullable intersections, and it was a trivial change, then I'd
> support it as a "quick win"; but that doesn't seem to be the case.
>

It is a trivial change from a technical pov, that's why I submit it for
8.1. There's zero risk.

Thanks for your feedback btw!

Nicolas

[PHP-DEV] Re: [RFC] Nullable intersection types

[Nicolas Grekas]

>
> Hi everyone,
>
> as proposed by Nikita and Joe, I'm submitting this late RFC for your
> consideration for inclusion in PHP 8.1. Intersection types as currently
> accepted are not nullable. This RFC proposes to make them so.
>
> I wrote everything down about the reasons why here:
> https://wiki.php.net/rfc/nullable_intersection_types
>
> Please have a look and let me know what you think.
>


Hi everyone,

I think it's time to move on. I'm going to open the vote tomorrow.
I'd still be happy to get more feedback before or during the vote!

Nicolas

Re: [PHP-DEV] Re: [RFC] Nullable intersection types

[Nicolas Grekas]

> On 10/08/2021 13:39, Nicolas Grekas wrote:
> > I will wait if I don't have the choice, but as many others reported, the
> > experience with 7.0 missing nullability was a pain.
>
> Apologies if you already did and I've forgotten, but could you please
> expand on what "pain" you are referring to here?
>

I personally did not experience that pain because I just skipped 7.0, since
it wouldn't allow expressing the nullable return types I had to express in
the APIs I maintain.

Firstly, I'm guessing we're talking about return types here, since
> parameters have had types since 5.0, and nullable types since 5.1 with
> the "TypeName $foo = null" syntax?
>

That's correct. And that made me realize I missed highlighting that the
situation with intersection types is actually worse than the one we had in
7.0 vs 7.1. As of https://github.com/php/php-src/pull/7254, not only return
types, but also *arguments* are affected by the non-nullable limitation. If
the RFC doesn't pass, I would still be in favor of reverting that part of
the PR. That would bring us a situation very similar to 7.0 vs 7.1, which
was not ideal but still better.


> Secondly, do you mean you postponed your adoption of the feature, or was
> there some larger issue?
>

I postponed it (and others did, as reported by Benjamin and Tobias in a
previous message.)
Note that I'm not running this RFC for my own personal benefit. I'm running
it because I care about making PHP (and each release of it) as good as
possible. I will postpone adopting the feature if I don't have the choice.
But until it's too late, I'm willing to engage in this topic because I
think the current state is far from ideal for 8.1.

The larger issue is that when used as a type on arguments, adding the
nullability flag isn't possible without a BC breaking change.

Regards,
Nicolas

[PHP-DEV] Re: [RFC] Nullable intersection types

[Nicolas Grekas]

> Hi everyone,
>
> as proposed by Nikita and Joe, I'm submitting this late RFC for your
> consideration for inclusion in PHP 8.1. Intersection types as currently
> accepted are not nullable. This RFC proposes to make them so.
>
> I wrote everything down about the reasons why here:
> https://wiki.php.net/rfc/nullable_intersection_types
>
> Please have a look and let me know what you think.
>
> Have a nice read,
>
> Nicolas
>

For the record, I was suggested to add a few more words about reflection in
the RFC.

In "Proposal", I added this:
> On the reflection side, ''ReflectionIntersectionType::allowsNull()'' will
return ''true''/''false'' depending on what the intersection type accepts.

In "Rationale", I added this:
> About reflection, one could imagine a more complex model based on a
''ReflectionIntersectionType'' nested inside a ''ReflectionUnionType''.
This RFC proposes to rely on ''ReflectionIntersectionType::allowsNull()''
instead. This is consistent with how ''T|null'' is returned without
''ReflectionUnionType'' wrapper, and is also simpler for userland to deal
with.