Re: [PHP-DEV] [VOTE] User Defined Operator Overloads

2022-01-03 Thread Nikita Popov
On Mon, Jan 3, 2022 at 1:14 AM Jordan LeDoux 
wrote:

> Hello internals,
>
> I've opened voting on
> https://wiki.php.net/rfc/user_defined_operator_overloads. The voting will
> close on 2022-01-17.
>
> To review past discussions on this RFC and the feature in general, please
> refer to:
>
> - https://externals.io/message/116611 | Current RFC discussion
> - https://externals.io/message/115764 | Initial RFC discussion
> - https://externals.io/message/115648 | Pre-RFC discussion and
> fact-finding
>
> Jordan
>

Voted No on this one. I did support the previous proposal on this topic,
but I don't like the changes that this iteration has introduced relative to
the previous proposal.

The part that I dislike most (and that I consider an exclusion criterion
regardless of any other merits of the proposal) is the introduction of a
new "operator +" style syntax. I found the motivation for this choice given
in the RFC rather weak -- it seems to be a very speculative
forward-compatibility argument, and I'm not sure it holds water even if we
accept the premise. There's nothing preventing us, from a technical
point-of-view, from allowing the use of some keyword only with magic
methods. On the other hand, the cost of this move is immediate: All tooling
will have to deal with a new, special kind of method declaration.

I'm also not a fan of the OperandPosition approach, though I could probably
live with it. The previous approach using static methods seemed more
natural to me, especially when it comes to operators that do not typically
commute (e.g. subtraction).

Regards,
Nikita


Re: [PHP-DEV] Surveying interest regarding CMake

2021-12-16 Thread Nikita Popov
On Thu, Dec 16, 2021 at 6:54 PM Horváth V. 
wrote:

> Hello internals,
>
> I'm writing to you to find out what the reception here is regarding the
> idea of moving the PHP project to build using CMake.
>
> I have looked around and I found 2 attempts of doing just that in the
> past (in 2008 via GSoC and something else maybe in 2014 that I couldn't
> find the exact info for before writing this email), but nothing came of
> those attempts. I have also briefly suggested the idea to Sara Golemon
> on Reddit and she didn't seem to be completely against the idea.
>
> For my attempt, I would also optionally use Conan as a means of fetching
> dependencies in a cross-platform manner, which would make the need for
> OS specific SDKs (like the Windows one) unnecessary. Thanks to CMake's
> amazing customizability, using Conan can be made completely optional and
> PHP could still continue to build with just system dependencies.
>
> Moving the build system to use CMake instead of the current split
> between a *nix and Windows solution would bring everything to one place
> and providing CMake bits in the install interface of PHP would make it
> easier to develop and use PHP from a development point of view thanks to
> CMake packages.
>
> I am planning to make a YouTube video of the whole process of me doing
> this grunt work, while I explore the current situation regarding
> building PHP and explain what I do and why. I think that it'd generally
> be a good thing to have such a video for transparency and it could be
> interesting educational material for people who are in a similar
> situation and wish to move to building their projects using CMake.
>
> For reference, I occasionally contribute to the CMake project, I'm the
> author of https://github.com/friendlyanon/cmake-init which aims to
> simplify quickly scaffolding a CMake project that's setup correctly and
> I'm in the process of peer reviewing a CMake related book.
>
> Let me know what you think.
>

My main question would be how this will affect 3rd party extensions, which
are currently using autoconf. Will they need to migrate to cmake, or will
we have to effectively maintain both build systems?

Generally, I do think that migrating to cmake makes sense though.

Regards,
Nikita


Re: [PHP-DEV] Re: Finishing AVIF support in getimagesize()

2021-12-09 Thread Nikita Popov
On Wed, Dec 8, 2021 at 12:41 PM Christoph M. Becker 
wrote:

> On 01.12.2021 at 00:52, Ben Morss via internals wrote:
>
> > Earlier this year, I added support for AVIF images
> >  to libgd
> > . My ultimate goal was to bring support
> for
> > this new image format to PHP, so that the world's top CMSs could let
> sites
> > serve AVIFs. A few of you kindly guided me as I made my first
> contributions
> > to the PHP codebase, propagating libgd's new AVIF support
> >  into PHP's bundled gd fork,
> > and adding
> > AVIF awareness  to non-gd
> > functions like getimagesize() and imagecreatefromstring().
> >
> > Unfortunately, when I worked on getimagesize(), AVIF experts advised that
> > there was no compact, reliable way to determine the size of an AVIF image
> > without relying on an external library like libavif. We decided
> >  that
> it
> > was useful to return the information that a given image was an AVIF, but
> > simply to return 0 for the actual dimensions and other details. The user
> > would simply need to decode the image to determine this information.
> >
> > Of course, users would really like
> >  to use
> > getimagesize() to return the actual size. So a colleague has kindly
> > created standalone
> > code 
> (that
> > doesn't depend on libavif) to do this, with the goal of adding it to PHP.
> >
> > The code comprises several hundred lines. But - it works, and it works
> well.
> >
> > Would it be ok to submit a PR containing this code to add this
> > functionality? Or does someone have a superior approach?
>
> Thanks for your and your colleague's work!  It's highly appreciated.
>
> Anyhow, a respective PR[1] has been submitted now, and I'm in favor of
> bundling libavifinfo.  I'm not too concerned regarding the additional
> size of the PHP binaries which would result by linking it in, but if
> others are, we could still introduce a configuration option (e.g.
> `--with-libavifinfo`).
>
> Thoughts?  Objections to bundling libavifinfo at all?
>
> [1] 
>

Assuming no licensing concerns, bundling libavifinfo sounds reasonable. The
library is small, our avif functionality is incomplete without it, and we
can't expect this to be available as a system library at this time.

Regards,
Nikita


[PHP-DEV] Re: [VOTE] Migrating to GitHub issues

2021-12-04 Thread Nikita Popov
On Sat, Nov 20, 2021 at 11:37 AM Nikita Popov  wrote:

> Hi internals,
>
> I've opened voting on https://wiki.php.net/rfc/github_issues. The vote
> closes 2021-12-04.
>
> Please see https://externals.io/message/116302 for the RFC discussion,
> and https://externals.io/message/114300 for the pre-RFC discussion.
>

The RFC has been accepted with 41 votes in favor and 4 against.

Regards,
Nikita


[PHP-DEV] Re: CI issues

2021-11-26 Thread Nikita Popov
On Wed, Nov 24, 2021 at 1:41 PM Nikita Popov  wrote:

> Hi internals,
>
> We're currently having some issues with CI, with both Travis and Azure not
> working, basically for the same reason (credits/parallelism for open-source
> projects got used up or expired). I've opened support requests on both
> platforms, but until then expect everything to fail (Cirrus and AppVeyor
> still work though).
>

Both Travis and Azure Pipelines are working again.

Regards,
Nikita


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

2021-11-26 Thread Nikita Popov
On Fri, Nov 12, 2021 at 2:07 PM Nikita Popov  wrote:

> Hi internals,
>
> I've opened the vote on
> https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will close
> 2021-11-26.
>

The RFC has been accepted with 52 votes in favor and 25 against.

Regards,
Nikita


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

2021-11-25 Thread Nikita Popov
On Fri, Nov 12, 2021 at 2:07 PM Nikita Popov  wrote:

> Hi internals,
>
> I've opened the vote on
> https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will close
> 2021-11-26.
>
> Regards,
> Nikita
>

As a reminder, voting on this RFC closes tomorrow. I usually don't specify
an exact time, but as the margin is so narrow: I plan to close the vote at
9am UTC.

Regards,
Nikita


Re: [PHP-DEV] Need Update regarding PHP Travis CI Execution

2021-11-25 Thread Nikita Popov
On Wed, Nov 24, 2021 at 12:42 PM Chandranana Naik 
wrote:

>
> Hi Team,
>
> Recently Travis builds have stopped running for PHP.
> We only see Cirrus and appveyor CI builds,  however travis.yml exists in
> the github repo.
>
> Could you please update if Travis builds will be started again or if there
> are plans to disable Travis CI support ?
>
> Regards,
> Chandranana
>

Travis is working again.

Regards,
Nikita


[PHP-DEV] CI issues

2021-11-24 Thread Nikita Popov
Hi internals,

We're currently having some issues with CI, with both Travis and Azure not
working, basically for the same reason (credits/parallelism for open-source
projects got used up or expired). I've opened support requests on both
platforms, but until then expect everything to fail (Cirrus and AppVeyor
still work though).

Regards,
Nikita


Re: [PHP-DEV] Need Update regarding PHP Travis CI Execution

2021-11-24 Thread Nikita Popov
On Wed, Nov 24, 2021 at 12:42 PM Chandranana Naik 
wrote:

>
> Hi Team,
>
> Recently Travis builds have stopped running for PHP.
> We only see Cirrus and appveyor CI builds,  however travis.yml exists in
> the github repo.
>
> Could you please update if Travis builds will be started again or if there
> are plans to disable Travis CI support ?
>

Hi,

I've sent a support request to Travis to extend us additional open-source
credits, so hopefully this will be running again soon.

Generally though, I would recommend you to migrate from Travis CI to GitHub
Actions. The PHP project itself uses Travis CI for testing non-x86
platforms only, which are generally not available on other CI providers.

Regards,
Nikita


Re: [PHP-DEV] PHP 8.2 proposal: "match", allow "default" as conditional_expression, e.g. 'en_US', 'en_GB', default => loadDefaults()

2021-11-22 Thread Nikita Popov
On Mon, Nov 22, 2021 at 7:18 PM Andreas  wrote:

> Hi internals,
>
> This is a proposal to allow to append the `default` pattern by comma to
> the end of the last match branch. (Like a conditional_expression)
>
> This allows to re-use the return_expression if required and avoids code
> duplication.
>
> PROPOSAL: PHP 8.2
>
>  return match ($locale) {
>  'de_DE', 'de_CH', 'de_AT' => loadGermanLanguageSettings(),
>  'en_US', 'en_GB', default => loadDefaultLanguageSettings(),
> };
> ?>
>

Isn't this equivalent to just this?

 loadGermanLanguageSettings(),
 default => loadDefaultLanguageSettings(),
};

'en_US' and 'en_GB' will already go to the default branch if they're not
listed explicitly.

Regards,
Nikita


[PHP-DEV] PHP Foundation

2021-11-22 Thread Nikita Popov
Hi internals!

I have two bits of news. The first is that I'm changing jobs at the end of
the month, and won't be working on PHP in a professional capacity anymore.
I'll still be around, but will have much less time to invest in PHP
development. I'd like to thank JetBrains for sponsoring my work on PHP for
the last three years -- I think we've managed to accomplish quite a lot.

The second one is that the long-discussed idea of a PHP Foundation is
finally becoming a reality! For the full details, please see the
announcement: https://jb.gg/phpfoundation

The initial purpose of the PHP Foundation is to support the development of
PHP by contracting developers to work on php-src either part-time or
full-time. If that sounds interesting to you, be sure to apply!

The foundation does not have any decision-making power on language changes:
these remain within the sole purview of the internals mailing list and the
RFC process. The fact that some work has been funded by the foundation does
not imply that it will be accepted into PHP.

The setup of the foundation was admittedly a bit of a rush job, with the
focus being on securing initial funding and making it available as soon as
possible. For this reason the foundation only has a temporary
administration that will need to figure out necessary bylaws for long-term
operation.

A big thanks goes to Roman Pronskiy from JetBrains for organizing this
effort, as well our initial sponsors and everyone who provided early
feedback.

Regards,
Nikita


[PHP-DEV] [VOTE] Migrating to GitHub issues

2021-11-20 Thread Nikita Popov
Hi internals,

I've opened voting on https://wiki.php.net/rfc/github_issues. The vote
closes 2021-12-04.

Please see https://externals.io/message/116302 for the RFC discussion, and
https://externals.io/message/114300 for the pre-RFC discussion.

Regards,
Nikita


Re: [PHP-DEV] PHP 8 Release Announcement Page

2021-11-19 Thread Nikita Popov
On Fri, Nov 19, 2021 at 4:16 AM Sara Golemon  wrote:

> In seven days, https://www.php.net/releases/8.0/en.php is going to be
> obsolete.
>
> Well, that's a harsh term, but it certainly won't reflect the current state
> on the ground, and we need to decide (should have decided, weeks ago) what
> we're going to do with it.
>
> 1/ Make a new announcement page for 8.1 ? Effort: High, Impact: Awesome
> 2/ Update the 8.0 page? Effort: Moderate, Impact: Still relatively awesome
> 3/ Remove the link from the banner (but still keep the page for archival
> purposes). Effort: Low, Impact: Shrugs all around
> 4/ Remove the link AND the page. Effort: Low, Impact: But... why?
>
> Personally, I've not got the cycles for 1 or 2, so I vote 3.  Anyone care
> to do more?  Bear in mind translations will be wanted.  If nobody steps up,
> then I'll plan on implementing #3 next Wednesday.
>

There's a draft page for the 8.1 announcement here:
https://github.com/php/web-php/pull/450

So if we want to do an announcement page for 8.1, there's probably not that
much work left in finishing that draft.

Regards,
Nikita


Re: [PHP-DEV] Re: [RFC] Migrating to GitHub issues

2021-11-18 Thread Nikita Popov
On Thu, Nov 18, 2021 at 2:53 PM Matthew Weier O'Phinney <
mweierophin...@gmail.com> wrote:

>
>
> On Thu, Nov 18, 2021, 7:32 AM Nikita Popov  wrote:
>
>> On Thu, Nov 18, 2021 at 2:07 PM Patrick ALLAERT 
>> wrote:
>>
>> > Le mer. 17 nov. 2021 à 13:30, Christoph M. Becker  a
>> > écrit :
>> > > Right.  An alternative might be to let users report security issues to
>> > > the security mailing list, where, if the issue turns out not to be a
>> > > security issue, the reporter could still be asked to submit a GH issue
>> > > about the bug.  In that case it might be useful to add more devs to
>> the
>> > > security mailing list.
>> >
>> > I was thinking about the same. Can't we work with security issues with
>> > mailing list *only*?
>> > It doesn't feel optimal to keep bugs.php.net active for just security
>> > ones.
>> > I miss seeing the motivation for it.
>> >
>>
>> The problem with the security mailing list is that it's ephemeral --
>> someone new can't look at past discussions before they were subscribed.
>> Additionally, it's not possible to make the issue and the whole
>> conversation around it public after the issue has been fixed.
>>
>
> With Laminas, we use an email alias to allow researchers to report to us.
> We then post the full report as a security issue on GitHub - it's a feature
> they rolled out late 2019/early 2020 that restricts visibility to
> maintainers initially, but allows inviting others to collaborate (we invite
> the reporter immediately, for instance). It also creates a private branch
> for collaboration. When the patch has been merged, you can mark the issue
> public.
>

Thanks for the suggestion! That does sound generally viable to me. Just to
clarify, this is not making use of issues, but rather of "advisories",
which GH implements as an independent feature.

I'm not involved in security response, so I can't say whether the security
group would want to adopt such a process. This is probably something that
should be decided among the people who handle security issues, rather than
here.

Regards,
Nikita


Re: [PHP-DEV] Re: [RFC] Migrating to GitHub issues

2021-11-18 Thread Nikita Popov
On Thu, Nov 18, 2021 at 2:07 PM Patrick ALLAERT 
wrote:

> Le mer. 17 nov. 2021 à 13:30, Christoph M. Becker  a
> écrit :
> > Right.  An alternative might be to let users report security issues to
> > the security mailing list, where, if the issue turns out not to be a
> > security issue, the reporter could still be asked to submit a GH issue
> > about the bug.  In that case it might be useful to add more devs to the
> > security mailing list.
>
> I was thinking about the same. Can't we work with security issues with
> mailing list *only*?
> It doesn't feel optimal to keep bugs.php.net active for just security
> ones.
> I miss seeing the motivation for it.
>

The problem with the security mailing list is that it's ephemeral --
someone new can't look at past discussions before they were subscribed.
Additionally, it's not possible to make the issue and the whole
conversation around it public after the issue has been fixed.

Regards,
Nikita


Re: [PHP-DEV] Proposal: &$result_code=null parameter in shell_exec()

2021-11-18 Thread Nikita Popov
On Thu, Nov 18, 2021 at 8:47 AM Luca Petrucci via internals <
internals@lists.php.net> wrote:

> Hi internals,
>
> This is a proposal to add an optional parameter &$result_code = null to
> the shell_exec() function.
>
> For clarity, the current signature is
> shell_exec(string $command): string|false|null
> The proposed signature is
> shell_exec(string $command, int &$result_code = null): string|false|null
>
> If present, the result_code parameter is set to the exit code of the
> command, as it is in exec() and system().
>
> This feature request was also posted by another user on
> https://bugs.php.net/bug.php?id=81493
> I have a draft pull request at https://github.com/php/php-src/pull/7663
>
> Thoughts?
>
> Thanks,
> Luca
>

This looks like a reasonable addition to me. Between shell_exec(),
system(), passthru() and exec(), shell_exec() is the only function that
doesn't currently accept a $result_code parameter, so including it makes
sense for consistency.

Regards,
Nikita


[PHP-DEV] Re: [RFC] Migrating to GitHub issues

2021-11-17 Thread Nikita Popov
On Mon, Nov 15, 2021 at 9:18 PM Björn Larsson 
wrote:

> Den 2021-11-02 kl. 15:19, skrev Nikita Popov:
> > Hi internals,
> >
> > The migration from bugs.php.net to GitHub issues has already been
> discussed
> > in https://externals.io/message/114300 and has already happened for
> > documentation issues.
> >
> > I'd like to formally propose to use GitHub for PHP implementation issues
> as
> > well: https://wiki.php.net/rfc/github_issues
> >
> > Regards,
> > Nikita
> >
> Hi,
>
> The current proposal is to move all new issues from bugs.php.net to
> Github except security ones.
>
> I think it's important to think a bit on what that means for reporting
> security issues in the future. I mean, if we leave bugs.php.net to rot
> in the corner, what are the consequences for reporting security issues?
>
> I think that aspect needs to be a bit further analysed like:
> - Will this move have a negative impact on reporting security issues
>on bugs.php.net?
># Both from a technical and people perspective.
> - Can one assume that by bugs.php.net having probably even less
>attention, that reporting security issues will work as is?
> - Is there an alternative for also handling security issues?
>
> Think it would be good if the RFC could analyse that a little, besides
> saying business as usual for security issues.
>

I don't think there's much more to say than that -- it should indeed be
business as usual. The only complication I see for security issues is that
we will not be able to easily move security issues that turn out to be
non-security bugs over to GitHub. As such, we may have a very low number of
new bugs appearing on bugs.php.net by being reported as security issues
first and being reclassified later. I don't view that as an immediate
problem, because to start with, we'll still be working with recent reports
on bugs.php.net anyway. Longer term, I do hope that GitHub will provide a
way to report issues privately (i.e. as indicated in
https://github.blog/2021-11-12-highlights-github-security-roadmap-universe-2021/),
so that we can consolidate everything in one tracker. But given the lack of
clear roadmap for this, I'm not basing any plans on it yet.

I do think that the handling of security issues is the weakest part of this
move, and probably the only area where choosing a different platform could
have a tangible advantage. However, we receive orders of magnitude less
security issues than other reports, and there is a much smaller number of
people involved in handling them, so I don't think we need to put too
strong a focus on this aspect.

Regards,
Nikita


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

2021-11-17 Thread Nikita Popov
On Wed, Nov 17, 2021 at 5:35 AM Paul Crovella 
wrote:

> On Fri, Nov 12, 2021 at 5:08 AM Nikita Popov  wrote:
> >
> > Hi internals,
> >
> > I've opened the vote on
> > https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will close
> > 2021-11-26.
> >
> > Regards,
> > Nikita
>
> In the Motivation section when talking about static analysis the RFC
> makes the claim:
>
> > The #[AllowDynamicProperties] attribute proposed in this RFC makes the
> cases where dynamic properties are used intentionally explicit.
>
> however this really isn't true as the attribute is on the class rather
> than the use. Static analysis will still have no idea whether any
> dynamic property assignment is indeed a bug or intentional. The
> information added is only whether the author of the class has deemed
> it okay for dynamic properties to be used on it, not by it. The class
> author and the dynamic property user might not be the same person or
> have any relation. The class being intentionally used with dynamic
> properties is not necessarily in the user's control. Similarly the
> class being unintentionally used with dynamic properties may not be
> either.
>

There is an assumption in the design, that certain classes are designed to
be used with dynamic properties, and all dynamic properties are acceptable
in that case. This is basically classes that could be using __get/__set,
but instead use dynamic properties for reasons of simplicity, performance
or migration ease. If the class only works with a fixed set of properties
then those should be declared instead, and if it has more complex
constraints, then __get/__set can be used to implement arbitrary rules.
(Setting dynamic properties on classes you do not own and that do not
opt-in is explicitly unsupported under this model, with the recommendation
to use WeakMaps for non-intrusive value association instead.)

Regards,
Nikita


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

2021-11-15 Thread Nikita Popov
On Mon, Nov 15, 2021 at 1:52 PM Andreas Heigl  wrote:

> Hea all.
>
> On 15.11.21 10:52, Derick Rethans wrote:
> > Dear Internals,
> >
> > On Wed, 10 Nov 2021, Nikita Popov wrote:
> >
> >> On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov 
> wrote:
> >>
> >>> 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.
> >>>
> >>
> >> I plan to open voting on this RFC soon. Most of the feedback was
> >> positive, apart from the initial choice of opt-int mechanism, and that
> >> part should be addressed by the switch to the
> >> #[AllowDynamicProperties] attribute.
> >
> > The voting is now open, but I think one thing was not taken into account
> > here, the many small changes that push work to maintainers of Open
> > Source library and CI related tools.
> >
> > In the last few years, the release cadence of PHP has increased, which
> > is great for new features. It however has not been helpful to introduce
> > many small deprecations and BC breaks in every single release.
> >
> > This invariably is making maintainers of Open Source anxious, and
> > frustrated as so much work is need to keep things up to date. I know
> > they are *deprecations*, and applications can turn these off, but that's
> > not the case for maintainers of libraries.
> >
> > Before we introduce many more of this into PHP 8.2, I think it would be
> > wise to figure out a way how to:
> >
> > - improve the langauge with new features
> > - keep maintenance cost for open source library and CI tools much lower
> > - come up with a set of guidelines for when it is necessary to introduce
> >BC breaks and deprecations.
> >
> > I am all for improving the language and making it more feature rich, but
> > we have not spend enough time considering the impacts to the full
> > ecosystem.
> >
> > I have therefore voted "no" on this RFC, and I hope you will too.
> >
> > cheers,
> > Derick
>
> After some thoughs on this RFC I have reverted my original vote and
> voted "No" due to several reasons.
>
> For one thing it is not clear to me what the benefits are.
>

That's my bad! The RFC did not really go into the motivation for the
change, especially after I dropped the discussion of internal motivations
in a later revision.

I hope that the new "Motivation" section clarifies things a bit, especially
in regards to why "static analysis" is only a partial solution to this
problem: https://wiki.php.net/rfc/deprecate_dynamic_properties#motivation

Regards,
Nikita


Re: [PHP-DEV] [RFC] Migrating to GitHub issues

2021-11-15 Thread Nikita Popov
On Mon, Nov 15, 2021 at 10:47 AM Derick Rethans  wrote:

> Dear Internals,
>
> I think it is a mistake to decide on a whim to move over to GitHub
> issues without having evaluated anything else.
>
> GitHub is a proprietary platform, where unlike with the code
> repositories, the interactions and issues are not easy to migrate out of
> again. It is also now owned by MSFT, and although they are making
> friendly noises towards Open Source, I remain largely skeptical (with as
> a hint, the stuff they're pulling with AI code completion).
>
> I understand and share that "bugsnet" has many issues, and I don't
> necessarily object moving to something else.
>
> However, in the last 25+ years we've always hosted this ourselves, and
> this prevented any sort of vendor lock in. I think it is important to
> own our own data here, or at least have full access to any interactions.
>
> This jump to GitHub Issues feels rushed, and I worry that we end up
> making the wrong decision, especially because from the RFC it seems we
> need to build some functionality (tags scheme, for example) on top of
> it. And this will need to be maintained separately, and I worry that too
> few people are going to be interested in gardening the issues on GH.
>
> I believe we would be much better served having a look what is
> available, and see what else is suitable. I'm therefore intending to
> vote no on this.


Hey Derick,

The RFC includes a bit of discussion of alternatives in abstract (
https://wiki.php.net/rfc/github_issues#alternatives) and the key point
(which has also been brought up by other people in the meantime) is that
the choice of GitHub issues is very much not a whim:

For better or worse, GitHub is where nearly all open-source projects are
hosted, which means that pretty much anyone involved in open-source has an
account there and is familiar with the workflows. It is also where PHP
hosts it's repos and where we accept pull requests. An alternative issue
tracker has to compete not just on technical grounds, but also on
integration, familiarity and network-effect. For an open-source project,
these aspects are quite important when it comes to interaction with casual
contributors.

Working at JetBrains, proposing YouTrack instead of GH issues has certainly
crossed my mind -- there is no doubt that at a technical level, it's a much
better bug tracker than GH issues. But that's simply not the right question
to ask. The right question is whether, given our rather simple
requirements, is it sufficiently better to overshadow the other benefits of
GitHub issues for an open-source project? I don't think so. We just need GH
issues to be "good enough" for our purposes, and I think that at this point
it is.

I'll also mention that the discussion about this migration has been going
on for six months, and in that time all I've ever seen are vague mentions
of alternatives, but nobody has provided any in-depth analysis that goes
beyond a simple name drop. I went through the trouble of providing a
detailed evaluation of how the GitHub issues migration will look like,
while the alternatives are still at the state of "Phabricator is a thing"
(ooops, it actually isn't -- it managed to be discontinued while the
discussion was going on!)

Finally, for me an important part of the migration (whether to GH Issues or
something else) is specifically that we don't host it ourselves, because we
have historically always been really bad at that. Of course, letting
someone else host it is incompatible with the desire to fully "own" our
data. I do appreciate the general sentiment here though.

Regards,
Nikita


[PHP-DEV] Re: [RFC] Migrating to GitHub issues

2021-11-14 Thread Nikita Popov
On Tue, Nov 2, 2021 at 3:19 PM Nikita Popov  wrote:

> Hi internals,
>
> The migration from bugs.php.net to GitHub issues has already been
> discussed in https://externals.io/message/114300 and has already happened
> for documentation issues.
>
> I'd like to formally propose to use GitHub for PHP implementation issues
> as well: https://wiki.php.net/rfc/github_issues
>

As a heads up, voting on this RFC starts in two days.

Regards,
Nikita


[PHP-DEV] Re: Unwrap reference after foreach

2021-11-14 Thread Nikita Popov
On Wed, Nov 10, 2021 at 10:06 AM Nikita Popov  wrote:

> On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov  wrote:
>
>> Hi internals,
>>
>> I'd like to address a common footgun when using foreach by reference:
>> https://wiki.php.net/rfc/foreach_unwrap_ref
>>
>> This addresses the issue described in the big red box at
>> https://www.php.net/manual/en/control-structures.foreach.php. While this
>> is "not a bug" (as our bug tracker can regularly attest), it's rather
>> unexpected, and we could easily avoid it...
>>
>
> As the discussion has died down, I plan to open voting on this RFC soon.
>
> I have to admit that I'm less convinced of this than I was originally,
> because there's a surprising number of edge cases involved. The behavior is
> more intuitive on the surface, but things get more complicated when you
> look at detailed language semantics.
>

I have ultimately decided to withdraw this proposal.

Regards,
Nikita


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

2021-11-12 Thread Nikita Popov
On Fri, Nov 12, 2021 at 5:34 PM Nikita Popov  wrote:

> On Fri, Nov 12, 2021 at 5:25 PM Ben Ramsey  wrote:
>
>> > On Nov 12, 2021, at 10:10, Derick Rethans  wrote:
>> >
>> > On 12 November 2021 13:07:42 GMT, Nikita Popov 
>> wrote:
>> >> Hi internals,
>> >>
>> >> I've opened the vote on
>> >> https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will
>> close
>> >> 2021-11-26.
>> >
>> > I've voted no on this one. Not because I don't like the idea, but
>> because I think the timeline for deprecation and removal is way too fast.
>> >
>> > This is IMO not something important enough to cause a fairly large BC
>> break for, and it should wait until the last in the 8.x series before we
>> deprecate it.
>> >
>> > cheers
>> > Derick
>>
>>
>> I’ve voted no for the same reason.
>>
>> I like this change, but the deprecation in 8.2 seems too disruptive. I’d
>> rather promote our intent to deprecate this with a statement in the
>> manual that says something like, “This feature will be deprecated in
>> PHP 8.3 and removed in 9.0.”
>
>
> FWIW I think we should always deprecate things as soon as possible, to
> give people the maximum amount of awareness and time to address the issue
> before the actual removal occurs. Most people will not be aware they need
> to take action if it's just a note in the manual. For that reason, I find
> it generally preferable to deprecate something closer to PHP 8.0 than to
> 8.4, which would be directly before a major version with limited time to
> act. Now, we don't usually tend to optimize for "time of deprecation"
> because that requires planning deprecations many years in advance, but if
> the choice existed I'd always go for deprecating early in the major release
> cycle, rather than late.
>

Another thing I want to add here is that in this instance, I think that the
deprecation warning is really helpful for updating your code. It tells you
exactly which property on which class is being created dynamically, so you
can quickly go through these and add missing property declarations or
#[AllowDynamicProperties] attributes. Without the deprecation warning in
place, you need a static analyzer to find problematic cases. And of course,
that only works well if you already have a fully typed code base that is
reasonably clean under static analysis. At the same time, this change is
most likely to affect projects where this is not the case. If you are
already using a static analyzer, you probably don't use dynamic properties
anyway, as these things tend to be incompatible.

Regards,
Nikita


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

2021-11-12 Thread Nikita Popov
On Fri, Nov 12, 2021 at 5:25 PM Ben Ramsey  wrote:

> > On Nov 12, 2021, at 10:10, Derick Rethans  wrote:
> >
> > On 12 November 2021 13:07:42 GMT, Nikita Popov 
> wrote:
> >> Hi internals,
> >>
> >> I've opened the vote on
> >> https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will
> close
> >> 2021-11-26.
> >
> > I've voted no on this one. Not because I don't like the idea, but
> because I think the timeline for deprecation and removal is way too fast.
> >
> > This is IMO not something important enough to cause a fairly large BC
> break for, and it should wait until the last in the 8.x series before we
> deprecate it.
> >
> > cheers
> > Derick
>
>
> I’ve voted no for the same reason.
>
> I like this change, but the deprecation in 8.2 seems too disruptive. I’d
> rather promote our intent to deprecate this with a statement in the
> manual that says something like, “This feature will be deprecated in
> PHP 8.3 and removed in 9.0.”


FWIW I think we should always deprecate things as soon as possible, to give
people the maximum amount of awareness and time to address the issue before
the actual removal occurs. Most people will not be aware they need to take
action if it's just a note in the manual. For that reason, I find it
generally preferable to deprecate something closer to PHP 8.0 than to 8.4,
which would be directly before a major version with limited time to act.
Now, we don't usually tend to optimize for "time of deprecation" because
that requires planning deprecations many years in advance, but if the
choice existed I'd always go for deprecating early in the major release
cycle, rather than late.


> Meanwhile, 8.2 should include the
> AllowDynamicProperties attribute so folks can start preparing.
>

Given how attributes in PHP work, this doesn't make sense to me. You can
already use #[AllowDynamicProperties] in your code right now, without any
special support from PHP. Only static analyzers / IDEs need to know that
they shouldn't complain about it even on versions where it does not
technically exist.

Regards,
Nikita


[PHP-DEV] [VOTE] Deprecate dynamic properties

2021-11-12 Thread Nikita Popov
Hi internals,

I've opened the vote on
https://wiki.php.net/rfc/deprecate_dynamic_properties. Voting will close
2021-11-26.

Regards,
Nikita


Re: [PHP-DEV] Automatic implementation of Stringable may conflict with old, untyped arginfo declarations

2021-11-12 Thread Nikita Popov
On Wed, Nov 10, 2021 at 10:13 PM Jeremy Mikola  wrote:

>
>
> On Tue, Nov 9, 2021 at 4:30 AM Nikita Popov  wrote:
>
>>
>> In
>> https://github.com/php/php-src/commit/a551b083073ea08f8fc53b0e1a6380b6de26cc6b
>> I've added a hack to add the string return type if it is missing and thus
>> make the signature compatible with the interface. That should address the
>> immediate compatibility issue.
>>
>
> Thanks for the quick fix.
>
> A related question that came up, and is most likely unique to ext-mongodb,
> follows. Many of our classes with toString() methods also implement a
> corresponding interface with a toString() method. For example:
>
>  * https://www.php.net/manual/en/class.mongodb-bson-binary.php
>  * https://www.php.net/manual/en/class.mongodb-bson-binaryinterface.php
>
> I'm in the process of adding explicit return type info to _all_ of our
> toString() arginfos (classes and interfaces), but a thought occurred to me
> that doing so may be a subtle BC break for userland classes implementing
> these interfaces, since an explicit string return type would then become
> necessary. But if I only modify our classes and leave our interfaces as-is,
> PHP rightfully reports an error because the class' toString() method cannot
> conform to both Stringable (with type info) and our interface (without type
> info) -- at least PHP versions before 8.1.0RC6.
>
> Reading the patch above, it looks like the automatic Stringable
> implementation only kicks in when the arginfo has no existing return type
> info. In that case, I think the only option to completely avoid a BC break
> would be to continue to leave our return type info omitted (on both our
> classes _and_ interfaces) and allow PHP 8.1+ to apply it automatically. Is
> that correct?
>

With the introduction of Stringable PHP also started automatically adding
the string result type to __toString(), specifically for compatibility with
the interface. As such, it should be safe to add the string return type
everywhere for PHP >= 8.0. (If you use stubs with legacy arginfo, that
would automatically give you the right behavior: types on PHP 8, no types
on old versions.)

Regards,
Nikita


Re: [PHP-DEV] [RFC] Migrating to GitHub issues

2021-11-11 Thread Nikita Popov
On Wed, Nov 10, 2021 at 7:23 PM Niklas Keller  wrote:

> Hey Nikita,
>
> I'd like to propose using HackerOne instead of bugs.php.net for security
> issues: https://www.hackerone.com/company/open-source-community
>
> Best,
> Niklas
>

Unfortunately I have no familiarity with HackerOne and as such don't know
whether it would work for our purposes. I think an important requirement
for us is that maintainers who are not otherwise involved in security
response can be assigned to (and see) issues.

I'm hazy on the details, but I believe that PHP used to be part of IBB on
HackerOne and was kicked out due to lack of responsiveness (apparently
nobody from the PHP side was actually involved there).

Regards,
Nikita


Re: [PHP-DEV] [RFC] Migrating to GitHub issues

2021-11-11 Thread Nikita Popov
On Wed, Nov 10, 2021 at 5:51 PM Nikita Popov  wrote:

> On Thu, Nov 4, 2021 at 5:58 PM Dan Ackroyd  wrote:
>
>> On Tue, 2 Nov 2021 at 14:19, Nikita Popov  wrote:
>> >
>> > I'd like to formally propose to use GitHub for PHP implementation
>> issues as
>> > well: https://wiki.php.net/rfc/github_issues
>>
>> In general, yes please. The only bit I'll chime in on is:
>>
>> > bugs.php.net will remain active for the following purposes:
>> > Reporting of issues against PECL extensions. (We may want to discontinue
>> > this as well. Many actively maintained extensions already use GitHub
>> issues
>> > rather than bugs.php.net.)
>>
>>
>> Providing a bug reporting site was a useful thing to do 23 years ago,
>> as it would have been either expensive or time consuming for each
>> project to set up their own. It doesn't seem as sensible now as:
>>
>> * Having bugs.php.net remain open for some bits of PHP, but not
>> others, would be confusing for users.
>>
>> * Most extensions are hosted on a platform that already provides issue
>> tracking - though it seems quite a few extensions (including Imagick)
>> have not realised that there is a bug tracking setting that can/should
>> be updated on pecl.
>>
>> * There's an ongoing issue of extensions becoming unmaintained over
>> time. If people are opening bugs on the PHP issue tracker, it's
>> natural for them to have an expectation that someone from the PHP
>> project would work on that issue at some point.
>>
>> So unless someone has a strong argument for keeping the bugs.php.net
>> open for PECL extensions, I think it shouldn't be.
>>
>> btw it would probably be useful to dump out a list of where to report
>> bugs for different extensions and put that as a page on bugs.php.net
>> though. If nothing else, Google thinks Imagick is an alias for
>> ImageMagick far too often and sometimes pecl.php.net is unresponsive.
>
>
> Yes, we should definitely migrate PECL extensions away from bugs.php.net
> as well. I didn't cover this in the RFC because it doesn't really relate to
> the setup described there and this part of the migration can happen in
> parallel.
>
> To migrate PECL extensions hosted by the PHP organization away from
> bugs.php.net, we need to:
> 1. Enable GH issues on the repo.
> 2. Disable the package on bugs.php.net.
> 3. Update the bug tracker URL on pecl.php.net.
> 4. (Maybe) manually migrate outstanding open bugs.
>
> For extensions not hosted by the PHP organization we may have to contact
> maintainers to determine which issue tracker to use.
>

As an update here: I've gone through all the PECL packages on bugs.php.net
and found that nearly all of them are either unmaintained or already have a
different bugtracker (almost always GitHub issues). Those packages are now
disabled and cmb has updated the bug tracker URLs on pecl.php.net. Next
step will be to open GH issues for repos that the PHP organization owns,
and then we'll have to see what to do with the handful of remaining
extensions.

I've updated the RFC to say that bugs.php.net to say that it will not
remain open for PECL extensions either.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Migrating to GitHub issues

2021-11-10 Thread Nikita Popov
On Thu, Nov 4, 2021 at 5:58 PM Dan Ackroyd  wrote:

> On Tue, 2 Nov 2021 at 14:19, Nikita Popov  wrote:
> >
> > I'd like to formally propose to use GitHub for PHP implementation issues
> as
> > well: https://wiki.php.net/rfc/github_issues
>
> In general, yes please. The only bit I'll chime in on is:
>
> > bugs.php.net will remain active for the following purposes:
> > Reporting of issues against PECL extensions. (We may want to discontinue
> > this as well. Many actively maintained extensions already use GitHub
> issues
> > rather than bugs.php.net.)
>
>
> Providing a bug reporting site was a useful thing to do 23 years ago,
> as it would have been either expensive or time consuming for each
> project to set up their own. It doesn't seem as sensible now as:
>
> * Having bugs.php.net remain open for some bits of PHP, but not
> others, would be confusing for users.
>
> * Most extensions are hosted on a platform that already provides issue
> tracking - though it seems quite a few extensions (including Imagick)
> have not realised that there is a bug tracking setting that can/should
> be updated on pecl.
>
> * There's an ongoing issue of extensions becoming unmaintained over
> time. If people are opening bugs on the PHP issue tracker, it's
> natural for them to have an expectation that someone from the PHP
> project would work on that issue at some point.
>
> So unless someone has a strong argument for keeping the bugs.php.net
> open for PECL extensions, I think it shouldn't be.
>
> btw it would probably be useful to dump out a list of where to report
> bugs for different extensions and put that as a page on bugs.php.net
> though. If nothing else, Google thinks Imagick is an alias for
> ImageMagick far too often and sometimes pecl.php.net is unresponsive.


Yes, we should definitely migrate PECL extensions away from bugs.php.net as
well. I didn't cover this in the RFC because it doesn't really relate to
the setup described there and this part of the migration can happen in
parallel.

To migrate PECL extensions hosted by the PHP organization away from
bugs.php.net, we need to:
1. Enable GH issues on the repo.
2. Disable the package on bugs.php.net.
3. Update the bug tracker URL on pecl.php.net.
4. (Maybe) manually migrate outstanding open bugs.

For extensions not hosted by the PHP organization we may have to contact
maintainers to determine which issue tracker to use.

Regards,
Nikita


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

2021-11-10 Thread Nikita Popov
On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov  wrote:

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

I plan to open voting on this RFC soon. Most of the feedback was positive,
apart from the initial choice of opt-int mechanism, and that part should be
addressed by the switch to the #[AllowDynamicProperties] attribute.

Regards,
Nikita


[PHP-DEV] Re: Unwrap reference after foreach

2021-11-10 Thread Nikita Popov
On Fri, Aug 13, 2021 at 3:28 PM Nikita Popov  wrote:

> Hi internals,
>
> I'd like to address a common footgun when using foreach by reference:
> https://wiki.php.net/rfc/foreach_unwrap_ref
>
> This addresses the issue described in the big red box at
> https://www.php.net/manual/en/control-structures.foreach.php. While this
> is "not a bug" (as our bug tracker can regularly attest), it's rather
> unexpected, and we could easily avoid it...
>

As the discussion has died down, I plan to open voting on this RFC soon.

I have to admit that I'm less convinced of this than I was originally,
because there's a surprising number of edge cases involved. The behavior is
more intuitive on the surface, but things get more complicated when you
look at detailed language semantics.

Regards,
Nikita


Re: [PHP-DEV] VM kinds

2021-11-10 Thread Nikita Popov
On Wed, Nov 10, 2021 at 8:13 AM Tim Starling 
wrote:

> What are VM kinds for? Are they just a prototyping tool to allow
> different implementation strategies to be benchmarked? Or are they
> permanent? Does anyone use them?
>
> I ask because I'm working on a VM bug, and non-default VM kinds make
> already complex code even more complex and harder to edit.
>
> -- Tim Starling
>

I don't think anyone uses them, and the GOTO/SWITCH VMs receive little
testing in practice (we don't use them in CI), and I believe they're not
supported by the JIT either. Personally, I would be fine with simply
dropping them, as they do add additional maintenance burden without any
apparent benefit. Maybe Dmitry has some thoughts on this.

Regards,
Nikita


Re: [PHP-DEV] Automatic implementation of Stringable may conflict with old, untyped arginfo declarations

2021-11-09 Thread Nikita Popov
On Tue, Nov 9, 2021 at 2:11 AM Jeremy Mikola  wrote:

>
> https://github.com/php/php-src/commit/b302bfabe7fadd07b022ee30aacf7f41ab1ae0fa
> recently added automatic implementation of Stringable (
> https://wiki.php.net/rfc/stringable) for internal classes. This introduced
> fatal errors for each existing __toString() in ext-mongodb:
>
> ```
> Declaration of MongoDB\BSON\BinaryInterface::__toString() must be
> compatible with Stringable::__toString(): string in Unknown on line 0
> Declaration of MongoDB\BSON\Binary::__toString() must be compatible with
> Stringable::__toString(): string in Unknown on line 0
> ...
> ```
>
> Our arginfos for these methods have historically never reported a return
> type, going back to when it was originally written for PHP 5.x. For
> example:
>
> ```
> ZEND_BEGIN_ARG_INFO_EX(ai_BinaryInterface_void, 0, 0, 0)
> ZEND_END_ARG_INFO()
>
> static zend_function_entry php_phongo_binary_interface_me[] = {
>   ZEND_ABSTRACT_ME(BinaryInterface, __toString, ai_BinaryInterface_void)
>   // ...
> ```
>
> I noted that _ZendTestClass (referenced in the original commit's tests)
> uses ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX to declare a string return
> type (
>
> https://github.com/php/php-src/blob/eda9d7ac22c70f75a44bf33139acf8c812d69944/ext/zend_test/test_arginfo.h#L74
> ).
>
> While that's a trivial change we can make in ext-mongodb, I wonder if this
> may have been an unanticipated BC break for third-party extensions. I
> imagine ext-mongodb is not the only extension with older arginfo
> declarations predating the introduction of type reporting in later PHP
> versions.
>

Thanks for pointing this out!

In
https://github.com/php/php-src/commit/a551b083073ea08f8fc53b0e1a6380b6de26cc6b
I've added a hack to add the string return type if it is missing and thus
make the signature compatible with the interface. That should address the
immediate compatibility issue.

In
https://github.com/php/php-src/commit/86379b6710f972e0d4a11c89ce28d5768d9824d3
I have added a warning if this happens. The warning is only for master
(i.e. PHP 8.2) to make sure that extensions add the type eventually, and we
can drop this workaround.

Regards,
Nikita


[PHP-DEV] [RFC] Migrating to GitHub issues

2021-11-02 Thread Nikita Popov
Hi internals,

The migration from bugs.php.net to GitHub issues has already been discussed
in https://externals.io/message/114300 and has already happened for
documentation issues.

I'd like to formally propose to use GitHub for PHP implementation issues as
well: https://wiki.php.net/rfc/github_issues

Regards,
Nikita


[PHP-DEV] Re: [VOTE] Deprecate partially supported callables

2021-10-22 Thread Nikita Popov
On Fri, Oct 8, 2021 at 4:16 PM Nikita Popov  wrote:

> Hi internals!
>
> I've opened voting on
> https://wiki.php.net/rfc/deprecate_partially_supported_callables, which
> deprecated callables that are not supported in $callable() syntax. The vote
> closes on 2021-10-22.
>

The proposal has been accepted unanimously, with 32 votes in favor.

Regards,
Nikita


Re: [PHP-DEV] Bugsnet

2021-10-21 Thread Nikita Popov
On Thu, Oct 21, 2021 at 10:42 PM Jakub Zelenka  wrote:

> On Thu, Oct 21, 2021 at 4:07 PM Nikita Popov  wrote:
>
>>
>> I'm proposing the following label structure (the list at
>> https://github.com/nikic/test-repo/labels is incomplete though): Each
>> report has either a bug or feature label. Additionally it starts out with
>> the T-needs-triage label. Once a project member triages the report, the
>> label is removed and a category label is added instead, e.g. C-OpenSSL for
>> issues relating to the OpenSSL extension.
>>
>> I also have a few more triage labels (T-needs-feedback, T-verified,
>> T-invalid, T-wontfix, T-duplicate), but I'm not sure we actually need any
>> but the first one or the first two -- I personally don't see a lot of
>> value
>> in having a label for why exactly an issue has been closed, the fact that
>> it is closed is usually sufficient.
>>
>>
> Have you considered custom fields that are now in beta with some other
> features in https://github.com/features/issues ? That seems a bit nicer
> to me...
>

Yes, I tested this as well (the PHP organization is signed up for the
beta). Unfortunately, I found this functionality rather awkward and don't
think it would work well for us. The key problem is that the new
functionality is not an improvement for issues -- it's an improvement for
"projects". You can add an issue to a project (manually) and then you can
add extra metadata to the issue in that project. It's not possible to add
custom fields to issues themselves.

Regards,
Nikita


Re: [PHP-DEV] Bugsnet

2021-10-21 Thread Nikita Popov
On Sun, May 9, 2021 at 8:49 AM Joe Watkins  wrote:

> Morning internals,
>
> We have a spam problem on bugsnet, it's not a new problem. Nikita had to
> waste time deleting 20 odd messages from bugsnet yesterday and this is a
> common, daily occurrence. We clearly don't have time for this.
>
> Quite aside from spam problems, bugsnet is hidden away in a dark corner of
> the internet that requires a special login, doesn't integrate with source
> code or our current workflow (very nicely), and doesn't get updated or
> developed.
>
> Having moved our workflow to github, now seems to be the time to seriously
> consider retiring bugsnet for general use, and using the tools that are
> waiting for us - Github Issues.
>
> I'm aware that bugsnet serves as the disclosure method for security bugs
> and github doesn't have a solution to that. Leaving that to one side for
> now ...
>
> I'm also aware that bugsnet carries with it 20 years worth of crusty old
> feature requests and bugs, that are never realistically going to be dealt
> with. In the past I've spent time trying to close very old bugs that no
> longer seem relevant, the fact is that there are so many of these that I
> don't think I made a dent.
>
> It seems obvious that we don't want to migrate all of the data on bugsnet,
> but nor do we want to loose the most recent and relevant reports.
>
> I propose that we disable bugsnet for all but security issues leaving
> responsible disclosure method to be handled in some other way at a later
> date. Leaving bugsnet in a (mostly) readonly mode.
>
> We then send a notification to all bugs that were opened against a specific
> and supported version of PHP, notifying the opener of the change and
> requesting that they take a couple of minutes to open their issue on
> github.
>
> I think we might get quite a good response here - anyone suffering the
> worst consequences of bugs - production servers can't be upgraded and so on
> - are already waiting for a notification from bugsnet, I'm sure the
> majority of them will do as we ask.
>
> In some set number of weeks (to be decided), and depending on the response
> to our switching over to github, we can try to determine at that time if
> it's worth trying to import any data from bugsnet. We can also consider at
> this time when it might be appropriate to retire bugsnet entirely.
>
> We will not be free of spam simply by moving, but github has the tools we
> need to moderate the content properly - you can block people. In addition,
> I feel people are less likely to misbehave if they think their co-workers
> or employers might be able to see what they are doing, which may have an
> effect also.
>
> It may be over optimistic, but we might get better engagement with bugs on
> github than anywhere else also - Github is where people are tending to do
> their business today.
>
> Github is maintained, hosted, developed, and free, and while it isn't the
> perfect tool for the job, nothing else is either. We could spend time
> (which we don't have) developing bugsnet, or installing some other solution
> in a dark corner of the internet, and solve no problems at all, and be
> burdened with the ongoing maintenance of that solution.
>
> The people who have to spend the most time on this are release managers,
> and so while I'm talking to everyone, it is release managers opinions that
> I'm most interested in, they are the people who will be and have been most
> effected by the shortcomings in bugsnet, whose opinions are most relevant
> in this space.
>
> I don't think a vote is appropriate, this decision should be made by the
> people whose "jobs" are directly effected - with input from the community,
> of course. Not least of all, it will take a month to close a vote, by which
> time we will have wasted another (working) day or more of Nikitas time.
> Having said all that, I am looking for a consensus before we take any
> action. My arm can be twisted, but this is my current position and I think
> it's a reasonable one.
>
> On the issue of responsible disclosure ... we can treat this separately,
> with the recent change in the workflow, this process is in need of review
> anyway. How that is handled should be decided by the people who have a hand
> in that process, and so it seems prudent to leave it aside for now.
>
> Cheers
> Joe
>

To follow up on this proposal: We have been using GitHub Issue for docs for
a while now (https://github.com/php/doc-en/issues) and I'm about to disable
submission of new docs issues on bugs.php.net. I think it's time to switch
non-security bugs for PHP proper as well, for all the reasons that have
already been discussed.

For docs we didn't really do anything special in terms of labels etc. I
think for the php-src repo we do want to introduce some more structure for
categorization and triage, as the volume tends to be higher there.

For that purpose I've created a prototype of how things could look like
here: 

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

2021-10-21 Thread Nikita Popov
On Thu, Oct 21, 2021 at 1:12 AM Dylan K. Taylor  wrote:

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

Sounds reasonable to me!

Nikita


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

2021-10-13 Thread Nikita Popov
On Wed, Oct 13, 2021 at 3:43 AM Tim Starling 
wrote:

> On 12/10/21 9:23 pm, Nikita Popov wrote:
> > Based on the received feedback, I've updated the RFC to instead provide
> an
> > #[AllowDynamicProperties] attribute as a way to opt-in to the use of
> > dynamic properties. As previously discussed, this won't allow us to
> > completely remove dynamic properties from the language model anymore, but
> > it will make the upgrade path smoother by avoiding multiple inheritance
> > issues. Especially given recent feedback on backwards-incompatible
> changes,
> > I think it's prudent to go with the more conservative approach.
>
> I think it would still be the biggest compatibility break since PHP
> 4->5. Adding a custom property is a common way for an extension to
> attach data to an object generated by an uncooperative application or
> library.
>
> As the RFC notes, you could migrate to WeakMap, but it will be very
> difficult to write code that works on both 7.x and 8.2, since both
> attributes and WeakMap were only introduced in 8.0. In MediaWiki
> pingback data for the month of September, only 5.2% of users are on
> PHP 8.0 or later.
>

Just to be clear on this point: While attributes have only been introduced
in 8.0, they can still be used in earlier versions and are simply ignored
there. It is safe to use #[AllowDynamicProperties]  without version
compatibility considerations.


> Also, in the last week, I've been analyzing memory usage of our
> application. I've come to a new appreciation of the compactness of
> undeclared properties on classes with sparse data. You can reduce
> memory usage by removing the declaration of any property that is null
> on more than about 75% of instances. CPU time may also benefit due to
> improved L2 cache hit ratio and reduced allocator overhead.
>

Huh, that's an interesting problem. Usually I only see the reverse
situation, where accidentally materializing the dynamic property table
(through foreach, array casts, etc) causes issues, because it uses much
more memory than declared properties. Based on a quick calculation, the
cost of having dynamic properties clocks in at 24 declared properties (376
bytes for the smallest non-empty HT vs 16 bytes per declared property), so
that seems like it would usually be a bad trade off unless you already end
up materializing dynamic properties for other reasons.

Did you make sure that you do not materialize dynamic properties (already
before un-declaring some properties)?


> So if the point of the RFC is to eventually get rid of property
> hashtables from the engine, I'm not sure if that would actually be a
> win for performance. I'm more thinking about the need for a sparse
> attribute which moves declared properties out of properties_table.
>

The ability to opt-in to dynamic properties would always remain in some
form (if only through stdClass extension as originally proposed), so if you
have a case where it makes sense, the option would still be there.

Regards,
Nikita


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

2021-10-12 Thread Nikita Popov
On Tue, Oct 12, 2021 at 3:52 PM Levi Morrison 
wrote:

> > Based on the received feedback, I've updated the RFC to instead provide
> an
> > #[AllowDynamicProperties] attribute as a way to opt-in to the use of
> > dynamic properties. As previously discussed, this won't allow us to
> > completely remove dynamic properties from the language model anymore, but
> > it will make the upgrade path smoother by avoiding multiple inheritance
> > issues.
>
> Quoting the updated RFC:
> > We may still wish to remove dynamic properties entirely at some later
> > point. Having the #[AllowDynamicProperties] attribute will make it much
> > easier to evaluate such a move, as it will be easier to analyze how much
> > and in what way dynamic properties are used in the ecosystem.
>
> But in this place it does not mention PHP 9.0. Other places still
> mention converting it to an error for PHP 9.0. Is that still the plan?
>

The current RFC proposes to make dynamic properties an error for classes
without #[AllowDynamicProperties] in PHP 9.0. On the other hand, classes
using the attribute will be able to continue using dynamic properties
without error. (That is, as usual: Deprecations are converted to error in
the next major version.)

Regards,
Nikita


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

2021-10-12 Thread Nikita Popov
On Wed, Aug 25, 2021 at 12:02 PM Nikita Popov  wrote:

> 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.
>
> Regards,
> Nikita
>

Based on the received feedback, I've updated the RFC to instead provide an
#[AllowDynamicProperties] attribute as a way to opt-in to the use of
dynamic properties. As previously discussed, this won't allow us to
completely remove dynamic properties from the language model anymore, but
it will make the upgrade path smoother by avoiding multiple inheritance
issues. Especially given recent feedback on backwards-incompatible changes,
I think it's prudent to go with the more conservative approach.

Regards,
Nikita


[PHP-DEV] [VOTE] Deprecate partially supported callables

2021-10-08 Thread Nikita Popov
Hi internals!

I've opened voting on
https://wiki.php.net/rfc/deprecate_partially_supported_callables, which
deprecated callables that are not supported in $callable() syntax. The vote
closes on 2021-10-22.

Regards,
Nikita


Re: [PHP-DEV] Change yield interpolation behaviour

2021-10-08 Thread Nikita Popov
On Fri, Oct 8, 2021 at 10:55 AM Nesmeyanov Kirill  wrote:

> Hello Internals!
>
>
> At the moment, there is a feature/bug in the PHP that allows to use
> interpolation of generators.
>
> ```
>
> $code = <<
>  Hello ${yield}
>
> EXAMPLE;
>
> ```
>
> I suspect that initially this functionality was not thought out, but it
> partially works, which allows you to implement useful functionality.
>
> ```
>
> [$query, $params] = sql(fn() => <<
>  SELECT * FROM users WHERE id = ${yield 42} OR id = ${yield 0xDEADBEEF}
>
> SQL);
>
> // Expected
> // $query = "SELECT * FROM users WHERE id = ? OR id = ?"
> // $params = [ 42, 0xDEADBEEF ]
>
> ```
>
> When I say that the functionality was not thought out initially, I mean
> the behavior of generators within strings. For example, the following
> code, which should (seemingly) implement this functionality:
>
> ```
>
> function sql(\Closure $expr)
>
> {
>
>  [$generator, $params] = [$expr(), $params];
>
>  while ($generator->valid()) {
>
> $params[] = $generator->current(); // Get the value from "yield"
>
> $generator->send('?'); // Insert placeholder
>
>  }
>
>  return [$generator->getReturn()];
>
> }
>
> ```
>
> Causes an error:
>
> ```
>
> Warning: Undefined variable $?
>
> ```
>
>
> That is, the expression "${yield 42}" expects back not the result of
> this expression, but the name of the variable. Therefore, a complete and
> workable implementation of such a functionality is as follows:
> https://gist.github.com/SerafimArts/2e7702620480fbce6c24bc87bfb9cb0e
>
>
> I think it makes sense to do something about it. I have two suggestions:
>
> 1) Forbid using "yield" inside strings
>
> 2) Expect not a variable name as a result of this expression, but a
> substitution value.
>

This doesn't really have anything to do with yield. ${expr} is PHP's
general variable-variable syntax, which looks up the variable with name
returned by expr. The syntax also works inside strings in the form of
"${expr}". Using "${yield $v}" is just a specific instance of the general
pattern following the same rules. (This syntax has a special case that
should be deprecated: "${label}" will be interpreted the same as "$label"
instead, which is inconsistent with how it works everywhere else. This is
also why ${yield} without argument will not perform a yield and instead
look for a variable called $yield.)

PHP unfortunately doesn't have a general expression interpolation syntax,
you can only interpolate variables and certain variable-like constructs.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Allow null as standalone type

2021-10-05 Thread Nikita Popov
On Tue, Oct 5, 2021 at 4:08 PM Côme Chilliet  wrote:

> Le lundi 4 octobre 2021, 10:09:12 CEST Nikita Popov a écrit :
> > If we make this change, I would however suggest to also support "false"
> as
> > a standalone type. I think this change primarily has benefits from a
> > typesystem completeness perspective rather than a strong practical need.
> > From that angle, it would be nice if all types that are usable in a union
> > are also usable as standalone types, rather than shifting the special
> case
> > from null to false.
>
> It feels weird/wrong to slowly add values in the type system like this,
> rather than directly supporting static values as type hints.
>
> Why would function a(): null|false {} be legal but function b(): null|0
> would not?
>
> This is inconsistent to me. And adding null, then false, then true for the
> sake of completeness feels like avoiding to treat the static value as type
> hint subject.
>

Both null and false are already part of the type system. They're not being
added, neither by the RFC in question, nor by my quoted suggestion. This
discussion is only about relaxing restrictions on standalone types.

So to answer your question: "null|false" would be legal because
"string|false" already is. "null|0" would be illegal because there is no
such thing as a "0" type (in our current type system).

Regards,
Nikita


Re: [PHP-DEV] Adding `final class Deque` to PHP

2021-10-05 Thread Nikita Popov
On Tue, Oct 5, 2021 at 12:47 AM tyson andre 
wrote:

> Hi Nikita Popov,
>
> > 1. There would be the possibility of having an interface Deque that is
> > backed by a VecDeque/ArrayDeque implementation. I'm not convinced this
> > would actually make sense, but I wanted to throw it out there, given that
> > the class is final (which is without any doubt the correct decision).
>
> Do you mean ArrayDeque for a hardcoded max capacity?
> I'm also not convinced there's a use case.
>
> > 2. How do set() / offsetSet() work with an out of range index? Is it
> > possible to push an element with $deque[count($deque)] = x? I would
> assume
> > that throws, but want to make sure. On a related note, why do we need
> both
> > get()/set() and offsetGet()/offsetSet()? These APIs seem redundant.
>
> It isn't possible to set an out of range index - both throws
> `\OutOfBoundsException`
>
> In order to support the ArrayAccess `$deque[$offset] = $value;` or
> `$deque[$offset]` shorthand,
> the offsetGet/offsetSet needed to be implemented to follow conventions.
> (because of ArrayAccess, offsetGet must accept mixed offsets)
>
> Those aren't type safe, though, so get()/set() are provided for a type
> safe alternative
> that will throw a TypeError for use cases that would benefit from runtime
> type safety.
>

offsetGet() and offsetSet() can (and should) still throw if the offset is
not an integer. We just can't actually declare that type. This is a
weakness of the PHP type system, so nominally "violating" LSP is fine here.


> > 3. I believe it's pretty standard for Deque implementations to provide
> > insert() / remove() methods to insert at any position (these would be
> O(n)).
>
> https://www.php.net/manual/en/class.splqueue.php didn't offer that
> functionality.
>
> https://www.php.net/manual/en/class.ds-deque.php did, apparently.
>
> > 4. The design of getIterator() here is rather unusual, and deserves some
> > additional justification. Normally, we let getIterator() see concurrent
> > modifications to the structure, though the precise behavior is
> unspecified.
> > I would also like to know how this will look like on a technical level
> (it
> > doesn't seem to be implemented yet?) This seems like something that will
> > require a check on every write operation, to potentially separate the
> > structure in some form.
>
> The original plan was to copy the entire array on iterator creation,
> to imitate the immediate copy nature of foreach on arrays.
>

There is no immediate copy with arrays though. The copy is just a refcount
increment and no actual copy will happen unless the array is modified
during iteration, which is a rare case. I'd expect the Deque implementation
(with those semantics) to do the same -- but we don't have a convenient
mechanism for it. Adding an indirection and a refcount to the underlying
storage would be possible, but also feels like overkill. The other approach
I see would be to track all the registered iterators, so we can explicitly
separate them on write.


> This was assuming that `foreach` over a Deque without removing elements
> would be a rare use case.
>
> That may have been a mistake since `foreach (clone $deque as $key =>
> $value)` can be done to explicitly do that.
> There're around 4 approaches I could take with different tradeoffs
>
> 1. Iterate over $offset = 0 and increment offset in calls to
> InternalIterator->next() until exceeding the size of the deque, not copying
> the deque.
>
>That's the **actual** current implementation, but would be unintuitive
> with shift()/unshift()
>
>This would repeat elements on unshift(), or skip over elements when
> shift() is called.
>
>The current implementation of `Ds\Deque` seems to do the same thing,
> but there's a similar discussion in its issue tracker in
> https://github.com/php-ds/ext-ds/issues/17
>
>
> 2. Similar iteration behavior, but also have it relative to a uint64
> indicating the number of times elements were added to the front of the
> deque minus the number of elements were removed from the back of the deque.
>
> (e.g. if the current Deque internalOffset is 123, the InternalIterator
> returned by getOffset would start with an offset of 123 and increase it
> when advancing.
> Calls to shift would raise the Deque internalOffset, calls to unshift
> would decrease it (by the number of elements).
> This would be independent of the circular buffer offset.
> And the InternalIterator would increase the internalOffset to catch up
> if the signed offset difference became negative, e.g. by calling shift()
> twice in a foreach block)
>

Something to keep in mind here is that there migh

[PHP-DEV] Re: [RFC] Deprecate partially supported callables

2021-10-04 Thread Nikita Popov
On Thu, Sep 2, 2021 at 4:32 PM Nikita Popov  wrote:

> Hi internals,
>
> Currently, there are some callables that are accepted by the "callable"
> type, but which cannot be used with $callable() syntax. This RFC proposes
> to deprecate support for such "callables":
>
> https://wiki.php.net/rfc/deprecate_partially_supported_callables
>

Is there any further feedback on this proposal? Otherwise I'd open the vote
in a few days.

Regards,
Nikita


Re: [PHP-DEV] Adding `final class Deque` to PHP

2021-10-04 Thread Nikita Popov
On Tue, Sep 21, 2021 at 11:05 PM Levi Morrison via internals <
internals@lists.php.net> wrote:

> The name "deque" is used in the standard library of these languages:
>
>  - C++: std::deque
>  - Java: java.util.Deque (interface)
>  - Python: collections.deque
>  - Swift: Collections.Deque (not standard lib, apparently, but Apple
> official? Don't know Swift)
>  - Rust: std::collections::VecDeque
>
> And these don't have it in the standard library:
>  - Go
>  - C#
>  - C
>  - JavaScript
>
> Anyway, it's pretty decent evidence that:
>  1. This functionality is pretty widely used across languages.
>  2. This functionality should have "deque" be in the name, or be the
> complete name.
>
> Discussion naming for "vector" I can understand, as it's less widely
> used or sometimes means something specific to numbers, but there's
> little point in discussing the name "deque." It's well established in
> programming, whether PHP programmers are aware of it or not.
>
> As I see it, the discussion should be centered around:
>  1. The API Deque provides.
>  2. The package that provides it.
>  3. Whether Deque's API is consistent with other structures in the same
> package.
>  4. Whether this should be included in php-src or left to extensions.
>
> I suggest that we try to make PHP as homogenous in each bullet as we
> can with other languages:
>  1. Name it "Deque."
>  2. Put it in the namespace "Collections" (and if included in core,
> then "ext/collections").
>  3. Support common operations on Deque (pushing and popping items to
> both front and back, subscript operator works, iteration, size, etc)
> and add little else (don't be novel here). To me, the biggest thing
> left to discuss is a notion of a maximum size, which in my own
> experience is common for circular buffers (the implementation chosen
> for Deque) but not all languages have this.
>  4. This is less clear, but I'm in favor as long as we can provide a
> few other data structures at the same time. Obviously, this a voter
> judgement call on the yes/no.
>
> Given that I've suggested the most common options for these across
> many languages, it shouldn't be very controversial. The worst bit
> seems like picking the namespace "Collections" as this will break at
> least one package: https://github.com/danielgsims/php-collections. We
> should suggest that they vendor it anyway, as "collections" is common
> e.g. "Ramsey\Collections", "Doctrine\Common\Collections". I don't see
> a good alternative here to "Collections", as we haven't agreed on very
> much on past namespace proposals.
>
> Anyway, that's what I suggest.
>

I generally agree with everything Levi has said here. I think that adding a
deque structure generally makes sense, and that putting it into a new
ext/collections extension under the corresponding namespace would be
appropriate. I wouldn't insist on introducing it together with other
structures (I'm a lot more sceptical about your Vector proposal), but do
think there should be at least some overarching plan here, even if we only
realize a small part of it in this version.

A few questions for the particular API proposed here:

1. There would be the possibility of having an interface Deque that is
backed by a VecDeque/ArrayDeque implementation. I'm not convinced this
would actually make sense, but I wanted to throw it out there, given that
the class is final (which is without any doubt the correct decision).

2. How do set() / offsetSet() work with an out of range index? Is it
possible to push an element with $deque[count($deque)] = x? I would assume
that throws, but want to make sure. On a related note, why do we need both
get()/set() and offsetGet()/offsetSet()? These APIs seem redundant.

3. I believe it's pretty standard for Deque implementations to provide
insert() / remove() methods to insert at any position (these would be O(n)).

4. The design of getIterator() here is rather unusual, and deserves some
additional justification. Normally, we let getIterator() see concurrent
modifications to the structure, though the precise behavior is unspecified.
I would also like to know how this will look like on a technical level (it
doesn't seem to be implemented yet?) This seems like something that will
require a check on every write operation, to potentially separate the
structure in some form.

5. The shift/unshift terminology is already used by our array API, but I'm
not sure it's the most intuitive choice in the context of a deque. SplQueue
has enqueue() and dequeue(). Another popular option from other languages
(which I would personally favor) is pushFront() and popFront().

Regards,
Nikita


Re: [PHP-DEV] [RFC] Random Extension 3.0

2021-10-04 Thread Nikita Popov
On Thu, Sep 9, 2021 at 6:31 AM Go Kudo  wrote:

> Hi Nikita, sorry for the late reply.
>
> This is a difficult problem. For now, MT19937 is left for compatibility.
> In other words, if you don't need compatibility, there is no benefit to
> using it.
>
> What about implementing both a new MT and a compatible MT? A compatible MT
> would have the following signature
>
> ```php
>
> use Random\NumberGenerator\Numbergenerator;
>
> /* for legacy compatibility */
> class MT19937 implements NumberGenerator
> {
> public function __construct(int $mode = MT_RAND_MT19937, int $seed) {}
> }
>
> /* a new implementation */
> class MersenneTwister implements NumberGenerator
> {
> public function __construct(int $seed) {}
> }
> ```
>
> I had originally removed the MT_RAND_PHP implementation on the grounds
> that legacy implementations should not be retained, but if the regular
> Mersenne twister is to be retained for compatibility, I think it should be
> as well.
>
> Also, maybe we should opt for a more SIMD friendly RNG.
>

To clarify, what I had in mind here is not the MT generator itself, but the
scaling done in Random::getInt(). This scaling is independent of the used
source. So while the raw numbers generated by
Random\NumberGenerator\MT19937 would be the same as before, the result
produced by Random::getInt() with this source wouldn't be.

Regards,
Nikita


> 2021年9月7日(火) 17:28 Nikita Popov :
>
>> On Thu, Sep 2, 2021 at 5:10 PM Go Kudo  wrote:
>>
>>> Hi Internals.
>>>
>>> Expanded from the previous RFC and changed it to an RFC that organizes
>>> the
>>> whole PHP random number generator. Also, the target version has been
>>> changed to 8.2.
>>>
>>> https://wiki.php.net/rfc/rng_extension
>>> https://github.com/php/php-src/pull/7453
>>>
>>> Hopefully you will get some good responses.
>>>
>>
>> This RFC currently tries to keep some semblance of compatibility with the
>> existing mt_rand() algorithm by retaining the same implementation for
>> mapping the raw random number stream into a range. However, the algorithm
>> we use for that is not exactly state of the art and requires two full-width
>> divisions at minimum. There are more efficient scaling algorithms based on
>> fixed-point multiplication, which are "nearly divisionless" (
>> https://arxiv.org/pdf/1805.10941.pdf). The variant at
>> https://github.com/apple/swift/pull/39143 is entirely divisionless.
>>
>> Doing this would improve performance (though I'm not sure by how much --
>> maybe once we add up our call overhead, it isn't important anymore?) but it
>> would provide a different sequence than mt_rand(). Something we might want
>> to consider.
>>
>> Regards,
>> Nikita
>>
>


Re: [PHP-DEV] [Pre-Vote Announcement] Move RNG functions Random Extension

2021-10-04 Thread Nikita Popov
On Wed, Sep 22, 2021 at 1:21 PM Go Kudo  wrote:

> The voting phase for the following RFCs will begin as soon as two weeks
> have passed.
>
> https://externals.io/message/115975
>
> I don't see any particular discussion, so I'm contacting you just in case.
>

I'm pretty neutral on this proposal. I don't think there's a strong
motivation to move this functionality into a separate extension, but I also
don't see any particular problems with doing it (as long as it's an
always-required extension, which this is).

I believe Levi is a proponent of splitting up ext/standard into smaller
extensions, maybe he has some thoughts on this.

Regards,
Nikita


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

2021-10-04 Thread Nikita Popov
On Thu, Sep 23, 2021 at 8:32 AM Tim Starling 
wrote:

> Please consider my RFC for locale-independent case conversion.
>
> https://wiki.php.net/rfc/strtolower-ascii
> https://github.com/php/php-src/pull/7506
>
> The RFC and associated PR ended up going some way beyond the original
> scope, because for consistency, it's best if everything has the same
> concept of case folding. I saw this as an opportunity to clean up a
> common kind of locale-dependence in PHP which was previously inconsistent.
>
> So not only will strtolower() and strtoupper() become
> locale-independent, converting only ASCII, but also stristr, stripos,
> strripos, lcfirst, ucfirst, ucwords, str_ireplace, the array sorting
> functions with SORT_FLAG_CASE, and array_change_key_case.
>
> Also, I changed a number of internal functions to use ASCII case
> folding, giving rise to a range of effects in callers throughout the
> core tree. The effects are all documented in the RFC.
>
> I am proposing that locale-sensitive case conversion be provided with
> the new names ctype_tolower() and ctype_toupper(). Those names might
> seem odd at first glance, but they are wrappers for functions in
> ctype.h and work in a very similar way to the rest of the ctype extension.
>

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)
and then fail because it doesn't support UTF-8.

Regards,
Nikita

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.


Re: [PHP-DEV] Proposal: Shorthand initialization and destructuring of associative arrays

2021-10-04 Thread Nikita Popov
On Mon, Sep 27, 2021 at 11:27 AM Konrad Baumgart  wrote:

> Hi everyone,
>
> I'd like to propose 2 syntactic sugars:
> $array = [ => $data]; // the same as $array = ['data' => $data]
> and
> [ => $data] = $array; // the same as ['data' => $data] = $array
>
> My biggest use-case for this would be conveniently returning multiple
> things from a function, like:
>
> function getDataForDashboard() {
>   …
>   return [ => $dailyAggregations, => $weeklyAggregations];
> }
>
> [ => $dailyAggregations, => $weeklyAggregations] = getDataForDashboard();
>
> Similar effects can be achieved with compact()/extract() functions,
> but I dislike those functions because they make it hard to find usages
> of variables. Using numerical arrays instead makes the code less
> readable and more error-prone for me. Using ['dailyAggregations' =>
> $dailyAggregations,…] would force you to split code into multiple
> lines and negatively affect readability.
>
> I was recently developing with js/ts and I liked the ease of returning
> multiple items from a function as an object, while still preserving
> their name.
>
> I have spare time this October, so I would happily get into php
> interpreter by developing this.
>
> I'm looking forward for your feedback,
> Konrad Baumgart
>

I'm generally in favor of this functionality. Unfortunately the syntax
options we have for this are not as nice as in other languages. [=> $x] is
probably the most reasonable choice where array literals are concerned.
https://wiki.php.net/rfc/named_params#shorthand_syntax_for_matching_parameter_and_variable_name
also has some thoughts on this topic, as named params have the same problem.

Regards,
Nikita


Re: [PHP-DEV] Allowing `(object)['key' => 'value']` in initializers?

2021-10-04 Thread Nikita Popov
On Sat, Sep 25, 2021 at 5:45 PM tyson andre 
wrote:

>
> Hi internals,
>
> In PHP 8.1, it is possible to allow constructing any class name in an
> initializer, after the approval of
> https://wiki.php.net/rfc/new_in_initializers
>
> ```
> php > static $x1 = new ArrayObject(['key' => 'value']);
> php > static $x2 = new stdClass();
> php > static $x3 = (object)['key' => 'value'];
>
> Fatal error: Constant expression contains invalid operations in php shell
> code on line 1
> ```
>
> What are your thoughts on allowing the `(object)` cast in initializer
> types where `new` was already allowed, but only when followed by an array
> literal node. (e.g. continue to forbid `(object)SOME_CONSTANT`) (see
> https://wiki.php.net/rfc/new_in_initializers)
>
> stdClass has never implemented a factory method such as `__set_state`
> (which is not yet allowed). Instead, `(object)[]` or the `(object)array()`
> shorthand is typically used when a generic object literal is needed. This
> is also how php represents objects in var_export.
>
> ```
> php > var_export(new stdClass());
> (object) array(
> )
> ```
>
> Reasons:
> - The ability to construct empty stdClass instances but not non-empty ones
> is something users would find surprising,
>   and a lack of support for `(object)[]` be even more inconsistent if
> factory methods were allowed in the future.
> - stdClass is useful for some developers, e.g. in unit tests, when using
> libraries requiring it for parameters,
>   when you need to ensure data is encoded as a JSON `{}` rather than `[]`,
> etc.
> - It would help developers write a clearer api contract for methods,
>   e.g. `function setData(stdClass $default = (object)['key' => 'value'])`
>   is clearer than `function setData(?stdClass $default = null) { $default
> ??= (object)['key' => 'value']; `
> - stdClass may be the only efficient built-in way to represent objects
> with arbitrary names if RFCs such as https://externals.io/message/115800
>   passed
>

I'm not super convinced about the usefulness of (object)[] in particular,
but I also think that we shouldn't artificially limit the types of
expressions supported in constant expressions -- if there's no strong
reason why something should be forbidden, it should be allowed.

>From that perspective, I think the root issue here is that constant
expressions currently don't support casts at all. It's not just a matter of
being unable to write (object)[], you also can't write (int)X (but you can
write +X). I think it's perfectly reasonable to support casts in constant
expressions, and if we do, then I don't think we need to go out of the way
to forbid object casts either, so support for (object)[] should just fall
out as a special case.

Regards,
Nikita


Re: [PHP-DEV] Static (factory) methods in attributes and initializers

2021-10-04 Thread Nikita Popov
On Mon, Sep 27, 2021 at 5:58 PM Andreas Hennings 
wrote:

> Hello list,
>
> currently, the default mode for attributes is to create a new class.
> For general initializers, with
> https://wiki.php.net/rfc/new_in_initializers we get the option to call
> 'new C()' for parameter default values, attribute arguments, etc.
>
> Personally I find class construction to be limiting, I often like to
> be able to use static factories instead.
> This allows:
> - Alternative "constructors" for the same class.
> - A single constructor can conditionally instantiate different classes.
> - Swap out the class being returned, without changing the factory name
> and signature.
>
> In fact, static factories for initializers were already mentioned in
> "Future Scope" in https://wiki.php.net/rfc/new_in_initializers.
> However this does not mention static factories for the main attribute
> object.
>
> For general initializers this is quite straightforward.
> For attributes, we could do this?
>
> // Implicitly call new C():
> #[C()]
> # Call the static factory instead:
> #[C::create()]
>
> So the only difference here would be that in the "traditional" case we
> omit the "new " part.
>
> We probably want to guarantee that attributes are always objects.
> We can only evaluate this when somebody calls ->newInstance(), because
> before that we don't want to autoload the class with the factory. So
> we could throw an exception if the return value is something other
> than an object.
> I was also considering to require an explicit return type hint on the
> factory method, but again this can only be evaluated when somebody
> calls ->newInstance(), so the benefit of that would be limited.
>
> The #[Attribute] annotation would allow methods as target.
>
> Reflection:
>
> ::getArguments() -> same as before.
> ::getName() -> returns "$class_qcn::$method_name".
> ::getTarget() -> same as before.
> ::isRepeated() -> This is poorly documented on php.net, but it seems
> to just look for other attributes with the same ->getName(). So it
> could do the same here.
> ::newInstance() -> calls the method. Throws an error if return value
> is non-object.
>
> we could add more methods like ReflectionAttribute::isClass() or
> ReflectionAttribute::isMethod(), or a more generic ::getType(), but
> these are not absolutely required.
>
> We could also allow regular functions, but this would cause ambiguity
> if a class and a function have the same name. Also, functions cannot
> be autoloaded, so the benefit would be small. I'd rather stick to just
> methods.
>

I see where you're coming from here, and I think an argument could be made
that our attribute syntax should have been #[new C()], allowing us to
associate arbitrary values -- which would naturally allow the use of static
factory methods once/if they are supported in constant expressions.

As that particular ship has sailed, I'm not convinced that supporting
static factory methods as "attributes" would be worthwhile. It's a
significant complication to the system (that users need to be aware of, and
consumers of the reflection API need to handle) for what ultimately seems
like a personal style choice to me. Do you have any examples where using
static factories over constructors for attributes would be particularly
compelling?

Regards,
Nikita


Re: [PHP-DEV] Spam on bugs.php.net

2021-10-04 Thread Nikita Popov
On Mon, Oct 4, 2021 at 1:20 AM Stanislav Malyshev 
wrote:

> Hi!
>
> I notice that we have increased amount of spam coming in to
> bugs.php.net. I'm not sure if anyone is maintaining it right now - but
> it'd be nice to have changes to counter that - I see that anti-spam
> measures exist on some forms but not on adding comments to closed bugs
> for some reason. And also maybe add ability to delete comments at least
> for some people, so it can be cleaned up.
>

Yes, our spam problem on bugs.php.net is getting worse over time. It's
already possible to delete comments (feel free to add yourself to
https://github.com/php/web-bugs/blob/master/include/trusted-devs.php to
enable it) and I tend to delete a handful on a good day (on one
particularly bad day, I had to do a mass delete from the DB).

I don't think anyone maintains bugs.php.net -- the spam problem (both link
spam and actively malicious users) is part of the motivation to switch to
GitHub Issues (which requires authentication and thus can be moderated
effectively).

Regards,
Nikita


Re: [PHP-DEV] Unified ReflectionType methods

2021-10-04 Thread Nikita Popov
On Sun, Oct 3, 2021 at 3:42 PM G. P. B.  wrote:

> On Sat, 2 Oct 2021 at 00:54, Andreas Hennings  wrote:
>
> > Hello list,
> > I would like to propose new methods for ReflectionType, that would
> > allow treating ReflectionNamedType and ReflectionUnionType in a
> > unified way.
> > This would eliminate the need for if (.. instanceof) in many use cases.
> >
> > Some details can still be discussed, e.g. whether 'null' should be
> > included in builtin type names, whether there should be a canonical
> > ordering of type names, whether we should use class names as array
> > keys, etc.
> >
> > [...]
> >
> > What do you think?
> >
> > -- Andreas
>
>
> I don't think this is a good idea, at least in a form like this.
> I understand that the ReflectionType API is cumbersome but I don't think
> merging everything together makes a lot of sense.
> Want does need to know if one is dealing with a Union, Intersection, or
> single type and conflating the APIs is IMHO a bad idea as it doesn't
> consider future extensions.
> And I am not talking about more complex composite types such as
> (A)|C|(X), as those will be handled within the existing design, but
> potential additions to the type system like function types, literal types,
> generics, etc.
>
> I also don't really understand what the argument for removing instanceof
> checks is other than more generic code which works, but if this breaks at
> the next type system addition, this seems rather pointless.
>

Agree, I don't think this suggestion makes sense. The current
ReflectionType hierarchy is specifically designed so you have to do those
instanceof checks and deal with the different kinds of types we support.
The ReflectionType hierarchy isn't complex for fun, it directly reflects
the complexity of the type system. As the type system becomes more
complicated, there will be more kinds of types that are exposed through
reflection and need to be handled appropriately.

I am fine with additions as Tyson suggests them: Methods that work across
*all* ReflectionTypes. The proposed methods do not fall in this category,
as they don't hold up for intersection types in PHP 8.1 already, let alone
future type system additions.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Allow null as standalone type

2021-10-04 Thread Nikita Popov
On Mon, Oct 4, 2021 at 5:33 AM Levi Morrison via internals <
internals@lists.php.net> wrote:

> On Sat, Oct 2, 2021 at 9:07 AM G. P. B.  wrote:
> >
> > Hello internals,
> >
> > I'm proposing a new RFC to make 'null' usable as a standalone type.
> >
> > RFC: https://wiki.php.net/rfc/null-standalone-type
> > GitHub PR: https://github.com/php/php-src/pull/7546
> >
> > Best regards,
> >
> > George P. Banyard
>
> I don't see the word `void` in the RFC. I think there ought to be
> something said about how naked `null` is different or not different
> than `void`.
>

Right. To quote the original union types RFC, this was the motivation for
the current limitation:

> The null type is only allowed as part of a union, and can not be used as
a standalone type. Allowing it as a standalone type would make both
function foo(): void and function foo(): null legal function signatures,
with similar but not identical semantics. This would negatively impact
teachability for an unclear benefit.

I'm not opposed to making null usable as a standalone type though. I think
the fact that a "void" function must use "return;" instead of "return
null;" and a "null" function conversely must use "return null;" instead of
"return;" will probably be sufficient disambiguation.

If we make this change, I would however suggest to also support "false" as
a standalone type. I think this change primarily has benefits from a
typesystem completeness perspective rather than a strong practical need.
>From that angle, it would be nice if all types that are usable in a union
are also usable as standalone types, rather than shifting the special case
from null to false.

Regards,
Nikita


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

2021-09-23 Thread Nikita Popov
On Wed, Sep 22, 2021 at 3:30 PM Matthew Weier O'Phinney <
mweierophin...@gmail.com> wrote:

> Yesterday, I opened an issue regarding a change in the pgsql extension (
> https://bugs.php.net/bug.php?id=81464).
>
> PHP 8.0 introduced the concept of "resource objects". Where previously we
> would have resources, and use `get_resource_type()` when we needed to
> differentiate various resources, resource objects give us immutable objects
> instead that allow us to type hint. Personally, I think this is wonderful!
>
> The rollout for 8.0 was incomplete, however, and only touched on something
> like 4-6 different resource types. Still, a good start.
>
> With PHP 8.1, we're seeing the addition of more of these, and it was
> encountering one of those changes that prompted the bug I previously linked
> to.
>
> Here's the issue: while overall, I like the move to resource objects,
> introducing them in a MINOR release is hugely problematic.
>
> Previously, you would do constructs such as the following:
>
> if (! is_resource($resource) || get_resource_type($resource) !==
> $someSpecificType) {
> // skip a test or raise an exception
> }
>
> Resource objects, however:
>
> - Return `false` for `is_resource()` checks.
> - Raise a warning for `get_resource_type()` checks, and/or report the
> resource object class name — which differs from the previous resource names
> in all cases.
>
> This means conditionals like the above BREAK. As a concrete example, I did
> PHP 8.1 updates for laminas-db last week, and assumed our postgres
> integration tests were running when we finally had all tests passing
> successfully. However, what was really happening was that our test suite
> was testing with `is_resource()` and skipping tests if resources were not
> present. We shipped with broken pgsql support as a result, and it wasn't
> until test suites in other components started failing that we were able to
> identify the issue.
>
> Further, the "fix" so that the code would work on both 8.1 AND versions
> prior to 8.1 meant complicating the conditional, adding a `! $resource
> instanceof \PgSql\Connection` into the mix. The code gets unwieldy very
> quickly, and having to do this to support a new minor version was
> irritating.
>
> When I opened the aforementioned bug report, it was immediately closed as
> "not an issue" with the explanation that it was "documented in UPGRADING".
>
> This is not an acceptable explanation.
>
> - There was no RFC related to 8.1 indicating these changes were happening.
> (In fact, there was no RFC for resource objects in the first place — which
> is concerning considering the BC implications!)
> - In semantic versioning, existing APIs MUST NOT change in a new minor
> version, only in new major versions.
>
> Reading the UPGRADING guide, there's a HUGE section of backwards
> incompatible changes for 8.1 — THIRTY-FOUR of them. Nested in these are
> notes of around a half-dozen extensions that once produced resources now
> producing resource objects.
>
> The pace of change in PHP is already breathtaking when you consider large
> projects (both OSS and in userland); keeping up with new features is in and
> of itself quite a challenge. Introducing BC breaks in minor versions makes
> things harder for everyone, as now you have to figure out not only if
> there's new features you want to adopt, but whether or not there are
> changes that will actively break your existing code. I strongly feel that
> anything in the backwards incompatible section of the UPGRADING guide
> should be deferred to 9.0, when people actually expect things to change.


I believe the changes in PHP 8.1 with the highest migration burden for
open-source libraries are the additional of tentative return types (aka
"put #[ReturnTypeWillChange] everywhere") and deprecation of null arguments
to internal functions, followed by the float to int precision-loss
deprecation and, depending on project, the Serializable deprecation.

What all of these have in common, is that they are all semver compliant
changes, because they "only" introduce deprecations. Deprecations are
explicitly not considered backwards-compatibility breaks.

Now, there are two problems with this picture: The first one is that
deprecations often get promoted to exceptions by generic error handlers. I
believe that this continues to be the default behavior of PHPUnit for
example. This means that in practice, deprecations do break code, even
though they are intended not to.

The second one is that this does not really hold up for open-source
libraries. At the application layer, you can suppress all deprecations, and
call it a day. At the library layer, the usual perception is that if your
library throws deprecation warnings, it's not compatible with the given
version of PHP. Taken in conjunction with deprecation to exception
promotion (in test suites if nothing else) there is some truth to that
perception.

While deprecations are intended as a backwards-compatible 

Re: [PHP-DEV] run-tests SKIPIF caching can be problematic for third-party extensions

2021-09-23 Thread Nikita Popov
On Thu, Sep 23, 2021 at 4:10 PM Nikita Popov  wrote:

> On Wed, Sep 15, 2021 at 7:23 PM Jeremy Mikola  wrote:
>
>> I just discovered that run-tests.php was changed to cache SKIPIF
>> evaluation
>> since 8.1.0beta3[1]. I believe ext-mongodb ran into the same issue as
>> mysqli[2], as we use SKIPIF to check that database contents are clean
>> going
>> into a test. The present solution in core is to check SKIPIF output for
>> "nocache" [3,4] and allow individual tests to opt out of caching; however,
>> I'm worried that won't be portable for third-party extensions, where we
>> test with run-tests.php for each version of PHP that we support.
>>
>> Is there still time to consider an alternative solution? If "nocache"
>> needs
>> to remain in place for mysqli, perhaps 8.1.0's run-tests.php can be
>> enhanced to consult an environment variable (for maximum BC and playing
>> nice with `make test`) that disables caching entirely.
>>
>
> I'm not sure I understand the setup you're using. The "nocache" tag that
> mysqli uses covers the situation where part of the test setup is done in
> the SKIPIF section -- because checking whether the test should be skipped
> requires doing the same setup as the actual test. If the test does get
> skipped, it's still fine to cache that.
>
> Now, it sounds like your situation is different from that, and you
> actually have SKIPIF sections where first one test should be skipped, but
> then a later test with identical SKIPIF shouldn't be skipped. Is that
> correct? What changes between the time these two tests run?
>

Independently of that question (which is about whether "nocache" is
sufficient if we ignore cross-version compatibility issue) I do agree that
an option to disable the skip cache entirely would make sense. Would
https://github.com/php/php-src/pull/7510 work for you?

Regards,
Nikita


Re: [PHP-DEV] run-tests SKIPIF caching can be problematic for third-party extensions

2021-09-23 Thread Nikita Popov
On Wed, Sep 15, 2021 at 7:23 PM Jeremy Mikola  wrote:

> I just discovered that run-tests.php was changed to cache SKIPIF evaluation
> since 8.1.0beta3[1]. I believe ext-mongodb ran into the same issue as
> mysqli[2], as we use SKIPIF to check that database contents are clean going
> into a test. The present solution in core is to check SKIPIF output for
> "nocache" [3,4] and allow individual tests to opt out of caching; however,
> I'm worried that won't be portable for third-party extensions, where we
> test with run-tests.php for each version of PHP that we support.
>
> Is there still time to consider an alternative solution? If "nocache" needs
> to remain in place for mysqli, perhaps 8.1.0's run-tests.php can be
> enhanced to consult an environment variable (for maximum BC and playing
> nice with `make test`) that disables caching entirely.
>

I'm not sure I understand the setup you're using. The "nocache" tag that
mysqli uses covers the situation where part of the test setup is done in
the SKIPIF section -- because checking whether the test should be skipped
requires doing the same setup as the actual test. If the test does get
skipped, it's still fine to cache that.

Now, it sounds like your situation is different from that, and you actually
have SKIPIF sections where first one test should be skipped, but then a
later test with identical SKIPIF shouldn't be skipped. Is that correct?
What changes between the time these two tests run?

Regards,
Nikita


Re: [PHP-DEV] Make strtolower/strtoupper just do ASCII

2021-09-17 Thread Nikita Popov
On Fri, Sep 17, 2021 at 12:07 PM Tim Starling 
wrote:

> On 17/9/21 7:15 pm, Kamil Tekiela wrote:
> > +1 from me. I wasn't even aware that these functions are
> > locale-dependent until recently. I see an added benefit that we could
> > add them to the optimizer once they are no longer locale-dependent.
> > What would happen to users who really need the locale-dependent
> > functions? Do we offer some workarounds?
>
> We could add a global mode, although that would prevent constant
> propagation, if that's what you mean by adding them to the optimizer.
> Or we could add variant functions like locale_strtolower() and
> locale_strtoupper(). But I think I would want to hear from someone who
> uses locale-dependence so I can understand what their needs are. I
> guess the RFC will sort that out.
>

I would expect that in nearly all cases the replacement would be one of
these:
1. You were using an UTF-8 locale (which you likely are), then just keep
using strtolower(). Without having checked all the details here, I think
strtolower() under UTF-8 locales already effectively behaves like ASCII
lowercase, because it skips continuation bytes.
2. If you were using some other charset, then using mb_strtolower() with
that charset should work. So if you were using de_DE.ISO8859-1, then using
mb_strtolower() with "ISO8859-1" encoding would be the replacement.

As a matter of general policy, it is unlikely that we will accept an option
(whether that be an ini option or something else) to control this behavior.
We can make the change or not make it, but not both ;)

Regards,
Nikita


Re: [PHP-DEV] Make strtolower/strtoupper just do ASCII

2021-09-17 Thread Nikita Popov
On Fri, Sep 17, 2021 at 4:59 AM Tim Starling 
wrote:

> I would like to know if a patch to make strtolower and strtoupper do
> plain ASCII case conversion would be accepted, or if an RFC should be
> created.
>
> The situation with case conversion is inconsistent.
>
> The following functions do ASCII case conversion: strcasecmp,
> strncasecmp, substr_compare.
>
> The following functions do locale-dependent case conversion:
> strtolower, strtoupper, str_ireplace, stristr, stripos, strripos,
> strnatcasecmp, ucfirst, ucwords, lcfirst.
>
> I would make them all do ASCII case conversion.
>
> Developers need ASCII case conversion, because it is used internally
> by PHP for things like class name comparison, and because it is a
> specified algorithm in HTML 5 and related standards.
>
> The existing options for ASCII case conversion are:
>
> * Never call setlocale(). But this breaks non-ASCII characters in
> escapeshellarg() and can't be guaranteed in a library.
>
> * Call setlocale(LC_ALL, "C.UTF-8"). But this is non-portable and also
> can't be guaranteed in a library.
>
> * Use strtr(). But this is ugly and slow.
>
> If mbstring has a way to do it, I can't find it. I tested
> mb_strtolower($s, '8bit') and mb_strtolower($s,'ascii').
>
> Note that locale-dependent case conversion is almost never a useful
> feature. Strings are passed through tolower() one byte at a time, to
> be interpreted with some legacy 8-bit character set. So the result
> will typically be mojibake even if the correct locale is selected.
>
> strtolower() mangles UTF-8 strings in many locales, such as fr-FR. I
> made a full list at <https://phabricator.wikimedia.org/T291234>. The
> UTF-8 locales mostly work, except for the Turkish ones, which mangle
> ASCII strings.
>
> At https://bugs.php.net/bug.php?id=67815 , Nikita Popov wrote: "My
> general recommendation is to avoid locales and locale-dependent
> functions, as locales are a fundamentally broken concept." I agree
> with that. I think PHP should migrate away from locale dependence.
> When PHP was young, it was convenient to use the C library, but we've
> progressed well past that point now.
>
> -- Tim Starling
>

We've been slowly moving away from locale-dependent functionality. Since
PHP 8 we no longer inherit any locales from the environment and have made
float to string conversion locale-independent.

I would very much support making strtolower() and friends a simple ASCII
case conversion operation. mb_strtolower() etc already offer full
Unicode-compliant case conversions that work correctly with multi-byte
encodings. The locale-sensitivity of strtolower() only works with legacy
single-byte encodings and as such is of questionable usefulness even in
cases where it is not actively harmful.

That said, I do think this change requires an RFC.

Regards,
Nikita


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

2021-09-07 Thread Nikita Popov
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.

Regards,
Nikita


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

2021-09-07 Thread Nikita Popov
On Mon, Sep 6, 2021 at 6:50 PM Kamil Tekiela  wrote:

> Hi Nikita,
>
> I think this might be a good idea, but I would like to propose yet another
> variant.
> Replace stdClass with DynamicObject and keep stdClass as an alias. It can
> be deprecated in 8.3.
> If we only add an alias, I am afraid that it will not catch on quickly
> enough. What I am proposing is that the cast to object will create
> DynamicObject by default.
>
> $arr = [1,2];
> var_dump((object) $arr);
> Output:
> object(DynamicObject)#1 (2) {
>   ["0"]=>
>   int(1)
>   ["1"]=>
>   int(2)
> }
>
> It will break unit tests and it might break some code (e.g. `if ('stdClass'
> === $class)`), but it will help people understand what is the preferred
> name going forward without deprecating it right now.
>

My only apprehension with making stdClass rather than DynamicObject the
alias is the widespread impact this will have on extension testing code.
https://github.com/php/php-src/pull/7475 shows the approximate impact on
php-src. We need to update references to stdClass in var_dump() output in
more than 300 tests. We can do this easily, but it will be an inconvenience
for 3rd party extension tests that need to deal with more than one PHP
version.

Apart from that I agree that making DynamicObject the actual name and
stdClass the alias would be better.

Regards,
Nikita


Re: [PHP-DEV] Adding a way to disable the stat cache

2021-09-07 Thread Nikita Popov
On Fri, Sep 3, 2021 at 7:10 PM Kevin Lyda  wrote:

> [sent a second time, now to the list, sorry]
>
> On Fri, Sep 3, 2021 at 3:53 PM Christian Schneider
>  wrote:
> > How can you say "it never was a problem" if we never had to live without
> stat cache?
> > Can you back up that claim with numbers? There are some of us who run
> high-volume websites where system load increase could be a problem.
>
> Using this bash script:
>
> #!/bin/bash
> echo "Without cache"
> time ./sapi/cli/php -d enable_stat_cache=3DFalse "$@"
> echo "With cache"
> time ./sapi/cli/php "$@"
>
> To run this php script:
>
>  $iterations =3D 100;
> function all_the_stats($filename) {
> @lstat($filename);
> @stat($filename);
> }
> while ($iterations--) {
> all_the_stats(__FILE__);
> }
>
> I see this output:
>
> Without cache
>
> real 0m7.326s
> user 0m5.877s
> sys 0m1.448s
> With cache
>
> real 0m5.010s
> user 0m5.009s
> sys 0m0.000s
>
> So that's 2 seconds slower to do 2 million uncached stat calls vs
> cached with a 100% cache hit rate (minus the first stat/lstat calls).
>
> Technically, yes, it's slower, but I'd suggest that making 2 million stat
> calls to a single file is a bad idea. And remember, the cache holds *one*
> file. If you stat a second file it's a cache miss.
>

These numbers look pretty good to me. It would be great if someone on
Windows and macos could repeat this experiment, so we have an idea of how
other platforms fare in this worst-case scenario.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Random Extension 3.0

2021-09-07 Thread Nikita Popov
On Thu, Sep 2, 2021 at 5:10 PM Go Kudo  wrote:

> Hi Internals.
>
> Expanded from the previous RFC and changed it to an RFC that organizes the
> whole PHP random number generator. Also, the target version has been
> changed to 8.2.
>
> https://wiki.php.net/rfc/rng_extension
> https://github.com/php/php-src/pull/7453
>
> Hopefully you will get some good responses.
>

This RFC currently tries to keep some semblance of compatibility with the
existing mt_rand() algorithm by retaining the same implementation for
mapping the raw random number stream into a range. However, the algorithm
we use for that is not exactly state of the art and requires two full-width
divisions at minimum. There are more efficient scaling algorithms based on
fixed-point multiplication, which are "nearly divisionless" (
https://arxiv.org/pdf/1805.10941.pdf). The variant at
https://github.com/apple/swift/pull/39143 is entirely divisionless.

Doing this would improve performance (though I'm not sure by how much --
maybe once we add up our call overhead, it isn't important anymore?) but it
would provide a different sequence than mt_rand(). Something we might want
to consider.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Random Extension 3.0

2021-09-07 Thread Nikita Popov
On Sat, Sep 4, 2021 at 10:57 PM Marc  wrote:

>
> On 9/2/21 5:10 PM, Go Kudo wrote:
> > Hi Internals.
> >
> > Expanded from the previous RFC and changed it to an RFC that organizes
> the
> > whole PHP random number generator. Also, the target version has been
> > changed to 8.2.
> >
> > https://wiki.php.net/rfc/rng_extension
> > https://github.com/php/php-src/pull/7453
> >
> > Hopefully you will get some good responses.
>
> For me (user land developer with no voting power) your RFC looks pretty :)
>
> Beside the abstract vs interface question I have some other notes:
>
> On the one hand you define "getBytes()" which returns a string and on
> the other hand you define "shuffleString()" - is the first one a binary
> string and the other a charset dependent string? I guess here
> "shuffleString()" works on byte level and so it should be "shuffleBytes()".
>
> Why are there no default values for min/max on "getInt()" - It seems
> unnecessary to me to pass min/max arguments if you are just interested
> in a random integer or passing max as well if you are interested in a
> positive integer only.
>

Because the default range is not obvious. For example mt_rand() without
min/max will actually return only non-negative integers, so someone might
expect $random->getInt() to do the same, even though it makes very little
sense. $random->getInt($n) could be interpreted either as a number in
$n..PHP_INT_MAX (if we just see it as leaving $max at the default value),
or 0..$n-1 (a very common convention for single-argument random integer
functions). Requiring both arguments makes the meaning unambiguous.

Regards,
Nikita


[PHP-DEV] Alias stdClass to DynamicObject?

2021-09-06 Thread Nikita Popov
Hi internals,

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?

Regards,
Nikita


Re: [PHP-DEV] Re: [RFC] Random Extension 3.0

2021-09-06 Thread Nikita Popov
On Sun, Sep 5, 2021 at 7:40 PM Ben Ramsey  wrote:

> Go Kudo wrote on 9/4/21 23:00:
> > Indeed, it may be true that these suggestions should not be made all at
> > once. If necessary, I would like to propose to organize the RNG
> > implementation first.
> >
> > Do we still need an RFC in this case? I'm not sure I'm not fully
> understand
> > the criteria for an RFC. Does this internal API change count as a BC
> Break
> > that requires an RFC?
>
> Yes, since re-organizing the RNG implementation is a major language
> change that affects extensions and other downstream projects, this
> requires an RFC.
>
> >> Personally, I don't see the benefit of including this OOP API in the
> core.
> >
> > On the other hand, can you tell me why you thought it was unnecessary?
> > Was my example unrealistic?
>
> You also said, in reply to Dan Ackroyd:
>
> > Either way, I'll try to separate these suggestions if necessary, but if
> we
> > were to try to provide an OOP API without BC Break, what do you think
> would
> > be the ideal form?
>
> The OOP API appears to be a wrapper around the RNG functions. The only
> thing it gains by being in the core is widespread use, but it loses a
> lot of flexibility and maintainability, since the API will be tied to
> PHP release cycles.
>
> By doing this as an OOP wrapper in userland code, you're not restricting
> yourself to release only when PHP releases, and you can work with the
> community (e.g., the PHP-FIG) to develop interfaces that other projects
> might use so they can make use of their own RNGs, etc.
>

The OO API is not just a wrapper around the RNG functions -- it provides
new functionality that cannot be efficiently implemented in userland code.
This is for two reasons:

1. It provides independent randomness streams, which means it's not
possible to reuse existing functionality like mt_rand() for this purpose,
which only provides a single global random number stream. You would have to
reimplement the random number generator in userland. While this is
possible, it will generally not be efficient, because PHP does not have
native support for unsigned modular arithmetic, which is what random number
generators generally need.

2. It allows using functions like shuffle() and array_rand() with an
independent randomness stream. These functions cannot be efficiently
implemented in userland, because userland does not have random access to
arrays. Internals can generate a random number and use it to pick the key
at that position, which is O(1). Userland would have to call array_keys()
first to allow random access on keys, which is O(n).

Which is why I think this is a good addition to php-src. There's three good
reasons to include functionality in php-src (performance, ubiquity and FFI)
and this falls squarely into the first category. And now that we have
fibers and need to worry more about multiple independent execution streams,
we also need to worry about multiple independent randomness streams.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Random Extension 3.0

2021-09-03 Thread Nikita Popov
On Thu, Sep 2, 2021 at 5:10 PM Go Kudo  wrote:

> Hi Internals.
>
> Expanded from the previous RFC and changed it to an RFC that organizes the
> whole PHP random number generator. Also, the target version has been
> changed to 8.2.
>
> https://wiki.php.net/rfc/rng_extension
> https://github.com/php/php-src/pull/7453
>
> Hopefully you will get some good responses.
>

This looks pretty nice to me. A few bits of feedback:

 * Unlike the previous versions of the RFC, this one also moves all of the
existing RNG-related functionality from ext/standard to ext/random. Why
does it do this? This doesn't really seem related to the problem this RFC
is solving.

 * Regarding the abstract class Random\NumberGenerator: You currently need
the abstract class, because php_random_ng is tied to the object. An
alternative design that would allow Random\NumberGenerator to be an
interface would be to make it independent of the object, such that the
structure can be allocated in the Random constructor for userland
implementations. Of course, internal implementations could still embed it
as part of their objects -- just don't make it a requirement.

 * The future scope mentions: "Changes random source to php_random_int() a
shuffle(), str_shuffle(), and array_rand()". I don't think it makes sense
to switch these functions to use cryptographic randomness by default...

Regards,
Nikita


Re: [PHP-DEV] Adding a way to disable the stat cache

2021-09-03 Thread Nikita Popov
On Fri, Sep 3, 2021 at 4:08 PM Kevin Lyda  wrote:

> On Fri, Sep 3, 2021 at 2:34 PM Christian Schneider
>  wrote:
> > If I remember correctly it was about reducing the number of system
> calls. Is this no issue any more?
> > Has a quick benchmark been done to see the positive / negative impact of
> the stat cache for a typical application?
>
> In the lifespan of php it really wasn't an issue unless someone was
> doing something that wasn't wise - I can't think of a single reason to
> stat a file in a tight loop.
>
> However more importantly the current behaviour returns bad data for
> perfectly correct programs. So for example on a unix box...
>
>  passthru('touch foo');
> if (is_file('foo')) {
> echo "Correct\n";
> }
> passthru('rm foo');
> if (is_file('foo')) {
> echo "Incorrect\n";
> }
> ?>
>
> Now this is a silly toy, but imagine using is_file to see if a
> graphics file exists, running an image processing program on it to
> modify it, and then using a stat call to get the file length to
> populate the Content-Length field - it will almost certainly be wrong.
>

Just to throw it out there: Maybe we should clear the stat cache when
functions in the exec family are used? Even if we allow disabling the stat
cache, I think we can easily avoid that particular footgun. And if calls to
external binaries are involved we likely don't have to worry about stat
overhead.

Regards,
Nikita


Re: [PHP-DEV] Adding a way to disable the stat cache

2021-09-03 Thread Nikita Popov
On Thu, Sep 2, 2021 at 10:27 AM Kevin Lyda  wrote:

> PHP has a stat cache which is... unfortunate. As noted in this bug from
> 2004[0] it causes a number of issues for PHP users and is irrelevant
> in modern operating systems. Heck, it's not even useful in OS's people
> might consider ancient at this point.
>
> I have a PR[1] to fix this, but had not noticed it had been referred to
> this mailing list.  I'll need fix the PR at this point as the code has
> drifted, but before I do, can whatever discussion happen so that the PR
> will be accepted?  The change I've made will allow people to disable the
> cache so that it won't cause errors and it leaves the current broken
> behaviour in place.  So there's no real change to the user experience
> except they now have a tool to disable buggy behaviour.  I feel that
> this is the least intrusive way to fix this bug.
>
> In the discussion on the bug there was resistance to getting rid of the
> cache so this was the solution proposed in 2007 and seemed reasonable.
> The default behaviour remains but can be turned off by people who want
> it off - and there are a number of people who would like it off.
>
> Is this an acceptable solution? Would this functionality be accepted
> either via my PR or another.
>
> Kevin
>
> [0]: https://bugs.php.net/bug.php?id=28790
> [1]: https://github.com/php/php-src/pull/5894
>

Looks like a reasonable addition to me.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Deprecate partially supported callables

2021-09-02 Thread Nikita Popov
On Thu, Sep 2, 2021 at 5:06 PM Côme Chilliet <
come.chill...@fusiondirectory.org> wrote:

> Le Thu, 2 Sep 2021 16:32:32 +0200,
> Nikita Popov  a écrit :
>
> > Hi internals,
> >
> > Currently, there are some callables that are accepted by the "callable"
> > type, but which cannot be used with $callable() syntax. This RFC proposes
> > to deprecate support for such "callables":
> >
> > https://wiki.php.net/rfc/deprecate_partially_supported_callables
>
> It took me some time to understand that this was not a proposal to remove
>  support for all array/string representation of callables.
> This should be made clearer if possible, maybe even list the callable kinds
>  which will not be deprecated.
>

Yeah, I definitely have no intention of deprecating string/array callables
in general. If something works in $callable(), then it's going to continue
working. Added the following note:

> Normal "function", "Foo::method", ["Foo", "method"] and [new Foo,
"method"] style callables are unaffected by this change.

Regards,
Nikita


[PHP-DEV] [RFC] Deprecate partially supported callables

2021-09-02 Thread Nikita Popov
Hi internals,

Currently, there are some callables that are accepted by the "callable"
type, but which cannot be used with $callable() syntax. This RFC proposes
to deprecate support for such "callables":

https://wiki.php.net/rfc/deprecate_partially_supported_callables

Regards,
Nikita


Re: [PHP-DEV] [DISCUSSION] ReflectionType::accepts

2021-09-01 Thread Nikita Popov
On Fri, Jan 29, 2021 at 11:16 AM Brent Roose  wrote:

> Hi Nikita
>
> I was indeed thinking about the former: acceptsValue; but I would excpect
> that if the concrete value is a subtype of the current type, the fuction
> would also return true. What I'm basically interested in: will this method
> throw a type error when I pass this value? In other words: "will this
> method accept this value?"
>
> Is that in line with your view on `acceptsValue`?
>
> Kind regards
> Brent
>

I just took a quick look at this, and apart from the strict_types
distinction, another annoying edge case here is that our type checks aren't
side-effect free. As of PHP 8.1, we have at least the deprecation warning
for precision loss in weak float -> int casts, and more generally,
arbitrary side-effects during __toString() calls on objects. So just
performing a normal type check may not be suitable for this use case.

Regards,
Nikita

> On 29 Jan 2021, at 10:33, Nikita Popov  wrote:
>
> On Fri, Jan 29, 2021 at 9:15 AM Brent Roose  wrote:
>
>> Hi Internals
>>
>> Since the addition of union types, it has become a little more cumbersome
>> to determine whether a given parameter type accepts specific input.
>> Personally I've been reusing this code blob in several places because of it:
>>
>> ```
>> /** @var \ReflectionNamedType[] $types */
>> $types = match ($type::class) {
>> ReflectionUnionType::class => $type->getTypes(),
>> ReflectionNamedType::class => [$type],
>> };
>>
>> foreach ($types as $type) {
>> if (! is_subclass_of($type->getName(), ShouldBeStored::class)) {
>> continue;
>> }
>>
>> // …
>> }
>> ```
>>
>> I wonder whether we would discuss adding a method on ReflectionType that
>> accepts any given input, and tells you whether that input is valid for that
>> type or not. I was thinking about `ReflectionType::accepts(string|object
>> $input): bool` but we could discuss another name. With it, the above
>> example could be refactored like so:
>>
>> ```
>> if (! $type->accepts(ShouldBeStored::class)) {
>> return;
>> }
>>
>> // …
>> ```
>>
>> I believe this method should accept both class names and objects to be
>> consistent with the existing reflection API. It would also work with
>> intersection types, if they are to be added some day.
>>
>> What do you think?
>>
>> Kind regards
>> Brent
>>
>
> There's two possible notions of what "accepts" does:
>
> * acceptsValue(), whether a given value satisfies the type. Probably needs
> a flag to determine whether strict_types semantics should be used or not.
>
> * isSubTypeOf(), whether a given type satisfies the type in a subtyping
> relationship.
>
> You seem to be mixing these two up into one concept, even though they are
> quite different. Which is the one you actually need? (I am much more open
> to providing the former than the latter.)
>
> Nikita
>
>
>


Re: [PHP-DEV] Re: [RFC] Increment/Decrement Fixes

2021-09-01 Thread Nikita Popov
On Sun, Mar 8, 2020 at 8:31 PM Rowan Tommins 
wrote:

> On 01/03/2020 21:23, Rowan Tommins wrote:
> > https://wiki.php.net/rfc/increment_decrement_fixes
>
>
> Hi all,
>
> I have just posted a substantially rewritten version of the above RFC.
>
> * The proposal to change boolean behaviour has been dropped.
> * More justification has been included for treating nulls differently.
> * The suggestion of raising errors just for the currently broken
> operators has been explicitly addressed.
>
> I would urge anyone who reacted instinctively to the original proposal
> to read this version, and engage with the reasoning it presents, even if
> they disagree with my conclusions.
>

For the record, increment/decrement on arrays, resources and objects now
throws due to https://wiki.php.net/rfc/arithmetic_operator_type_checks.

Personally I think it's fine to change the behavior of "$null--", to make
it consistent with "$null - 1" and "$null++".

Making "$null--" into a TypeError should imho only be done if we also make
other arithmetic on null a TypeError (e.g. $null + 1, $null * 2, etc.)

Regards,
Nikita


Re: [PHP-DEV] Resources and object resources

2021-08-31 Thread Nikita Popov
On Tue, Aug 31, 2021 at 4:16 PM Côme Chilliet <
come.chill...@fusiondirectory.org> wrote:

> Le Tue, 31 Aug 2021 15:56:42 +0200,
> Côme Chilliet  a écrit :
> > It seems to be consistent accross versions: https://3v4l.org/BHtAh
> >
> > But what will happen when LDAP connections are turned into objects in
> 8.1?
> > Will they also become int(0) upon serialization or will they behave in an
> > other way? I could not find a case of resource that became an object and
> is
> > allowed on 3v4l to test this. There is no LDAP or CURL in there.
>
> Quick followup on this, I found a case that can be tested on 3v4l:
> https://3v4l.org/EKFP0
>
> -> Fatal error: Uncaught Exception: Serialization of 'XMLParser' is not
> allowed
> in /in/EKFP0:13
>
> So it seems objects of this kind throw when serialized (I hope they all do
> that).


Unless we made a mistake somewhere, yes, all "resource-like" objects are
not serializable and will throw an attempted serialization or
unserialization.


> It is not clear to me how that behaves with session, will it throw as
> soon as affected to $_SESSION, or at the end of the request?
>

Storing it in $_SESSION temporarily should work fine, it will only throw
when the session is serialized.

Regards,
Nikita


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

2021-08-30 Thread Nikita Popov
On Mon, Aug 30, 2021 at 2:14 PM Côme Chilliet <
come.chill...@fusiondirectory.org> wrote:

> Le Wed, 25 Aug 2021 12:02:49 +0200,
> 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
>
> If I understand correctly the RFC, in these two classes:
>
> class A {
> public $prop;
>
> public function __construct() {
> unset($this->prop);
> }
> }
>
> class B {
> public $prop;
> }
>
> The property $prop is not in the same state in the end? What is the
> difference?
> isset returns FALSE for both, no? And property_exists?
>

In the latter case, the property has value null. In the former case, it is
unset. In both cases, it is declared. Accessing an unset property will
trigger __get/__set. Accessing a null property will not (assuming it is
visible).

Is it something that was the same before the RFC and would be different
> after,
> or is it already two different cases and how?


This RFC does not have any impact on this behavior. This is a "standard"
pattern used for lazy initialization.

Regards,
Nikita


Re: [PHP-DEV] Re: 8.1 / Exception / Property Type / Backwards compatbility

2021-08-27 Thread Nikita Popov
On Fri, Aug 27, 2021 at 5:35 PM Ben Ramsey  wrote:

> Nikita Popov wrote on 8/26/21 14:57:
> > On Thu, Aug 26, 2021 at 8:34 PM Ben Ramsey  wrote:
> >
> >> Nikita Popov wrote on 8/26/21 09:57:
> >>> Right. I at least do not plan to address this issue. If you take a
> >>> protected property and publicly re-export it, then any compatibility
> >> issues
> >>> are on you.
> >>
> >> This does not appear to affect only cases where one is re-exporting a
> >> protected property as public.
> >>
> >> Exception protected properties without type hints:
> >>
> >> * PHP <= 8.0 - https://3v4l.org/GWmrk
> >> * PHP 8.1 - https://3v4l.org/GWmrk/rfc
> >>
> >>
> >> Exception protected properties with type hints:
> >>
> >> * PHP <= 8.0 - https://3v4l.org/UX1Pa
> >> * PHP 8.1 - https://3v4l.org/UX1Pa/rfc
> >>
> >
> > These are not really meaningful examples, because you could simply not
> > redeclare the property at all and assign it in the constructor instead
> (or
> > preferably, let the parent constructor assign it). That's the normal way
> to
> > extend exceptions. What made the original case interesting is that the
> > property redeclaration isn't immediately redundant there, because it
> > changes visibility.
>
> If we don't want downstream users to redeclare the properties, why are
> they protected?
>

To allow downstreams users to write $this->line = 123.

Regards,
Nikita


Re: [PHP-DEV] Re: 8.1 / Exception / Property Type / Backwards compatbility

2021-08-26 Thread Nikita Popov
On Thu, Aug 26, 2021 at 8:34 PM Ben Ramsey  wrote:

> Nikita Popov wrote on 8/26/21 09:57:
> > Right. I at least do not plan to address this issue. If you take a
> > protected property and publicly re-export it, then any compatibility
> issues
> > are on you.
>
> This does not appear to affect only cases where one is re-exporting a
> protected property as public.
>
> Exception protected properties without type hints:
>
> * PHP <= 8.0 - https://3v4l.org/GWmrk
> * PHP 8.1 - https://3v4l.org/GWmrk/rfc
>
>
> Exception protected properties with type hints:
>
> * PHP <= 8.0 - https://3v4l.org/UX1Pa
> * PHP 8.1 - https://3v4l.org/UX1Pa/rfc
>

These are not really meaningful examples, because you could simply not
redeclare the property at all and assign it in the constructor instead (or
preferably, let the parent constructor assign it). That's the normal way to
extend exceptions. What made the original case interesting is that the
property redeclaration isn't immediately redundant there, because it
changes visibility.

Regards,
Nikita


Re: [PHP-DEV] Re: 8.1 / Exception / Property Type / Backwards compatbility

2021-08-26 Thread Nikita Popov
On Thu, Aug 26, 2021 at 4:43 PM Dan Ackroyd  wrote:

> On Thu, 26 Aug 2021 at 12:42, Björn Larsson via internals
>  wrote:
> >
> > Den 2021-08-10 kl. 11:55, skrev Philip Hofstetter:
> > > The following valid <= PHP 8.0 code that intends to make the $line
> property
> > > public  is a fatal error in 8.1
> 
> > >
> > > For method return types, we have #[ReturnTypeWillChange], but for
> property
> > > types 路‍♀️
>
> > Hi,
> >
> > Has this been adressed / solved in some way and does it needs
> > to be fixed?
>
> My understanding is that the two scenarios are not the same and that
> there isn't much enthusiasm for 'fixing' it.
>
> For return types, the #[ReturnTypeWillChange] annotation is a
> temporary work-around, and one that would be used by a lot of people.
> It would only be used by a library until that library drops support
> for older versions of PHP.
>
> For the exception case, the way that (for legacy reasons) PHP supports
> having a public property in a child class that "overwrites" (mostly)
> the protected property in the parent class, is a pretty hinkey thing
> to do, and so this is likely to only affect a small number of people.
> It's not obvious to me how long a bridging annotation would need to
> hang around for, and it's really not obvious what the exact details of
> how it would work would be.
>
> I'd suggest using a getter method, which would work on all relevant
> versions.
>

Right. I at least do not plan to address this issue. If you take a
protected property and publicly re-export it, then any compatibility issues
are on you.

If you need to keep doing this for API stability reasons, then the way to
do it would be a conditional class declaration. Or to keep the scope
smaller, you can conditionally declare a trait with just the property  and
then use it inside a larger class.

Regards,
Nikita


Re: [PHP-DEV] Unwrap reference after foreach

2021-08-26 Thread Nikita Popov
On Sat, Aug 14, 2021 at 6:00 PM Claude Pache  wrote:

>
> Le 13 août 2021 à 15:28, Nikita Popov  a écrit :
>
> Hi internals,
>
> I'd like to address a common footgun when using foreach by reference:
> https://wiki.php.net/rfc/foreach_unwrap_ref
>
> This addresses the issue described in the big red box at
> https://www.php.net/manual/en/control-structures.foreach.php. While this
> is
> "not a bug" (as our bug tracker can regularly attest), it's rather
> unexpected, and we could easily avoid it...
>
> Regards,
> Nikita
>
>
> A case that should be mentioned in the RFC is destructuring:
>
> ```php
> foreach ($foo as [ $x, &$y ]) { /*  */ }
> ```
> https://3v4l.org/A5Vi7
>
> I assume that the reference in `$y` would be unwrapped after the loop?
>

I clarified to the RFC to say that
a) unwrapping also occurs if the loop is left using break, continue, goto
etc.
b) unwrapping occurs for array destructuring targets under the same
limitation of "simple variables only". So yes, $y would be unwrapped in
your example.

Regards,
Nikita


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

2021-08-26 Thread Nikita Popov
On Wed, Aug 25, 2021 at 5:28 PM Sara Golemon  wrote:

> On Wed, Aug 25, 2021 at 5:03 AM Nikita Popov  wrote:
> > 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 RFC offers `extends stdClass` as a way to opt-in to the use of
> dynamic properties.
> >
> This makes the assumption that existing uses of dynamic properties are all
> root classes. I don't think that assumption can be made.
>
> > Some people have suggested that we could use a magic marker
> > interface (implements SupportsDynamicProperties),
> > an attribute (#[SupportsDynamicProperties])
> > or a trait (use DynamicProperties;) instead.
> >
> My gut-check says an Attribute works well enough.  This will unlock the
> class (disable deprecation warning) in 8.2+ and be ignored in earlier
> releases (8.0 and 8.1 would need an auto-loaded polyfill userspace
> attribute obviously, but that's a tractable problem).
>
> #[SupportsDynamicProperties]
> class Foo { ... }
>

The crux of the issue is what our end goal is:

1. Require users to explicitly annotate classes that use dynamic
properties, but otherwise keep dynamic properties as a fully supported part
of the core language.
2. Remove support for dynamic properties entirely. The core language only
has declared properties and __get/__set, and nothing else. stdClass is just
a very efficient implementation of __get/__set.

The RFC is currently written under the assumption that the end goal is (2).
To support that, I believe that "extends stdClass" and "use
DynamicProperties" are the only viable approaches. The former, because it
inherits stdClass behavior, the latter because it's literally __get/__set.
An attribute would require retaining the capability to have arbitrary
classes with dynamic properties.

Now, if the actual goal is (1) instead, then I fully agree that using a
#[SupportsDynamicProperties] attribute is the ideal solution. It can be
easily applied anywhere and has no BC concerns. Heck, someone who just
doesn't care could easily slap it on their whole codebase without spending
brain cycles on more appropriate solutions.

I do think that just (1) by itself would already be very worthwhile. If
that's all we can reasonably target, then I'd be fine with the
#[SupportsDynamicProperties] solution. Though if (2) is viable without too
many complications, then I think that would be the better final state to
reach.


> > The Locked classes RFC took an alternative approach to this problem
> space:
> > Rather than deprecating/removing dynamic properties and providing an
> opt-in
> > for specific classes, it instead allowed marking specific classes as
> locked in
> > order to forbid creation of dynamic properties on them.
> >
> > I don't believe that this is the right strategy, because in contemporary
> code,
> > classes being “locked” is the default state, while classes that require
> dynamic
> > properties are a rare exception. Additionally, this requires that class
> owners
> > (which may be 3rd party packages) consistently add the “locked” keyword
> > to be effective.
> >
> I struggle here, because the opt-in approach is the most BC, but I
> actually think you're right.  I think[citation needed] that dynamic props
> are probably rare enough that as long as the escape hatch is clear, we can
> be a little more bold about nudging the ecosystem forward.  I will counter
> however, that the same issue with 3rd party libraries applies to opt-out as
> opt-in.  A third party library that's undermaintained (and uses dynamic
> props) won't be able to be used out of the box once we apply this.  I don't
> think that's enough to scare us off, it just means that the opt-in side of
> that argument cancels out.
>

I think the argument isn't quite symmetric: A 3rd-party library in
maintenance-only mode has essentially no incentive to go out of their way
and add a "locked" keyword/attribute to their classes. Adding some missing
property declarations to keep working on new PHP versions is a different
matter.

Regards,
Nikita


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

2021-08-25 Thread Nikita Popov
On Wed, Aug 25, 2021 at 4:26 PM tyson andre 
wrote:

> Hi Nikita Popov,
>
> > 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.
>
> I'd had some concerns.
>
> It might be too soon after your addition of WeakMap.
> https://www.php.net/weakmap was introduced in PHP 8.0 (and WeakReference
> in 7.4),
> so applications/libraries fixing the deprecation may need to drop support
> for php 7.x early.
> Applications attempting to polyfill a WeakMap in earlier PHP versions
> would potentially leak a lot of memory in php 7.x.
>
> - I don't know how many minor versions to expect before 9.0 is out
> - Is it feasible for a developer to create a native PECL polyfill for
> WeakMap for earlier PHP versions that has a
>   subset of the functionality the native weak reference counting does?
>   (e.g. to only free polyfilled weak references when cyclic garbage
> collection is triggered and the reference count is 1).
>

For compatibility with < PHP 7.4 in this use case I'd suggest to either
suppress the deprecation for a particular assignment, or to pick the used
mechanism depending on PHP version.

We don't have an official schedule for major versions, but going by the
last few releases, I'd guess that it happens after PHP 8.4, or around that
time :) I think an approximate PECL polyfill (that tries to free on GC)
would be possible, but I'm not sure it would really be useful relative to
having version-dependent behavior.


> Additionally, it makes it less efficient (but still feasible) to associate
> additional fields
> with libraries or native classes/PECLs you don't own (especially for
> circular data structures), especially if they need to be serialized later.
>

Efficiency probably depends on the case here -- while I'd expect the
dynamic property to be more efficient on access than the WeakMap, it will
likely also use more memory, sometimes much more.


> (it isn't possible to serialize WeakMap, and the WeakMap would have fields
> unrelated to the data being serialized)
> I guess you can have a wrapper class that iterates over a WeakMap to
> capture and serialize the values that still exist in SplObjectStorage,
> though.
> (Though other languages do just fine without this functionality)
>

Right, the WeakMap approach will not impact the serialization of the
object, as well as other property based behaviors, such as comparison
semantics. I would usually count that as an advantage (what you do
shouldn't impact the behavior of 3rd-party classes), but there's probably
use cases for everything :)


> I'm not sure if a library owner would want to change their class hierarchy
> to extend stdClass (to avoid changing the behavior of anything using `$x
> instanceof stdClass`) and the attribute/trait approach might be more
> acceptable to library owners.
> E.g.
> https://github.com/vimeo/psalm/blob/master/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
> would set a dynamic property `$stmt->pure` in
> `PhpParser\Node\Expr\FuncCall $stmt` in a vendor dependency on php-parser.
>

I wouldn't want library authors to change their class hierarchy for
3rd-party use cases, unless those use cases are actually fundamental to the
library in some way. For PHP-Parser this is actually the case, which is why
it offers the Node::setAttribute() API. The intended way to do this would
be $stmt->setAttribute('pure', true) in this case.

Regards,
Nikita


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

2021-08-25 Thread Nikita Popov
On Wed, Aug 25, 2021 at 3:51 PM Rowan Tommins 
wrote:

> On 25/08/2021 13:45, Nikita Popov wrote:
>
> > We obviously need to keep support for dynamic properties on stdClass,
> > and if we do so, I would expect that to apply to subclasses as well.
>
> Does that actually follow, though? Right now, there is no advantage to
> extending stdClass, so no reason to expect existing code to do so, and
> no reason for people doing so to expect it to affect behaviour.
>

Isn't this a direct consequence of LSP? We can't just drop behavior when
extending.

Only way for this not to follow would be if we disallowed extending from
stdClass entirely, which we presumably don't want to do.

> Second, I consider "extends stdClass" to be something of a last-ditch
> > option. If you encounter a dynamic property deprecation warning, you
> > should generally resolve it in some other way, and only fall back to
> > "extends stdClass" as the final option.
>
>
> That's a reasonable argument in terms of the multiple inheritance case.
>
> My concern about the name remains though: people already do get confused
> by the name "stdClass", because it's not in any way "standard", and
> tells you nothing about what it does.
>
> Reading "class Foo extends stdClass" gives the reader no clues what
> magic behaviour is being inherited; "class Foo extends DynamicObject"
> would be much more clear. Similarly, "$foo = new DynamicObject;
> $foo->bar = 42;" is clearer than "$foo = new stdClass; $foo->bar = 42;"
>

Yes, I can see the argument for aliasing stdClass to something more
meaningful, even independently of this RFC. I'm not sure it will have a big
impact for this use-case though, because using the new name would require
polyfilling it, so I'd expect people would stick to the old name in the
foreseeable future...

Regards,
Nikita


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

2021-08-25 Thread Nikita Popov
On Wed, Aug 25, 2021 at 12:45 PM Rowan Tommins 
wrote:

> On 25/08/2021 11:02, Nikita Popov wrote:
> > 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 is a bold move, and in principle seems sensible, although I'm
> slightly scared how many places will need fixing in legacy code bases.
>
> I have a couple of concerns with using stdClass as the opt-in mechanism:
>
> * The name of that class already leads to a lot of confusion about its
> purpose - it's not actually "standard" in any way, and new users seeing
> it as a base class are even more likely to mistake it as some kind of
> "universal ancestor". Would it be feasible to introduce an alias like
> "DynamicObject" which more clearly defines its role?
>
> * Adding a parent to an existing class isn't always possible, if it
> already inherits from something else. Perhaps the behaviour could also
> be available as a trait, which defined stub __get and __set methods,
> allowing for the replacement of the internal implementation as you've
> described?
>

So, a few thoughts: First of all, I should say that "extends stdClass" is
not so much an explicit escape hatch I built into this, it's more something
that arises naturally: We obviously need to keep support for dynamic
properties on stdClass, and if we do so, I would expect that to apply to
subclasses as well. As such, I believe that the "extends stdClass" escape
hatch is something that's going to work anyway -- the only question would
be whether we want to provide additional opt-ins in the form of
interfaces/traits/attributes/etc.

Second, I consider "extends stdClass" to be something of a last-ditch
option. If you encounter a dynamic property deprecation warning, you should
generally resolve it in some other way, and only fall back to "extends
stdClass" as the final option.

All that said, yes, we could add a trait. It would basically be
ArrayLikeObject from the RFC with "class" replaced by "trait". However, I'm
not sure this is really worthwhile. The nice thing about extending stdClass
is that it a) gives you much more efficient property access than
__get/__set and b) matches current behavior. Implementing such a trait,
even if bundled, doesn't really give you any advantages over implemening
__get/__set yourself. It would not make use of an optimized implementation
(or at least, the implementation would still be calling real __get/__set
methods) and the behavior would be limited to what the trait provides. A
custom implementation is not significantly harder (the minimal
implementation is five lines of code) but gives you more control, e.g. you
might want just the minimal implementation, or you might want to also
support Traversable, have a custom __debugInfo(), etc.

My preliminary position would be that if a) you can't avoid dynamic
properties in any other way and b) you can't extend stdClass either, then
you should go with c) implementing __get/__set yourself, as appropriate for
the given use-case.

Regards,
Nikita


[PHP-DEV] [RFC] Deprecate dynamic properties

2021-08-25 Thread Nikita Popov
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.

Regards,
Nikita


Re: [PHP-DEV] mb_check_encoding slow performance?

2021-08-25 Thread Nikita Popov
ard to improvements in this area!

Regards,
Nikita

By the way, I would welcome special-casing commonly used text encodings in
> mb_check_encoding. *AFTER* we have close to 100% test coverage, that is.
>
> Thanks,
> Alex
>
> On Mon, Aug 16, 2021 at 10:11 AM Nikita Popov 
> wrote:
>
>> On Mon, Aug 9, 2021 at 10:14 PM Rowan Tommins 
>> wrote:
>>
>>> On 07/08/2021 18:57, Hans Henrik Bergan wrote:
>>> > can someone shed some light on this? why does mb_check_encoding seem
>>> to be
>>> > so much slower than the alternatives?
>>> > benchmark code+results is here
>>> https://stackoverflow.com/a/68690757/1067003
>>>
>>>
>>> Hi Hans,
>>>
>>> Since you ran the test on PHP 7.4, the relevant implementation is here:
>>>
>>> https://heap.space/xref/PHP-7.4/ext/mbstring/mbstring.c?r=0cafd53d#php_mb_check_encoding_impl
>>>
>>> As you can maybe see, it takes a rather "brute force" approach: it runs
>>> the entire string through a conversion routine, and then checks (among
>>> other things) that the output is identical to the input. That makes it
>>> scale horribly with string length, with no optimization for returning
>>> false early.
>>>
>>> The good news is that Alex Dowad has been doing a lot of work to improve
>>> ext/mbstring recently, and landed a completely new implementation for
>>> mb_check_encoding a few months ago:
>>> https://github.com/php/php-src/commit/be1a2155 although it was then
>>> changed slightly by later cleanup:
>>> https://github.com/php/php-src/commit/3e7acf90
>>>
>>> That was too late for PHP 8.0, so I compiled an up to date git checkout,
>>> and ran your benchmark (with 100_000 iterations instead of 1_000_000; I
>>> guess my PC's a lot slower than yours!)
>>>
>>> PHP 7.4:
>>> mbstring: 57000 / 57100 / 56200
>>> PCRE: 1500 / 1200 / 12400
>>>
>>> PHP 8.1 beta:
>>> mbstring: 35600 / 1200 / 36700
>>> PCRE: 1400 / 1200 / 12100
>>>
>>> So, mbstring now detects a failure at the start of the string as quickly
>>> as PCRE does, because the new algorithm has an early return, but is
>>> still slower than PCRE when it has to check the whole string.
>>>
>>> Looking at the PCRE source, I think the relevant code is this:
>>> https://vcs.pcre.org/pcre2/code/trunk/src/pcre2_valid_utf.c?view=markup
>>>
>>> It has the advantage of only handling a handful of encodings, and only
>>> needing to do a few operations on them. The main problem ext/mbstring
>>> has is that it supports a lot of operations, on a lot of different
>>> encodings, so it's still reusing a general purpose "convert and filter"
>>> algorithm.
>>>
>>
>> I think a key problem with the mbstring implementation is that input
>> (encoding to wchar) filters work by handling one byte at a time. This means
>> that state has to be managed internally by the filter, and we need to use a
>> filter-chain interface.
>>
>> What would be better is an interface along the lines of int decode(char
>> **input, size_t *input_len), where the filter returns the decoded
>> character, while advancing the input/input_len pointers. Possibly with an
>> indication that the input is incomplete and more characters are necessary
>> to allow streaming use.
>>
>> This would allow the filter to handle one unicode character at a time
>> (regardless of how many bytes it is encoded as), and would allow to use the
>> calling code to use a simple while loop rather than a filter chain.
>>
>> Of course, this would require rewriting all our filter code...
>>
>> Regards,
>> Nikita
>>
>


Re: [PHP-DEV] mb_check_encoding slow performance?

2021-08-16 Thread Nikita Popov
On Mon, Aug 9, 2021 at 10:14 PM Rowan Tommins 
wrote:

> On 07/08/2021 18:57, Hans Henrik Bergan wrote:
> > can someone shed some light on this? why does mb_check_encoding seem to
> be
> > so much slower than the alternatives?
> > benchmark code+results is here
> https://stackoverflow.com/a/68690757/1067003
>
>
> Hi Hans,
>
> Since you ran the test on PHP 7.4, the relevant implementation is here:
>
> https://heap.space/xref/PHP-7.4/ext/mbstring/mbstring.c?r=0cafd53d#php_mb_check_encoding_impl
>
> As you can maybe see, it takes a rather "brute force" approach: it runs
> the entire string through a conversion routine, and then checks (among
> other things) that the output is identical to the input. That makes it
> scale horribly with string length, with no optimization for returning
> false early.
>
> The good news is that Alex Dowad has been doing a lot of work to improve
> ext/mbstring recently, and landed a completely new implementation for
> mb_check_encoding a few months ago:
> https://github.com/php/php-src/commit/be1a2155 although it was then
> changed slightly by later cleanup:
> https://github.com/php/php-src/commit/3e7acf90
>
> That was too late for PHP 8.0, so I compiled an up to date git checkout,
> and ran your benchmark (with 100_000 iterations instead of 1_000_000; I
> guess my PC's a lot slower than yours!)
>
> PHP 7.4:
> mbstring: 57000 / 57100 / 56200
> PCRE: 1500 / 1200 / 12400
>
> PHP 8.1 beta:
> mbstring: 35600 / 1200 / 36700
> PCRE: 1400 / 1200 / 12100
>
> So, mbstring now detects a failure at the start of the string as quickly
> as PCRE does, because the new algorithm has an early return, but is
> still slower than PCRE when it has to check the whole string.
>
> Looking at the PCRE source, I think the relevant code is this:
> https://vcs.pcre.org/pcre2/code/trunk/src/pcre2_valid_utf.c?view=markup
>
> It has the advantage of only handling a handful of encodings, and only
> needing to do a few operations on them. The main problem ext/mbstring
> has is that it supports a lot of operations, on a lot of different
> encodings, so it's still reusing a general purpose "convert and filter"
> algorithm.
>

I think a key problem with the mbstring implementation is that input
(encoding to wchar) filters work by handling one byte at a time. This means
that state has to be managed internally by the filter, and we need to use a
filter-chain interface.

What would be better is an interface along the lines of int decode(char
**input, size_t *input_len), where the filter returns the decoded
character, while advancing the input/input_len pointers. Possibly with an
indication that the input is incomplete and more characters are necessary
to allow streaming use.

This would allow the filter to handle one unicode character at a time
(regardless of how many bytes it is encoded as), and would allow to use the
calling code to use a simple while loop rather than a filter chain.

Of course, this would require rewriting all our filter code...

Regards,
Nikita


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

2021-08-16 Thread Nikita Popov
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


Re: [PHP-DEV] [RFC] Never For Argument Types

2021-08-16 Thread Nikita Popov
On Sat, Aug 14, 2021 at 3:44 PM Jordan LeDoux 
wrote:

>
>
> On Sat, Aug 14, 2021 at 6:25 AM Nikita Popov  wrote:
>
>>
>> function addMultiple(CollectionInterface $collection, mixed ...$inputs):
>> void {
>> foreach ($inputs as $input) $collection->add($input);
>> }
>>
>> A static analyzer should flag this CollectionInterface::add() call as
>> invalid, because mixed is passed to never. Effectively, this means that an
>> interface using never argument types cannot actually be used in anything
>> *but* inheritance -- so what is its purpose?
>>
>>
> When used as a sort of... pseudo-generics replacement, you'd need to use
> Docblocks to specify these, because **this feature is not generics** (which
> you correctly pointed out). I probably should have made that MORE clear so
> as to not confuse or trick anyone.
>
> If this RFC were passed, it could be sort of used like generics but it
> would be a bit hacky to use it that way as your example illustrates. In the
> absence of generics, this would probably be used as a stopgap in
> combination with docblocks. That's the point I was trying to make. :)
>
> The main value I see from an inheritance perspective is using never to
> disallow an omitted type. The inheriting class may specify *any* type, even
> mixed, but it must do so explicitly.
>

I don't think this really addresses my concern, so let me repeat it: You
cannot actually call a method using a never-type argument while typing
against the interface. What's the point of the interface then?

I don't think "you must use this in conjunction with a 3rd-party phpdoc
generics implementation for it to make any sense at all" is a suitable way
to resolve that.

The only case where this is somewhat sensible is with interfaces
controlling engine behavior, like the operator overloading interfaces
Addable etc you clearly have in mind here. If you never actually type
against them and only use them as a marker for the engine, then this works
out fine. But this still leaves the interface useless from a typesystem
perspective, it merely becomes an engine marker.

We have a better way to specify markers for the engine: Magic methods. I
think some people have a mistaken notion that engine-integrated interfaces
are always better than magic methods. This is only the case if such
interfaces are actually useful from a type system perspective. For example,
Countable and Traversable are useful magic interfaces, because you can
sensibly type against them. The recently introduced Stringable interface
(for the magic method __toString) falls in the same category.

Conversely, Serializable is actively harmful as a magic interface (apart
from the other issues with it), because whether an class implements
Serializable does not determine whether it is serializable -- all objects
are a priori serializable, the Serializable interface just gives it custom
serialization behavior. You'll note that the new __serialize/__unserialize
methods are plain magic methods without an interface. With exception of a
custom serializer implementation, user code should never be checking for
Serializable.

The operator overloading case is in between: The interface is not actively
harmful, but they also aren't useful. Given the lack of generics, it's not
really possible to write code against a "Multiplyable" interface that
actually provides a useful guarantee. The interface does not distinguish
whether "T * T" is valid, or only scalar multiplication "T * float" is
supported. When working with operator overloads in PHP, I expect usage to
type against a specific class implementing operator overloading, say Money,
rather than typing against something like Addable The latter
would accept both Money and Matrix, both of which have entirely different
rules on the kinds of operands they accept, even if they are, in some
sense, addable and multiplyable.

Regards,
Nikita


Re: [PHP-DEV] [RFC] Never For Argument Types

2021-08-14 Thread Nikita Popov
On Sat, Aug 14, 2021 at 1:27 AM Jordan LeDoux 
wrote:

> Hey internals,
>
> I've been working on the draft for my operator overloading RFC, and in
> doing so I encountered a separate change that I would like to see.
>
> That is, the use of `never` as an argument type for interfaces. Since
> arguments in PHP are contravariant to preserve Liskov substitution, `never`
> as the bottom type should indicate that implementing classes can require
> any type combination they want. This is in fact consistent with type theory
> and set theory, and is how the bottom type is treated in several other
> languages.
>
> In this case, the bottom type would be used to indicate covariant parameter
> polymorphism while not conflicting with LSP.
>
> This would provide a sort of minimal form of generics to PHP without the
> issues that actual generics present from an implementation perspective. It
> would not, however, restrict or hinder any future RFC for generics.
>
> This is at the first draft stage, and I currently have the RFC on a github
> repo to allow for easy contribution and collaboration.
>
> Any feedback is greatly appreciated.
>
> https://github.com/JordanRL/never-argument-type
>

There's two sides to this coin: While using a never argument type allows
you to avoid the LSP problem, it also means that you cannot call the method
while typing against the interface. Let's take your CollectionInterface

interface CollectionInterface {
public function add(never $input): self;
}

and actually try to make use of it:

function addMultiple(CollectionInterface $collection, mixed ...$inputs):
void {
foreach ($inputs as $input) $collection->add($input);
}

A static analyzer should flag this CollectionInterface::add() call as
invalid, because mixed is passed to never. Effectively, this means that an
interface using never argument types cannot actually be used in anything
*but* inheritance -- so what is its purpose?

Compare this to generics. The interface changes to

interface CollectionInterface {
public function add(T $input): self;
}

and the use changes to

function addMultiple(CollectionInterface $collection, T ...$inputs):
void {
foreach ($inputs as $input) $collection->add($input);
}

Now a T argument is passed to a T parameter, and everything is fine.

Regards,
Nikita


Re: [PHP-DEV] readonly properties

2021-08-13 Thread Nikita Popov
On Thu, Aug 12, 2021 at 9:16 PM Marc  wrote:

> Hi,
>
> As 8.1 adds readonly properties I wonder which build-in properties
> should be defined readonly.
>
> Currently I could find build-in readonly properties only on PDO and DOM.
>
>
> Very incomplete list where readonly properties could make sense:
>
> 1. Enum properties:
>
> enum Test:string {
>  case TEST = 'test';
> }
>
> $case = TEST::TEST;
> $refl = (new ReflectionObject($case))->getProperty('value');
> var_dump($refl->isReadOnly());  // false
> var_dump($refl->isPublic());  // true
> $case->value = 'foo'; // Fatal error: Uncaught Error: Enum properties
> are immutable
>

Yeah, these are a perfect use case for "readonly". Done in
https://github.com/php/php-src/commit/caefc6a50789295b0993c4e657c825484650172a.
This actually fixes a bug, because the homegrown "readonly" implementation
for enums was not quite correct.


> 2. DateInterval->days
>
> $interval = (new DateTime())->diff(new DateTime());
> var_dump($interval->days); // 0
> $refl = (new ReflectionObject($interval))->getProperty('days');
> var_dump($refl->isReadOnly()); // false
> var_dump($refl->isPublic()); // true
> $interval->days = 2;
> var_dump($interval->days);  // 0
>

The DateInterval properties are currently implemented as getters/setters on
some internal state. Some of those are getter-only, but probably not fully
immutable.

3. Exception properties
>
> Exception properties are protected but does it really make sense to be
> able to modify an exception property after initialization?
>
> I know this would be a BC break :(
>

There is definitely code out there relying on modifying both protected and
private Exception properties, I don't think we want to touch these without
cause.

Regards,
Nikita


[PHP-DEV] Unwrap reference after foreach

2021-08-13 Thread Nikita Popov
Hi internals,

I'd like to address a common footgun when using foreach by reference:
https://wiki.php.net/rfc/foreach_unwrap_ref

This addresses the issue described in the big red box at
https://www.php.net/manual/en/control-structures.foreach.php. While this is
"not a bug" (as our bug tracker can regularly attest), it's rather
unexpected, and we could easily avoid it...

Regards,
Nikita


Re: [PHP-DEV] Shut down gcov.php.net

2021-08-04 Thread Nikita Popov
On Wed, Aug 4, 2021 at 2:25 PM Christoph M. Becker 
wrote:

> Hi all,
>
> I suggest to shut down  altogether.  It is barely
> maintained for quite a while: PHP 7.4 builds are failing, and there are
> no PHP 8.0 builds at all.  We already run a coverage job on Azure[1], so
> there is no *need* for this site anymore, and we can't spent our time on
> more useful things.
>
> See also .
>
> Any objections?
>
> [1] 
>

+1 to shutting down this server. Coverage is now on azure and also
published to https://app.codecov.io/gh/php/php-src/, and valgrind has been
replaced by asan jobs. Newer PHP versions don't work on gcov because the OS
is too old.

Regards,
Nikita


Re: [PHP-DEV] Method compatibility checks and order in which files are included

2021-07-22 Thread Nikita Popov
On Thu, Jul 22, 2021 at 10:32 AM Sebastian Bergmann 
wrote:

> We noticed that the PHARs of PHPUnit stopped working with PHP 8.1 [1].
> Before [2], a commit pushed today, PHPUnit loaded all sourcecode files
> packaged in the PHAR on startup (to work around problems that (hopefully)
> no longer exist because PHPUnit's PHAR is scoped using PHP-Scoper
> nowadays). This loading of all sourcecode files lead to the error reported
> in [1]. The "static include list" used for this is generated by PHPAB [3]
> which performs a topological sort based "class implements interface",
> "class uses trait", "class extends class", etc. dependencies. This
> topological sort does not, however, consider type declarations for
> properties as well parameters and return values as dependencies.
>
> What follows is a minimal, reproducing example:
>
> .
> ├── autoload.php
> ├── Collection.php
> └── CollectionIterator.php
>
> 0 directories, 3 files
>
> This is the contents of autoload.php:
>
>  require __DIR__ . '/Collection.php';
> require __DIR__ . '/CollectionIterator.php';
>
> This is the contents of Collection.php:
>
>  class Collection implements IteratorAggregate
> {
>  public function getIterator(): CollectionIterator
>  {
>  return new CollectionIterator($this);
>  }
> }
>
> And this is the contents of CollectionIterator.php:
>
>  class CollectionIterator implements Iterator
> {
>  public function __construct(Collection $collection)
>  {
>  }
>
>  public function current(): mixed
>  {
>  }
>
>  public function next(): void
>  {
>  }
>
>  public function key(): int
>  {
>  }
>
>  public function valid(): bool
>  {
>  }
>
>  public function rewind(): void
>  {
>  }
> }
>
> When I run autoload.php then I get
>
> Fatal error: Could not check compatibility between
> Collection::getIterator(): CollectionIterator
> and IteratorAggregate::getIterator(): Traversable, because class
> CollectionIterator is not
> available in /home/sb/example/Collection.php on line 4
>
> I am using PHP 8.1-dev (590af4678bbdbb9422bfcc568a0b8d2d1a6f74f5). The
> error does not occur with PHP 8.0.8.
>
> I assume the following happens:
>
> * The compiler works on Collection.php
> * The compiler sees that Collection implements IteratorAggregate
> * The compiler sees that Collection::getIterator() returns
> CollectionIterator
> * The compiler performs the method compatiblity check
> * The method compatiblity check cannot be performed because
> CollectionIterator has not been compiled yet
>
> When I change autoload.php like so
>
>  require __DIR__ . '/CollectionIterator.php';
> require __DIR__ . '/Collection.php';
>
> then the method compatiblity check can be performed (and succeeds without
> error).
>
> After this long introduction (sorry!), I can finally ask my question: can
> there be situations where this alternate order of requiring the source
> files would also fail? Is there a potential for cyclic dependencies
> between two sourcefiles that can only be resolved when autoloading is
> used? I am "scared" that using "static include list" may become unfeasible
> when more compile-time checks such as the method compatibility check are
> added.
>

Hey Sebastian,

This behavior isn't new in PHP 8.1, it exists since the introduction of
variance support in PHP 7.4. Presumably you just didn't hit a case where a)
a non-trivial subtying relationship had to be established and b) the files
did not happen to be included in the right order. The only thing that PHP
8.1 adds are more types on internal classes, which means that there are
more type constraints to satisfy.

To answer your actual question, yes, it's possible to have cyclic
dependencies, and such cases are (and can only be) supported if an
autoloader is used. A simple example is this structure:

class A {
public function method(): B {}
}
class B extends A {
public function method(): C {}
}
class C extends B {}

There is no legal ordering of these classes that does not involve
autoloading during inheritance.

Regards,
Nikita


Re: [PHP-DEV] intersection types and null for defaults, properties and return types

2021-07-20 Thread Nikita Popov
On Mon, Jul 19, 2021 at 8:16 PM G. P. B.  wrote:

> On Mon, 19 Jul 2021 at 18:26, Guilliam Xavier 
> wrote:
>
> > On Mon, Jul 19, 2021 at 4:26 PM Nicolas Grekas  >
> > wrote:
> >
> > >
> > > https://github.com/php/php-src/pull/7259
> > >
> >
> > Great! Thanks! Interesting how it works out-of-the-box with just this
> > addition in Zend/zend_language_parser.y:
> >
> > ```diff
> >  type_expr:
> >   type { $$ = $1; }
> >   | '?' type { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
> >   | union_type { $$ = $1; }
> >   | intersection_type { $$ = $1; }
> > + | '?' intersection_type { $$ = $2; $$->attr |= ZEND_TYPE_NULLABLE; }
> >  ;
> > ```
> >
> > On Mon, Jul 19, 2021 at 5:09 PM Dan Ackroyd 
> > wrote:
> >
> > > nicolas-grekas wrote on the PR:
> > > > ?X cannot be confused with
> > >
> > > It confused me. A compiler might understand it, but as a human I have
> > > trouble understanding it.
> > >
> > > Trowski wrote:
> > > > The syntax should be either ?(X) or (X)|null
> > >
> > > Non-ambiguous syntax is much better than ambiguous syntax.
> > >
> >
> > Maybe it's just a matter of habit?
> > For instance I got used to seeing things like `!$x = f()` (e.g. `if (!$x
> =
> > f()) { throw /*...*/; } /* use $x */`) because some CS consider explicit
> > parentheses in `!($x = f())` redundant (as PHP has a special case that
> > "overrides" the normal precedence `(!$x) = f()` which would be an error).
> > If you first consider `X` as a type "unit", then it makes sense to make
> > it "nullable" by prefixing it with `?`, I think?
> >
> >
> > >
> > > But this discussion is moot for 8.1.
> > >
> > > This limitation might make intersection types not be considered usable
> > > by some projects, but the feature freeze is today.
> > >
> >
> > Which can also be reversed: "The feature freeze is today, but this
> > limitation might make intersection types not be considered usable by some
> > projects"? (playing devil's advocate, I don't master the process)
> >
> > Regards,
> >
> > --
> > Guilliam Xavier
> >
>
> Since when is usability for a specific project a consideration an RFC needs
> to have?
> If Symfony can't use it in its current state tough luck,
> I'm sure plenty of other projects can, especially now that using 'new
> Class' is possible as a default object value.
>
> I frankly don't care about being able to have some sort of partial union
> possible with the usage of intersection types,
> because it seems the machinery in the engine which makes null work, should
> also allow any standard PHP types as those are part of a bitflag and
> handling variance with them seems to work just fine...
>
> But for the love of god, this proposed syntax is horrendous, saying ?X is
> unambiguous is disingenuous.
> It can either mean (?X) = (X|null) or ?(X) = (X)|null, the former
> one being bogus as null is an impossible type, something which should
> error out to point out a potential bug in the same way we do for redundant
> types that we know at compile time.
> And if we allow this syntax then we really should be allowing ?A|B which is
> dumb.
> (and no ?X is NOT something I consider forward compatible when it is just
> going to become one more edge case we need to maintain because people have
> no patience).
>
> If ?(X) is allowed then ?(A|B) should also be allowed, and that needs an
> RFC for sure due to the controversy it had in the union type RFC.
>
> The only remaining sensible choice is (X)|null / null|(X), but as I
> said above if the machinery for null is there it must mean the machinery
> for int/string/array/float/bool is also there, and frankly being able to do
> something like (Traversable)|array is also extremely valuable,
> maybe even more than nullability, but in any case this is going to be
> confusing for end-users why only null (or standard PHP types) can be
> combined in a union with intersection types.
>
> That's one reason why it's only pure intersection types, if I had more time
> (or for crying out loud somebody would have paid me) to work on this I
> would have loved to get full composite types working.
>
> And I find it frankly insulting that in the four month this RFC has been
> published for discussion, with multiple push backs for voting due to bugs
> and me wanting that people know what implementation is - for the most part
> - going to land in php-src, this specific point has not been raised.
> It just feels like you are pissing on 6+ months of *unpaid* work I did
> because it doesn't suit your needs, and you just realised that and so
> decided to throw a wrench into a carefully crafted RFC to "fix" it by using
> a Twitter mob to put pressure on us/me.
>
> Maybe this topic didn't come up because for nearly everyone else "Pure
> Intersection Types" means what it says on the can, moreso that in the RFC
> the following line:
> > This means it would *not* be possible to mix intersection and union types
> together such as A|C, this is left as a future scope
> makes it clear, and most voters also understood that '?' 

Re: [PHP-DEV] License for PHP 8.x?

2021-07-20 Thread Nikita Popov
On Tue, Jul 20, 2021 at 12:01 PM Mike Schinkel  wrote:

> On Jul 19, 2021, at 3:31 AM, Nikita Popov  wrote:
>
> On Mon, Jul 19, 2021 at 5:56 AM Mike Schinkel  wrote:
>
>> I was just checking to see what the license was for PHP and this page[1]
>> states:
>>
>> "PHP 4, PHP 5 and PHP 7 are distributed under the PHP License
>> v3.01, copyright (c) the PHP Group."
>>
>> Can I assume that PHP 8 is also distributed under the PHP License v3.01
>> and that this page on PHP.net <http://php.net/> has just not yet been
>> updated to reflect the existence of PHP 8?
>>
>
> Yes, see https://github.com/php/php-src/blob/master/LICENSE. The page
> should be updated to say something like "PHP 4 and newer".
>
>
> Thanks for the quick reply.
>
> Maybe this would be a good first contribution for me?  How could I go
> about updating that page?
>

You can edit the page here:
https://github.com/php/web-php/blob/master/license/index.php

Regards,
Nikita


Re: [PHP-DEV] Request for karma to vote on RFCs

2021-07-19 Thread Nikita Popov
On Sun, Jul 18, 2021 at 8:48 PM Tobias Nyholm 
wrote:

> Hey.
> I would like to get karma to be able to vote on RFCs. I understand that
> voting karma isn’t usually given out to people who write their first
> mailing list entry.
>
> But I do believe I qualify as “Lead developers of PHP based projects
> (frameworks, cms, tools, etc.)”
>
> For those of you who don’t know me, I’ve been working with open source PHP
> projects since 2015. I am part of Symfony core team, I wrote PSR-18 and was
> part of the working group for PSR-17. I also maintain Guzzle,
> webmozart/assert, Flysystem, HTTPlug and the php-http ecosystem and about
> 50 other packages with more than 100.000 monthly downloads.
>
> I think I am the most downloaded PHP maintainer.
>
> I have been following the RFCs more closely the past 2 years and I’ve
> finally gathered some courage to ask for karma. There has not been many
> (maybe just one or two) RFCs where I wished the vote turned out the other
> way. So, I don’t think I would have any radical opinions about future RFCs.
>
> If I’ve understad the process correctly, I do need someone with a php.net
> VCS account to sponsor me.
>
> My username is: nyholm
>
> Regards
> Tobias Nyholm
>

Hey Tobias,

My response here is basically the same as the last time the topic came up:
https://externals.io/message/110936#110937 Voting is just the very last
step of the RFC process, at which point the proposal can no longer be
influenced. If you have feedback about a proposal based on your extensive
experience in PHP's open source ecosystem, then the discussion phase is the
time to provide it, while it can still influence the proposal, as well as
other people's view of the proposal.

At least in my personal opinion, I think it's important that people granted
voting rights as community representatives have at least some historical
involvement in RFC discussions.

Regards,
Nikita


Re: [PHP-DEV] LGPL Question

2021-07-19 Thread Nikita Popov
On Fri, Jul 16, 2021 at 6:04 PM Jordan LeDoux 
wrote:

> I'm fairly certain that it is compatible, however I wanted to double check.
> Can LGPLv3 sources be included with the PHP source or is the PHP License
> incompatible?
>

In principle, it is compatible. We do bundle one LGPL 2.1 library (libmbfl).

Having LGPL licensed code may affect downstream users though, e.g. someone
shipping a proprietary version of PHP will be required to build mbstring as
a shared extension, to ensure that the LGPL licensed code portion may be
substituted. For that reason, I don't think we can accept LGPL licensed
code in any required PHP components (that are always statically linked).

Generally, not having LGPL licensed code is strongly preferred.

Regards,
Nikita


  1   2   3   4   5   6   7   8   9   10   >