Re: [PHP-DEV] [RFC][Vote] Property Hooks

[Nikita Popov]

On Mon, Apr 15, 2024, at 18:43, Larry Garfield wrote:
> The vote for the Property Hooks RFC is now open:
> 
> https://wiki.php.net/rfc/property-hooks
> 
> Voting will close on Monday 29 April, afternoonish Chicago time.

I know I'm a few months late on this one, but I figured I'd still leave a 
couple of thoughts... Overall, the proposal is well thought out and does 
address many of the reservations I had about my original accessors proposal.

One of the more interesting choices in this proposal is to base whether the 
property is virtual or backed on the presence of a "$this->prop" reference in 
the accessor implementation. I think that, conceptually at least, this is a 
good choice.

What I find concerning though, is that the presence/absence of such a reference 
also affects the meaning of the get and set hooks. If the property is virtual 
and it only has get, then the property cannot be set. If the property is backed 
and only has get, then the property *can* be set. A no-op setter is implied. 
(Similar for only having a set hook.)

This has the unfortunate consequence that you actually have to look at the 
accessor implementation to determine the API of the class -- only looking at 
the "prototypes" (i.e. signatures without implementation bodies) is no longer 
sufficient! This seems both unfortunate and unprecedented.

This could have been avoided by still requiring an explicit no-op "set;" at the 
expensive of some additional verbosity.

The other thing that stood out to me are the short-hand notations using =>. 
There was a prior RFC on the topic (https://wiki.php.net/rfc/short-functions), 
which has been declined. That RFC would have introduced => ... as a general 
shorthand for { return ...; }.

The shorthand notation for get is compatible with that formulation. However, 
the shorthand notation for set is not. In that case => ... isn't short for { 
return ...; }, but rather for { $this->prop = ...; }.

This seems pretty unfortunate to me, and possibly closes the door on revisiting 
a general short function syntax in the future. Mostly I'm scratching my head at 
why this was included in the proposal at all, as I would not expect this use of 
the set hook to be common enough to justify a shorthand. The common case is a 
guard that checks the value without modifying it.

Putting this to the "would this shorthand have passed if it were introduced by 
a separate RFC on top of the base implementation" test, I think the answer 
would have been a clear "no".

Regards,
Nikita

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

[Nikita Popov]

On Mon, Jan 15, 2024, at 13:54, Nuno Maduro wrote:
> On Mon, 15 Jan 2024 at 10:36, Daniil Gentili 
> wrote:
> 
> > Hi all,
> >
> > I've opened voting for the final-by-default anonymous classes RFC:
> > https://wiki.php.net/rfc/final_by_default_anonymous_classes
> >
> > Regards,
> >
> > Daniil Gentili.
> >
> 
> Hi Daniil,
> 
> Thank you for your work on this RFC. In my opinion, this RFC should not
> move forward for consistency reasons. If regular class definitions are
> non-final by default, anonymous classes should be non-final too.

I don't think that consistency is a good argument here, because anonymous 
classes are qualitatively different in this regard. In order to extend a class 
you need to name it -- which *should* be impossible-by-design for an anonymous 
class.

Unfortunately, due to an implementation oversight, this is not actually true. I 
consider the fact that extending anonymous classes is possible to be a bug, 
which is why I also accept the minor backwards compatibility break that comes 
with fixing it. Of course, the line between a bug and a feature is quite fine 
sometimes, so I understand that people have differing views on where this falls.

Regards,
Nikita

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

[Nikita Popov]

On Sun, Dec 3, 2023, at 16:05, Nicolas Grekas wrote:
> Hello Nikita,
>  
>> > I've just opened voting for the final anonymous classes RFC @ 
>> > https://wiki.php.net/rfc/final_anonymous_classes.
>> > 
>> > Voting started now, and will run until December 18th 2023, 00:00 GMT.
>> 
>> For the record, I've voted against this proposal because I believe it should 
>> have gone with option 2, that is to *always* make anonymous classes final.
>> 
>> It makes very little sense to me that everyone needs to explicitly mark 
>> their anonymous classes as final just because there is a class_alias 
>> loophole that could, in theory, have been used to extend anonymous classes 
>> in the past. Especially given that there is no evidence of this "feature" 
>> being used in the wild (or if there is such evidence, it was not presented 
>> in the proposal).
> 
> You might have missed my message in the discussion thread,
> 
> see https://externals.io/message/121685#121690
> 
> There is such evidence (not in the RFC though).

Thanks, I did indeed miss this. However, I also don't think that a test-only 
use is particularly compelling. If this were something that Symfony had exposed 
as a public API promise, we might be caught between a rock and a hard place, 
but this thankfully doesn't seem to be the case.

I think I can fairly confidently say that the ability to extend anonymous 
classes by abusing class_alias is a bug in the original implementation, rather 
than an intentional feature. Yes, people do start to depend on bugs if they 
exist for long enough, but I don't think this means we shouldn't fix them 
(within the bounds of pragmatism).

Regards,
Nikita

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

[Nikita Popov]

On Sun, Dec 3, 2023, at 11:40, Daniil Gentili wrote:
> Hi all,
> 
> I've just opened voting for the final anonymous classes RFC @ 
> https://wiki.php.net/rfc/final_anonymous_classes.
> 
> Voting started now, and will run until December 18th 2023, 00:00 GMT.

For the record, I've voted against this proposal because I believe it should 
have gone with option 2, that is to *always* make anonymous classes final.

It makes very little sense to me that everyone needs to explicitly mark their 
anonymous classes as final just because there is a class_alias loophole that 
could, in theory, have been used to extend anonymous classes in the past. 
Especially given that there is no evidence of this "feature" being used in the 
wild (or if there is such evidence, it was not presented in the proposal).

Regards,
Nikita

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

[Nikita Popov]

On Thu, Nov 23, 2023, at 09:48, Nicolas Grekas wrote:
> Hi Rowan,
> 
> Le jeu. 23 nov. 2023 à 08:56, Rowan Tommins  a
> écrit :
> 
> > On 23 November 2023 01:37:06 GMT, Claude Pache 
> > wrote:
> > >What you describe in the last sentence is what was initially designed and
> > implemented by the RFC: https://wiki.php.net/rfc/typed_properties_v2
> > (section Overloaded Properties).
> > >
> > >However, it was later changed to the current semantics (unset() needed in
> > order to trigger __get()) in https://github.com/php/php-src/pull/4974
> >
> >
> > Good find. So not only is it not specified this way in the RFC, it
> > actually made it into a live release, then someone complained and we rushed
> > out a more complicated version "to avoid WTF". That's really unfortunate.
> >
> > I'm not at all convinced by the argument in the linked bug report -
> > whether you get an error or an unexpected call to __get, the solution is to
> > assign a valid value to the property. And making the behaviour different
> > after unset() just hides the user's problem, which is that they didn't
> > expect to *ever* have a call to __get for that property.
> >
> > But I guess I'm 4 years too late to make that case
> >
> 
> Sorry this comes as a surprise to you but you're rewriting history here.
> The current behavior, the one that was fixed in that commit, matches how
> PHP behaved before typed properties, so this commit brought consistency.
> 
> About the behavior, it's been in use for many years to build lazy proxies.
> I know two major use cases that leverage this powerful capability: Doctrine
> entities and Symfony lazy services. There are more as any code that
> leverages ocramius/proxy-manager relies on this.
> 
> About the vocabulary, the source tells us that "uninitialized" properties
> that are unset() become "undefined". I know that's not super accurate since
> a typed property is always defined semantically, but that's nonetheless the
> flag that is used in the source. Maybe this could help with the RFC.

This. The lazy initialization use case is the only reason why we still allow 
declared properties to be unset at all.

Our long term plan was to find an alternative way to support lazy 
initialization for properties, and then forbid calling unset() on declared 
properties. However, we still don't have that alternative today.

Regards,
Nikita

Re: [PHP-DEV] Two new functions array_first() and array_last()

[Nikita Popov]

On Sat, Oct 14, 2023, at 20:00, David Grudl wrote:
> PHP lacks two very basic functions for working with arrays:
> 
> - array_first() returning the first element of an array (or null)
> - array_last() returning the last element of the array (or null)
> 
> While PHP has functions that return the first and last keys,
> array_key_first() and array_key_last(), it does not have more useful
> functions for values.
> 
> a) What about reset() and end()?
> Programmers "abuse" the reset() and end() functions for this purpose.
> The problem is that these functions are used to move the internal
> pointer in the array. Which is why they have a name that is
> inappropriate when used in the sense of "return me the first element".
> 
> Much worse, they shouldn't to be used to get first/last value, because
> they have a side effect (i.e. moving the pointer).
> 
> Further, in the absence of an element, they return the obsolete false
> and not the currently expected null, which can be combined with the ??
> operator. In this they differ from the similar functions
> array_key_first() and array_key_last().
> 
> b) What about $array[array_key_first($array)]?
> 
> For such basic functions as returning the first and last item in an
> array, there should be a function in the basic package, not a
> workaround. Moreover, this requires having the array in a local
> variable, since $this->getFoo()[array_key_first($this->getFoo())]
> would be very inefficient and possibly incorrect.
> 
> c) Two such functions were proposed and rejected during the
> array_key_first/last RFC
> (https://wiki.php.net/rfc/array_key_first_last)
> 
> Yes, that was in 2018. At that time, functions like str_contains() or
> str_starts_with() wouldn't have even come into existence, just because
> there was an obscure way to do it without them. I believe we've moved
> on since then. Today we know how useful it is to use simple,
> easy-to-understand methods, both for programmers who write and read
> the code.
> 
> DG

I'm in favor of adding these.

To add to what you already said, because reset/end modify the array, there's a 
good chance that calling these functions will copy the whole array due to a 
modification you are not actually interested in.

So basically you have the choice between calling end(), which is the wrong 
thing to do semantically and may be slow, or using 
$array[array_key_last($array)], which is rather convoluted, and incorrect if 
the array is potentially empty.

Regards,
Nikita

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

[Nikita Popov]

On Mon, Jun 26, 2023, at 20:22, Ben Ramsey wrote:
> > On Jun 26, 2023, at 08:36, Máté Kocsis  wrote:
> > 
> > Hi Everyone,
> > 
> > As previously announced on the list, I have just started the vote about the
> > "Deprecate functions with overloaded signatures".
> > 
> > Link to the RFC:
> > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures
> > Link to the discussion thread: https://externals.io/message/120146
> > 
> > The vote is open until 2023-07-10 16:00:00 UTC.
> > 
> > Regard,
> > Máté
> 
> 
> Clarifying my “no” votes…
> 
> I voted “no” on `array_keys()` because I do not see these as two different 
> function signatures. To me, the single signature should look like this:
> 
> function array_keys(array $array, ?mixed $filter_value = null, bool 
> $strict = false): array {}
> 
> I voted “no” on `IntlCalendar::set()` because it seems to me that `setDate()` 
> and `setDateTime()` could share the same signature if `$hour`, `$minute`, and 
> `$second` all default to zero, like this:
> 
> public function setDate(int $year, int $month, int $dayOfMonth, int $hour 
> = 0, int $minute = 0, int $second = 0): void {}
> 
> In the same way, with `IntlGregorianCalendar::__construct()`, 
> `createFromDate()` and `createFromDateTime()` could be combined as:
> 
> public static function createFromDate(int $year, int $month, int 
> $dayOfMonth, int $hour = 0, int $minute = 0, int $second = 0): void {}

So commonality here is that these are all arity overloads.

I think there's three different kinds of overloads in involved in the RFC:

1. static/non-static overloads. Support for this was dropped in PHP 8, but had 
to be retained internally just for that one usage in FII, so getting rid of 
that seems quite high value.

2. type/meaning overloads, where certain parameters change meaning entirely 
across overloads. These are incompatible with named parameters and result in 
very unclear function signatures. They only become intelligible once split into 
separate signatures, which is not something PHP supports. Removing these is 
also fairly high value.

3. arity overloads, where behavior depends on number of parameters, or certain 
parameter counts are forbidden, but the actual meaning of the parameters does 
not change. Equivalent to a func_num_args() check in userland code. I think 
these arity overloads are pretty harmless. The function signature is meaningful 
and compatible with named arguments.

My overall inclination here is to vote No on all the deprecations that involve 
arity overloads and vote Yes on the remainder.

Possibly I'm missing some kind of complication that the arity overloads are 
causing?

Regards,
Nikita

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

[Nikita Popov]

On Sat, Jun 24, 2023, at 21:39, Nikita Popov wrote:
> On Fri, Dec 30, 2022, at 22:39, Christoph M. Becker wrote:
> > On 30.12.2022 at 22:12, Nikita Popov wrote:
> > 
> > > On Thu, Nov 10, 2022, at 14:29, Christoph M. Becker wrote:
> > >
> > >> On 09.11.2022 at 23:27, Nikita Popov wrote:
> > >>
> > >>> It looks like GitHub has just added support for private security 
> > >>> reports:
> > >>> https://github.blog/changelog/2022-11-09-privately-report-vulnerabilities-to-repository-maintainers/
> > >>>
> > >>> I haven't looked into the details, but it probably makes sense to enable
> > >>> those on php-src and make this our official venue for security bug 
> > >>> reports.
> > >>> This would allow retiring the last remaining use of bugs.php.net (well,
> > >>> apart from the archive of old issues, which should of course remain).
> > >>
> > >> I agree, but maybe the security team is in favor of sticking with
> > >> bugs.php.net.
> > >
> > > I noticed that the php-src repo does enable private vulnerability reports 
> > > now, and there is one sitting around without response at 
> > > https://github.com/php/php-src/security/advisories/GHSA-54hq-v5wp-fqgv.
> > >
> > > Possibly this was enabled unintentionally / without coordination with the 
> > > security team? That should probably either be disabled again, or someone 
> > > needs to keep an eye on it.
> > 
> > I had enabled that some weeks ago, since there has been a spam attack on
> > bugsnet, so we could test the new feature.  I probably should have
> > written to list right away, or at least have kept an eye on it, but I've
> > assumed to be notified about reported issues.
> > 
> > I'll have a closer look at the rather verbose report tomorrow, if nobody
> > beats me to it.
> > 
> > Generally, I'm in favor of keeping security reports on Github enabled;
> > we should stop user (not developer) comments on bugsnet as soon as
> > possible; there is already more spam than useful comments for quite a
> > while, and I think Github offers better feature to handle that.
> > 
> > Regarding the access rights on security advisories: currently only php
> > owners[1] may see and collaborate there.  To my knowledge, most of those
> > who are subscribed to the security mailing list are already in that
> > group, but if need be, others might be added, or maybe it's preferable
> > to create a new team for this.
> > 
> > Thoughts?
> 
> Security bug reports on GitHub have been active for a while now, with about 
> 10 reports having been processed.
> 
> I wanted to check back whether security folks are happy with the process, and 
> whether it is time to make this the official channel for security reports, 
> which would allow us to disable issue creation on bugs.php.net entirely. (I 
> saw that the reports are 90% spam at this point.)

I just realized that our security policy already points at GitHub security 
advisories rather than bugs.php.net here: 
https://github.com/php/php-src/security/policy#how-do-i-report-a-security-issue

So I went ahead and submitted a PR to remove support for creation of new bug 
reports on bugs.php.net: https://github.com/php/web-bugs/pull/115

Regards,
Nikita

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

[Nikita Popov]

On Fri, Dec 30, 2022, at 22:39, Christoph M. Becker wrote:
> On 30.12.2022 at 22:12, Nikita Popov wrote:
> 
> > On Thu, Nov 10, 2022, at 14:29, Christoph M. Becker wrote:
> >
> >> On 09.11.2022 at 23:27, Nikita Popov wrote:
> >>
> >>> It looks like GitHub has just added support for private security reports:
> >>> https://github.blog/changelog/2022-11-09-privately-report-vulnerabilities-to-repository-maintainers/
> >>>
> >>> I haven't looked into the details, but it probably makes sense to enable
> >>> those on php-src and make this our official venue for security bug 
> >>> reports.
> >>> This would allow retiring the last remaining use of bugs.php.net (well,
> >>> apart from the archive of old issues, which should of course remain).
> >>
> >> I agree, but maybe the security team is in favor of sticking with
> >> bugs.php.net.
> >
> > I noticed that the php-src repo does enable private vulnerability reports 
> > now, and there is one sitting around without response at 
> > https://github.com/php/php-src/security/advisories/GHSA-54hq-v5wp-fqgv.
> >
> > Possibly this was enabled unintentionally / without coordination with the 
> > security team? That should probably either be disabled again, or someone 
> > needs to keep an eye on it.
> 
> I had enabled that some weeks ago, since there has been a spam attack on
> bugsnet, so we could test the new feature.  I probably should have
> written to list right away, or at least have kept an eye on it, but I've
> assumed to be notified about reported issues.
> 
> I'll have a closer look at the rather verbose report tomorrow, if nobody
> beats me to it.
> 
> Generally, I'm in favor of keeping security reports on Github enabled;
> we should stop user (not developer) comments on bugsnet as soon as
> possible; there is already more spam than useful comments for quite a
> while, and I think Github offers better feature to handle that.
> 
> Regarding the access rights on security advisories: currently only php
> owners[1] may see and collaborate there.  To my knowledge, most of those
> who are subscribed to the security mailing list are already in that
> group, but if need be, others might be added, or maybe it's preferable
> to create a new team for this.
> 
> Thoughts?

Security bug reports on GitHub have been active for a while now, with about 10 
reports having been processed.

I wanted to check back whether security folks are happy with the process, and 
whether it is time to make this the official channel for security reports, 
which would allow us to disable issue creation on bugs.php.net entirely. (I saw 
that the reports are 90% spam at this point.)

Regards,
Nikita

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

[Nikita Popov]

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

I've voted No on the mt_rand() deprecation proposal. This is a high impact 
deprecation without sufficient justification.

I believe this proposal would have been much better served deprecating only the 
srand() and mt_srand() functions, as I previously suggested. Going through the 
arguments against mt_rand() in the RFC:
> They are not cryptographically secure.

This is a feature, not a bug. Not everything needs or wants cryptographically 
secure random numbers. There's a reason we support both.

> They are not properly reproducible, because the state could be changed 
> unpredictably by any function call, e.g. when a library is updated and adds 
> additional calls to `mt_rand ()`.

This is addressed by deprecating mt_srand() only, making Randomizer the only 
way to get reproducible random number sequences, and relegating mt_rand() to 
only the cases where reproducibility is not required.

> Any function calling `mt_srand ()` with a fixed 
> seed for whatever reason, will ruin randomness for the remainder of the 
> request.

Deprecating (and removing) mt_srand() address this one trivially.

> PHP's Mt19937 implementation only supports passing a single 32 bit integer as 
> the initial seed. Thus there are only ~4 billion possible sequences of random 
> integers generated by Mt19937. If a random seed is used, collisions are 
> expected with 50% probability after roughly 2**16 seeds (as per the birthday 
> paradox). In other words: After roughly 8 requests without explicit 
> seeding it is likely that a seed and thus a sequence is reused.

Deprecating (and removing) allows us to address this as well, as we are no 
longer bound to a specific PRNG algorithm or seeding procedure.

> Both the quality of the returned randomness as well as the generation 
> performance of Mt19937 is worse than that of the Xoshiro256StarStar and 
> PcgOneseq128XslRr64 engines that are provided in the object-oriented API.

Same as previous point: If we don't allow seeding we can change the algorithm 
however we like.

> They are functions with overloaded signatures 
> , 
> which are problematic for the reasons outlined in the “Deprecate functions 
> with overloaded signatures 
> ” 
> RFC.

While this is technically true, the particular overload in question here is a 
very mild and unproblematic one. It only enforces that the function is called 
with zero or two arguments, forbidding one argument. It does not require 
accepting different combinations of argument types, or change the meaning of 
arguments, which is what makes overloads problematic.


I think the only somewhat sound motivation for this deprecation is that 
programmers may use the non-cryptographic mt_rand() in a context where 
cryptographic numbers are required. This is a legitimate concern, but I don't 
think it's sufficient to deprecate such a widely used function.

For the longest time, we only had mt_rand(), and no easy access to a CRPRNG. A 
lot of mt_rand() misuse dates back to that time. Since the introduction of 
random_int() and random_string(), getting cryptographic randomness is no harder 
than getting non-cryptographic randomness (easier, in the case of strings).

Regards,
Nikita

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

[Nikita Popov]

On Mon, May 29, 2023, at 08:05, Máté Kocsis wrote:
> Hi Everyone,
> 
> Together with multiple authors, we'd like to start the discussion of the
> usual
> deprecation RFC for the subsequent PHP version. You can find the link below:
> https://wiki.php.net/rfc/deprecations_php_8_3
> 
> Regards:
> Máté Kocsis

I don't think we should deprecate mt_rand().

There are plenty of use-cases that require neither a seedable (predictable) RNG 
sequence, nor a cryptographically-secure RNG. For those use-cases (and 
especially one-off uses), mt_rand() is perfect, and going through Randomizer is 
an entirely unnecessary complication.

I think I could get on board with deprecating srand/mt_srand to make 
rand/mt_rand non-seedable, directing people who need a seedable RNG to use 
Randomizer, which is much better suited to that use-case. However, we should 
retain rand/mt_rand themselves for non-seeded use-cases.

With srand/mt_srand removed, we also would not have to produce any particular 
sequence, and would be free to switch the internal RNG to something other than 
Mt19937.

The same extends to array_rand(), shuffle() and str_shuffle() -- in fact the 
RFC is missing an important voting option, which is "leave them alone", or 
rather "convert to some non-seedable non-CSPRNG" if you prefer.

Regards,
Nikita

Re: [PHP-DEV] PHP code refactoring (was: include cleanup)

[Nikita Popov]

I'm a bit out of the loop on the higher level discussion, but as I got named 
dropped here, a quick note...

On Tue, Feb 28, 2023, at 23:21, Max Kellermann wrote:
> On 2023/02/28 22:31, Dmitry Stogov  wrote:
> > https://github.com/php/php-src/commit/0270a1e54c0285fa3c89ee2b0120073ef57ab5fa
> 
> This kind of change was favored by a supermajority.
> 
> You argue that this supermajority vote is irrelevant, and formally it
> indeed is, but pondering about formalities is kind of ignorant against
> the now well-known community opinion.
> 
> > https://github.com/php/php-src/commit/b98f18e7c3838cf587a1b6d0f033b89e9909c79d
> 
> No vote was made on this, therefore this doesn't violate any community
> rules, does it?
> 
> If you think this should be reverted, explain why.
> 
> > https://github.com/php/php-src/commit/42577c6b6b7577c57c161ee4a74cb193382bf1e0
> 
> Favored by supermajority, see above.
> 
> > https://github.com/php/php-src/commit/c7637ed1c03f556c6fb65884cfc5bfea4920b1c7
> 
> No vote, no rule violation, see above.

This commit broke a valuable debugging reference: I used to often check these 
defines to determine what a type code means. If you get type 18, good luck 
figuring out what that means now.

> 
> > https://github.com/php/php-src/commit/371ae12d890f1887f79b7e2a32f808b4595e5f60
> 
> As you see in the commit message, this implements an (unwritten) rule
> cited by Nikita Popov (which is now written as of
> https://github.com/php/php-src/pull/10630).  I personally don't agree
> with this rule (there's a thread on this mailing list about it), and I
> would favor reverting this commit - I only submitted this trying to
> help with implementing a rule even though I don't agree with it.

This is a pretty cherry-picked statement. In that mailing list thread I 
explicitly pointed out that while this is the rule (and if you introduce new 
code, that would be the convention to follow), that does not necessarily mean 
that it's a good idea to fix existing cases that don't follow it. I also 
pointed out that for public APIs, this is a very insidious API break, because 
it doesn't make using code fail to compile: It just silently inverts the result 
from it's previous meaning. It looks like those parts of my mail just got 
ignored.

Regards,
Nikita

> If this gets reverted, then https://github.com/php/php-src/pull/10630
> should be reverted as well.  Again, not my opinion, I'm just trying to
> help implement somebody else's opinion.
> 
> Max
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
> 
> 

Re: [PHP-DEV] What's the purpose of zend_result?

[Nikita Popov]

On Sun, Feb 19, 2023, at 09:21, Max Kellermann wrote:
> On 2023/02/19 08:56, Nikita Popov  wrote:
> > If you have a function like zend_stream_open_function(), SUCCESS and 
> > FAILURE are directly meaningful values.
> 
> Agree, but that doesn't explain why FAILURE needs to be negative.

I expect that there are two main reasons for that:
 - There are probably some places that return a (non-negative) value or FAILURE.
 - There are probably some places that check for success/failure using >= 0 and 
< 0. Another POSIX-ism.

I don't think we endorse such usage, but it likely exists.

Let me turn the question around: Is there some reason to change the value of 
FAILURE at this point?

> > The current guideline for use of bool and zend_result in php-src is that 
> > bool is an appropriate return value for "is" or "has" style functions, 
> > which return a yes/no answer. zend_result is an appropriate return value 
> > for functions that perform some operation that may succeed or fail.
> 
> What does the return value of these functions mean?
> 
> - zend_make_printable_zval()
> - zend_make_callable()
> - zend_parse_arg_bool()
> - zend_fiber_init_context()
> - zend_observer_remove_begin_handler()
> - php_execute_script()1
> 
> If I understand the guideline correctly, then those examples (and
> countless others) are defective and should be fixed, correct?

At least in principle, yes. Of course, actually doing a change may not always 
be worthwhile, especially as this might result in a silent behavior change for 
extensions (as putting the return value into an "if" would invert behavior now).

I believe Girgias has done extensive work on making the int vs bool vs 
zend_result situation more consistent, so you might want to coordinate with him.

Regards,
Nikita

Re: [PHP-DEV] What's the purpose of zend_result?

[Nikita Popov]

On Sun, Feb 19, 2023, at 08:31, Max Kellermann wrote:
> Hi,
> 
> I've done a bit of refactoring work around code using "zend_result",
> but I keep wondering why it even exists.
> 
> It was added in 1999 by commit 573b46022c46 in a huge 16k line commit
> (as macros, converted to enum in 2012 by commit e3ef84c59bf).
> 
> In 1999, C99 was brand new, and the "bool" type had just been
> introduced to the C language - yes, C was 18 years old when a native
> boolean type was added - but PHP didn't switch to C99 for another 19
> years later (b51a99ae358b).
> 
> C had a long history of abusing "int" as boolean type, wasting 2 or 4
> bytes for something that could have easily fit in 1 byte.  As if that
> wasn't obscure enough, POSIX used 0 for success and -1 for error (and
> missed the chance to return error codes, and instead added a global
> variable for that which caused more headaches).  (And don't get me
> started on the horrible strcmp() return value.)  Returning errors in C
> is a huge obscure and inconsistent mess; every function does it
> differently.
> 
> This is PHP's original definition:
> 
> #define SUCCESS 0
> #define FAILURE -1 /* this MUST stay a negative number, or it may effect 
> functions! */
> 
> This appears to follow the POSIX school of bad error return values.
> There's a comment which makes the thing even more obscure.
> 
> Really, why does it need to be negative?
> 
> Why does it imitate POSIX instead of using a boolean? (i.e. "int" and
> non-zero for success for old-schoolers)
> 
> The obvious way to indicate success or failure (without giving details
> about the nature of the failure) would be to just return "bool".  With
> C99, any discussion on "0 or 1" vs "-1 or 0" is obsolete - there is
> now a canonical boolean type that should be used.
> 
> Of course, I know already that getting rid of "zend_result" in favor
> of "bool" would be a major API breakage that requires careful
> refactoring of almost all extensions.
> 
> I just want to understand why "zend_result" was ever invented and why
> it uses those surprising integer values.  The commit message and that
> code comment doesn't explain it.
> 
> Rephrased: do you consider it a worthy goal to eventually get rid of
> "zend_result", or do you believe it's good API design that should stay
> forever?
> 
> (Yet again I wish PHP was fully C++ - unlike C, C++ has strongly-typed
> enums which cannot be casted implicitly to "int" or "bool"; that makes
> refactoring a lot easier and safer.)
> 
> Max

Any type that has only two values is isomorphic to a boolean. However, for us 
humans, not all two-valued types are semantically equivalent.

If you have a function like zend_hash_exists(), true and false are directly 
meaningful values.

If you have a function like zend_stream_open_function(), SUCCESS and FAILURE 
are directly meaningful values.

Now, if you make zend_stream_open_function() return a boolean instead, it's no 
longer clear what the return value means. Does a true return value mean that 
the stream was opened successfully, or that it failed?

For a PHP programmer, that might sound silly -- of course true means success. 
However, in C the common error reporting convention is actually the other way 
around: Non-zero return values indicate failure. This means that false 
indicates success and true indicates failure. (I'm not kidding you -- I'm 
literally working on a project that uses boolean return values with this 
convention right now.)

The current guideline for use of bool and zend_result in php-src is that bool 
is an appropriate return value for "is" or "has" style functions, which return 
a yes/no answer. zend_result is an appropriate return value for functions that 
perform some operation that may succeed or fail.

I think that's a pretty reasonable state of things, and don't think there is a 
need to change it.

Regards,
Nikita

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

[Nikita Popov]

On Thu, Nov 10, 2022, at 14:29, Christoph M. Becker wrote:
> On 09.11.2022 at 23:27, Nikita Popov wrote:
> 
> > It looks like GitHub has just added support for private security reports:
> > https://github.blog/changelog/2022-11-09-privately-report-vulnerabilities-to-repository-maintainers/
> >
> > I haven't looked into the details, but it probably makes sense to enable
> > those on php-src and make this our official venue for security bug reports.
> > This would allow retiring the last remaining use of bugs.php.net (well,
> > apart from the archive of old issues, which should of course remain).
> 
> I agree, but maybe the security team is in favor of sticking with
> bugs.php.net.

I noticed that the php-src repo does enable private vulnerability reports now, 
and there is one sitting around without response at 
https://github.com/php/php-src/security/advisories/GHSA-54hq-v5wp-fqgv.

Possibly this was enabled unintentionally / without coordination with the 
security team? That should probably either be disabled again, or someone needs 
to keep an eye on it.

Regards,
Nikita

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

[Nikita Popov]

On Fri, Nov 19, 2021 at 9:44 PM Stanislav Malyshev 
wrote:

> Hi!
>
>  > 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.
>  >
>  > If the plan is to move to GH anyways, this could solve security
> reporting.
>
> Not familiar with it, but on the initial look it seems it could work,
> with one caveat. We have a ton of reports which aren't security issues
> and some which need to be discussed before we are sure which one is that.
>
> We could do it on the list, of course, but that creates the same dangers
> as mentioned before - too easy to lose info in an un-archived ML.
> --
> Stas Malyshev
> smalys...@gmail.com
>

It looks like GitHub has just added support for private security reports:
https://github.blog/changelog/2022-11-09-privately-report-vulnerabilities-to-repository-maintainers/

I haven't looked into the details, but it probably makes sense to enable
those on php-src and make this our official venue for security bug reports.
This would allow retiring the last remaining use of bugs.php.net (well,
apart from the archive of old issues, which should of course remain).

Regards,
Nikita

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

[Nikita Popov]

On Fri, Nov 4, 2022 at 3:26 PM Ilija Tovilo  wrote:

> Hi everyone
>
> I'd like to propose a simple RFC to introduce looking up class
> constants by name. We have dedicated syntax for basically all other
> language constructs. This RFC aims to get rid of this seemingly
> arbitrary limitation.
>
> https://wiki.php.net/rfc/dynamic_class_constant_fetch
>
> Please let me know if you have any thoughts.
>

The proposal looks reasonable to me. While I wouldn't expect this syntax to
see much use, it does remove a syntactical inconsistency in the language.

> Order of execution

See
https://www.npopov.com/2017/04/14/PHP-7-Virtual-machine.html#writes-and-memory-safety
for why the order of execution is the way it is. As class constants do not
support writes, these concerns do not apply, and the "normal" order can be
used (as you propose).

Regards,
Nikita

Re: [PHP-DEV] RFC Idea - is_json - looking for feedback

[Nikita Popov]

On Fri, Jul 29, 2022 at 4:27 PM juan carlos morales <
dev.juan.mora...@gmail.com> wrote:

> I am following the RFC guideline for the first time. (
> https://wiki.php.net/rfc/howto)
>
> As suggested there, I am here to get a feeling from you, regarding the
> following RFC for PHP.
>
> # Change (draft):
>
> New function in php called like:
>
> is_json(string $string): bool
>
> ## Description
> ### Parameters
> string $string -> string to find out if is a valid JSON or not
>
> ### Return
> Returns a bool. The function is capable to determine if the passed string
> is a valid JSON (true) or not (false).
>
> # Why this function ?
>
> At the moment the only way to determine if a JSON-string is valid we have
> to execute the json_decode() function.
>
> The drawback about this, is that json_decode() generates an in memory an
> object/array (depending on parameters) while parsing the string; this leads
> to a memory usage that is not needed (because we use memory for creating
> the object/array) and also can cause an error for reaching the memory-limit
> of the php process.
>
> Sometimes we just need to know is the string is a valid json or not, and
> nothing else.
>
> # Do we need something like this? If a check to an string is valid JSON
> then for sure I will have to use it in my code either as an object or an
> array.
>
> Well that is not true. There are plenty of cases where you just need to
> check if a string is a valid json and that is it. Just looking into
> stackoverflow will give you an idea about how many people is looking for
> something like this in an efficient way.
>

Could you please give some specific examples where the proposed
functionality would be useful?

Regards,
Nikita

Re: [PHP-DEV] [RFC] [VOTE] Constants in traits

[Nikita Popov]

On Sun, Jul 10, 2022 at 3:46 PM Rowan Tommins 
wrote:

> On 10/07/2022 13:51, Björn Larsson via internals wrote:
> > I think it's quite unlikely to deprecate such a rather big feature and
> > from that perspective I think one should do it as good as possible.
> >
> > Even if one thinks that this is a bad feature not to be expanded, why
> > not try to make it work better? So, I hope this RFC passes!
>
>
> I agree.
>
> Having sat down and read through the RFC, it is extremely conservative
> in its scope, and presents a clear case that it will fix a source of
> bugs in things that people can already do, i.e. reference constants
> inside a trait definition.
>
> It seems very unlikely that this change will make people suddenly use
> traits in more "wrong" places, nor prevent any alternative horizontal
> reuse / composition aid features being added in future.
>

I tend to agree. While I strongly dislike traits (or at least our
implementation of them), they're here to stay and we should make them less
bad where we can. Adding support for constants in traits makes sense to me,
because it removes an arbitrary limitation and inconsistency, and removes
the need for people to work around this in ways that are much worse -- for
example, by having an implicit contract between the trait and the using
class, as shown in the RFC.

Regards,
Nikita

Re: [PHP-DEV] Removing Travis CI

[Nikita Popov]

On Sun, Jul 10, 2022 at 10:07 PM Ayesh Karunaratne  wrote:

> Dear Internals,
> Historically, we have been using Travis CI for our automated tests,
> but since 2021 June, travis-ci.org has ceased operations, and no
> longer runs any builds. There was an Internals discussion
> (https://externals.io/message/112709) to move to the successor,
> travis-ci.com, but I don't think we ever moved there.
>
> Quoting Nikita from that thread:
>
> > We haven't been using Travis as our primary CI for a while already. We
> use
> > AppVeyor for Windows testing and Azure Pipelines for everything else. The
> > only thing Travis is still used for is a daily cron job that tests PHP on
> > "exotic" architectures like aarch64 and s390x. Having those builds is a
> > nice to have, but not particularly critical.
>
> As far as I see, Travis does not run php-src builds anymore; neither
> on push, nor on cron.
>
> https://travis-ci.org/github/php/php-src leads to a page that says the
> project was moved to travis-ci.com, and the linked page
> (https://travis-ci.com/php/php-src) throws a 404. I think we have now
> fully moved to GitHub Actions (thanks to amazing efforts by Ilija) +
> Azure Pipelines + Appveyor + Circle CI, so perhaps it's time we remove
> all the Travis-related code from php-src? I'd gladly volunteer for it,
> if we reach a consensus to remove it.
>

We use Travis CI to test architectures that are not available through other
CI providers, in particular aarch64 and s390x. Testing aarch64 is very
important, because we provide a JIT implementation for it. Testing s390x is
useful, because it is a big-endian architecture.

The build page you are looking for is
https://app.travis-ci.com/github/php/php-src. Just like other CI runs, it
is linked from GitHub's commit CI interface, so I'm not sure why you got
the impression that it does not run builds.

It's unfortunate that we need to use multiple CI providers, because a
single one does not offer all relevant architectures and operating systems
(Cirrus CI is used for their FreeBSD support).

Regards,
Nikita

Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3

[Nikita Popov]

On Thu, Jun 9, 2022 at 9:39 PM Marco Pivetta  wrote:

> Hey Nikita,
>
> On Thu, 9 Jun 2022 at 21:35, Nikita Popov  wrote:
>
>> On Thu, Jun 9, 2022 at 9:29 PM Marco Pivetta  wrote:
>>
>>>
>>> On Thu, 9 Jun 2022 at 21:27, Nikita Popov  wrote:
>>>
>>>> On Thu, Jun 9, 2022 at 8:15 PM Arnaud Le Blanc 
>>>> wrote:
>>>>
>>>>> > Would that allow us to get rid of `static fn () {` declarations, when
>>>>> > creating one of these closures in an instance method context?
>>>>>
>>>>> It would be great to get rid of this, but ideally this would apply to
>>>>> Arrow
>>>>> Functions and Anonymous Functions as well. This could be a separate
>>>>> RFC.
>>>>>
>>>>
>>>> I've tried this in the past, and this is not possible due to implicit
>>>> $this uses. See
>>>> https://wiki.php.net/rfc/arrow_functions_v2#this_binding_and_static_arrow_functions
>>>> for a brief note on this. The tl;dr is that if your closure does "fn() =>
>>>> Foo::bar()" and Foo happens to be a parent of your current scope and bar()
>>>> a non-static method, then this performs a scoped instance call that
>>>> inherits $this. Not binding $this here would result in an Error exception,
>>>> but the compiler doesn't have any way to know that $this needs to be bound.
>>>>
>>>> Regards,
>>>> Nikita
>>>>
>>>
>>> Hey Nikita,
>>>
>>> Do you have another example? Calling instance methods statically is...
>>> well... deserving a hard crash :|
>>>
>>
>> Maybe easier to understand if you replace Foo::bar() with parent::bar()?
>> That's the most common spelling for this type of call.
>>
>> I agree that the syntax we use for this is unfortunate (because it is
>> syntactically indistinguishable from a static method call, which it is
>> *not*), but that's what we have right now, and we can hardly just stop
>> supporting it.
>>
>
> Dunno, it's a new construct, so perhaps we could do something about it.
> I'm not suggesting we change the existing `fn` or `function` declarations,
> but in this case, we're introducing a new construct, and some work already
> went in to do the eager discovery of by-val variables.
>
> Heck, variable variables already wouldn't work here, according to this RFC
> :D
>

We're not introducing a new construct. We're just extending existing fn()
functions to accept {} blocks, with exactly the same semantics as before. I
would find it highly concerning if fn() => X and fn() => { return X; } had
differences in capture semantics. Those two expressions should be strictly
identical -- the former should be desugared to the latter.

Nikita

Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3

[Nikita Popov]

On Thu, Jun 9, 2022 at 9:29 PM Marco Pivetta  wrote:

>
> On Thu, 9 Jun 2022 at 21:27, Nikita Popov  wrote:
>
>> On Thu, Jun 9, 2022 at 8:15 PM Arnaud Le Blanc 
>> wrote:
>>
>>> Hi,
>>>
>>> On jeudi 9 juin 2022 18:46:53 CEST Marco Pivetta wrote:
>>> > ## nesting these functions within each other
>>> >
>>> > What happens when/if we nest these functions? Take this minimal
>>> example:
>>> >
>>> > ```php
>>> > $a = 'hello world';
>>> >
>>> > (fn () {
>>> > (fn () {
>>> > echo $a;
>>> > })();
>>> > })();
>>> > ```
>>>
>>> Capture bubbles up. When an inner function uses a variable, the outer
>>> function
>>> in fact uses it too, so it's captured by both functions, by-value.
>>>
>>> This example prints "hello world": The inner function captures $a from
>>> the
>>> outer function, which captures $a from its declaring scope.
>>>
>>> This is equivalent to
>>>
>>> ```php
>>> (function () use ($a) {
>>> (function () use ($a) {
>>> echo $a;
>>> })();
>>> })();
>>> ```
>>>
>>> > ## capturing `$this`
>>> >
>>> > In the past (also present), I had to type `static fn () => ...` or
>>> `static
>>> > function () { ...` all over the place, to avoid implicitly binding
>>> `$this`
>>> > to a closure, causing hidden memory leaks.
>>> >
>>> > Assuming following:
>>> >
>>> >  * these new closures could capture `$this` automatically, once
>>> detected
>>> >  * these new closures can optimize away unnecessary variables that
>>> aren't
>>> > captured
>>> >
>>> > Would that allow us to get rid of `static fn () {` declarations, when
>>> > creating one of these closures in an instance method context?
>>>
>>> It would be great to get rid of this, but ideally this would apply to
>>> Arrow
>>> Functions and Anonymous Functions as well. This could be a separate RFC.
>>>
>>
>> I've tried this in the past, and this is not possible due to implicit
>> $this uses. See
>> https://wiki.php.net/rfc/arrow_functions_v2#this_binding_and_static_arrow_functions
>> for a brief note on this. The tl;dr is that if your closure does "fn() =>
>> Foo::bar()" and Foo happens to be a parent of your current scope and bar()
>> a non-static method, then this performs a scoped instance call that
>> inherits $this. Not binding $this here would result in an Error exception,
>> but the compiler doesn't have any way to know that $this needs to be bound.
>>
>> Regards,
>> Nikita
>>
>
> Hey Nikita,
>
> Do you have another example? Calling instance methods statically is...
> well... deserving a hard crash :|
>

Maybe easier to understand if you replace Foo::bar() with parent::bar()?
That's the most common spelling for this type of call.

I agree that the syntax we use for this is unfortunate (because it is
syntactically indistinguishable from a static method call, which it is
*not*), but that's what we have right now, and we can hardly just stop
supporting it.

Regards,
Nikita

Re: [PHP-DEV] [RFC] Short Closures 2, aka auto-capture take 3

[Nikita Popov]

On Thu, Jun 9, 2022 at 8:15 PM Arnaud Le Blanc  wrote:

> Hi,
>
> On jeudi 9 juin 2022 18:46:53 CEST Marco Pivetta wrote:
> > ## nesting these functions within each other
> >
> > What happens when/if we nest these functions? Take this minimal example:
> >
> > ```php
> > $a = 'hello world';
> >
> > (fn () {
> > (fn () {
> > echo $a;
> > })();
> > })();
> > ```
>
> Capture bubbles up. When an inner function uses a variable, the outer
> function
> in fact uses it too, so it's captured by both functions, by-value.
>
> This example prints "hello world": The inner function captures $a from the
> outer function, which captures $a from its declaring scope.
>
> This is equivalent to
>
> ```php
> (function () use ($a) {
> (function () use ($a) {
> echo $a;
> })();
> })();
> ```
>
> > ## capturing `$this`
> >
> > In the past (also present), I had to type `static fn () => ...` or
> `static
> > function () { ...` all over the place, to avoid implicitly binding
> `$this`
> > to a closure, causing hidden memory leaks.
> >
> > Assuming following:
> >
> >  * these new closures could capture `$this` automatically, once detected
> >  * these new closures can optimize away unnecessary variables that aren't
> > captured
> >
> > Would that allow us to get rid of `static fn () {` declarations, when
> > creating one of these closures in an instance method context?
>
> It would be great to get rid of this, but ideally this would apply to
> Arrow
> Functions and Anonymous Functions as well. This could be a separate RFC.
>

I've tried this in the past, and this is not possible due to implicit $this
uses. See
https://wiki.php.net/rfc/arrow_functions_v2#this_binding_and_static_arrow_functions
for a brief note on this. The tl;dr is that if your closure does "fn() =>
Foo::bar()" and Foo happens to be a parent of your current scope and bar()
a non-static method, then this performs a scoped instance call that
inherits $this. Not binding $this here would result in an Error exception,
but the compiler doesn't have any way to know that $this needs to be bound.

Regards,
Nikita

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

[Nikita Popov]

On Sat, May 28, 2022 at 11:44 AM Ilija Tovilo 
wrote:

> Hi everyone
>
> I'd like to start a discussion on a simple RFC to allow fetching
> properties in constant expressions.
> https://wiki.php.net/rfc/fetch_property_in_const_expressions
>
> The RFC proposes adding support for fetching properties in constant
> expressions using the -> operator. I'm looking forward to your
> feedback.
>
> Regards,
> Ilija
>

This looks like a reasonable addition.

Could there be any expectation that if -> works, ?-> does as well?

Regards,
Nikita

Re: [PHP-DEV] Change the values of the LOCK_* constants

[Nikita Popov]

On Sun, Apr 24, 2022 at 12:14 PM Ilija Tovilo 
wrote:

> Hi everyone
>
> The issue was raised that PHPs LOCK_* constants don't match the Unix
> LOCK_* constants.
> https://github.com/php/php-src/pull/8429
>
> // Unix
> #define LOCK_SH 1
> #define LOCK_EX 2
> #define LOCK_NB 4
> #define LOCK_UN 8
>
> // PHP
> #define PHP_LOCK_SH 1
> #define PHP_LOCK_EX 2
> #define PHP_LOCK_UN 3
> #define PHP_LOCK_NB 4
>
> Essentially, in PHPs binary representation UN doesn't get its own bit,
> but is instead represented as 0b11. I'm guessing the reasoning was
> that SH, EX and UN must not be combined, while they can all be
> combined with NB. This avoids additional error handling when multiple
> of those bits were to be set.
>
> However, this has a downside of making checking of bits harder and
> different from how you would do it in other languages.
> https://3v4l.org/41ebV
>
> We could update the PHP constants to match the Unix values of those
> constants. Unfortunately, there seems to be a not insignificant number
> of usages of flock with hard-coded integer values.
>
>
> https://sourcegraph.com/search?q=context:global+file:%5C.php%24+count:10+flock%5C%28%5C%24%5Ba-zA-Z0-9_%5D%2B%2C+%5B0-9%5D%2B%5C%29=regexp
>
> (The regex engine of sourcegraph is flaky, but the majority of results
> are correct)
>
> The process of replacing these hard-coded values could be partially
> automated with a few caveats.
>
> 1. The value must be direct ($flags = 1; flock($file, $flags); would not
> work)
> 2. The migration script would assume that flock is a global and not
> local function
>
> Overall, I'm not completely sure this change is worth it since flock
> flags are just passed and not read.
>
> Let me know what you think.
>
> Ilija
>

I think the current state of things here makes perfect sense. I might help
to think of it as a structure of the form:

struct {
unsigned lock_type : 2;
unsigned non_blocking : 1;
}

The first member of that structure is not a bitmask -- the three options
are mutually exclusive, and doing something like LOCK_SH | LOCK_UN is
semantically meaningless.

Consulting https://man7.org/linux/man-pages/man2/flock.2.html, nothing on
the flock() man page suggests that LOCK_SH, LOCK_EX and LOCK_UN can be used
as bitflags -- it so happens that they can in C, but this is not an API
guarantee. The kernel code for these flags handles things properly by first
removing the LOCK_NB flag and then doing equality comparisons against the
lock type -- not flag checks. Incidentally these get mapped to F_RDLCK,
F_WRLCK and F_UNLCK internally, which just so happen to have the same
values as LOCK_SH, LOCK_EX and LOCK_UN in PHP ;)

Is there some kind of evidence that people are actually trying to use these
as bitflags, and you're trying to solve a real problem here? Or is the only
problem being solved that somebody is celebrating their own ignorance and
incompetence over at r/lolphp again?

Regards,
Nikita

Re: [PHP-DEV] Timezone Rules, which dataset to pick?

[Nikita Popov]

On Thu, Apr 7, 2022 at 11:34 AM Derick Rethans  wrote:

> Hi!
>
> As you might be aware, I maintain the date time support in PHP. As part
> of that I regularly have to update the rules that timezones employ -
> changes in Daylight Saving Time rules, or other changes to rules due to
> political foibles.
>
> In the last few years, the maintainer of the Iana TZ Data project has
> diverged somewhat from the consensus of the community, and degraded some
> data by no longer having an entry for each country and merged timezones
> where data does not differ since 1970. (Removing transitions from these
> regions where data **does** differ before 1970, even if these were
> available).
>
> Java's date/time maintainer has created a fork based on the original
> Iana TZ data to put back some of the removed/deprecated data to better
> serve their users, and I would think that this is also best suited as a
> data set for PHP.
>
> If you want to read about the intricacies, see:
> https://github.com/JodaOrg/global-tz#rationale
>
> But this does mean a divergence from the "official" TZ data, although
> Joda's data is arguably better. My recommendation is that from the 2022b
> release we switch to Joda's version. (I will today merge in the 2022a
> data from the Iana source.)
>
> Comments?
>
> If you want to discuss this live, come find me in "Room 11":
> https://chat.stackoverflow.com/rooms/11/php
>
> cheers,
> Derick
>

Keeping in mind that people deploying PHP on Linux usually end up using
OS-provided zoneinfo, do you know which source distros base that on? I
think we should follow the distro-consensus here, whatever that may be.

Regards,
Nikita

Re: [PHP-DEV] [RFC] Embedding multiple PHP engines in a single thread

[Nikita Popov]

On Tue, Feb 1, 2022 at 3:36 AM  wrote:

> Hello internals,
>
> As you know, the PHP codebase makes heavy use of global variables. In ZTS
> builds, access to these globals are cleverly mapped to thread local storage
> via macros. To my knowledge, the limitation here is that there's no way to
> run multiple PHP engines in a single thread. (Please let me know if I'm
> missing something.)
>
> Naturally this is an extremely slim corner case. It would however unlock
> some interesting things like user-land zend_extensions and SAPIs without
> spawning threads needlessly. It would also enable one to build a
> coroutine-based SAPI[1].
>
> I'm curious if there's been any previous discussion around this, or if
> anyone has general feedback before I take a shot at this.
>
> My rough idea is to modify TSRM to support multiple
> `tsrm_tls_entry.storage` per thread, keyed by some caller-supplied id.
> Currently a thread-safe resource is accessed like
> `thread[thread_id].storage[resource_id]`. In my idea it would be
> `thread[thread_id].storage[storage_id][resource_id]` with some API function
> to push/pop `storage_id`. Any thoughts on that approach?
>
> Support for non-ZTS builds would be rad but would require much larger code
> changes.
>
> Feedback appreciated.
>
> Thank you,
>
> Adam
>
> [1] for example, to call into PHP concurrently from Go's green-threads
> (which may share a single OS thread)
>

In places we also use real thread-local storage (ZEND_TLS) -- actually,
this is the preferred method to manage global state if it is possible.
Nowadays TSRM exists essentially only to work around deficiencies in
cross-DLL TLS support on Windows. I don't think your scheme could extend to
those cases.

I'm not really convinced that it's worthwhile to invest effort in this
directly.

Regards,
Nikita

Re: [PHP-DEV] Long-Term Planning for PHP 9.0 Error Promotion

[Nikita Popov]

On Sun, Jan 30, 2022 at 5:41 PM Christian Schneider 
wrote:

> Am 30.01.2022 um 16:55 schrieb Nikita Popov :
> > Something I want to add here is that there is also an important technical
> > motivation behind promoting undefined variable notices to exceptions: The
> > big problem with these (from a pure implementation perspective) is that
> we
> > need to throw the warning and continue running. But the warning might
> call
> > a custom error handler, which may modify state that the virtual machine
> > does not expect to be modified. The PHP VM plays increasingly complex
> games
> > to prevent this, but despite all that complexity, this problem cannot be
> > fully solved while this remains a warning, rather than an exception.
>
>
> Just so it has been mentioned: This could also be addressed by removing
> the warning (-:C
>
> Out of curiosity: If undefined variables would be turned into exceptions,
> is the fact that now almost every operation can throw an exception that
> much easier to handle?
>

Almost every operation can *already* throw an exception -- if something
throws a warning, then that warning may be promoted to an exception by a
custom error handler.

But yes, dealing with exceptions is much simpler, because they abort
control flow.

Regards,
Nikita

Re: [PHP-DEV] Long-Term Planning for PHP 9.0 Error Promotion

[Nikita Popov]

On Tue, Jan 25, 2022 at 12:47 AM Mark Randall  wrote:

> Internals,
>
> PHP 9.0, likely a few years away at this point, is our next opportunity
> to make significant breaking changes.
>
> So I thought it would be appropriate to start a thread discussing what
> breaking changes we might want to include in it, specifically in
> relation to error handling behaviour both at engine level, and
> potentially library level.
>
> By discussing and passing RFCs sooner rather than later, end users and
> library maintainers will have much more advanced notice than if we
> waited until 8.4 had released.
>
> My goal is to help coordinate putting forth a set of individual RFCs, or
> maybe a collective set similar to
> https://wiki.php.net/rfc/engine_warnings that will specifically target
> PHP 9.0, even though that version does not yet have a release date, or
> even a release year.
>
> Nothing in this conversation will preclude others from passing
> additional RFCs in the future years that also target PHP 9 error
> promotion, or, for that matter, reversing those decisions potentially
> made here, if necessary.
>
>
> For my part I will be putting forward two votes which will hopefully
> complete the migration process started in Nikita's engine warnings RFC:
>
>
> ** Undefined Variables Promoted to Error **
>
> PHP currently treats reading an undefined variable as though it were a
> null, emitting a warning message in the process. This was previously
> promoted from a notice in the PHP 8 engine warnings RFC.
>
> At the time a 3 way vote was held between promoting to an error
> exception, a warning, or leaving it as a notice.
>
> At the time, 56% voted in favour of throwing an Error, 28% in favour of
> a warning, and the remainder leaving it as a notice.
>
> My understanding is that many of those who voted to raise it to a
> warning did so because they felt that jumping straight from a notice to
> an Error was too much in one go.
>
> As it will have been a warning for around 5 years by the time PHP 9 is
> released, I expect that there will now be a healthy super majority to
> bump this up to throwing an error.
>

Something I want to add here is that there is also an important technical
motivation behind promoting undefined variable notices to exceptions: The
big problem with these (from a pure implementation perspective) is that we
need to throw the warning and continue running. But the warning might call
a custom error handler, which may modify state that the virtual machine
does not expect to be modified. The PHP VM plays increasingly complex games
to prevent this, but despite all that complexity, this problem cannot be
fully solved while this remains a warning, rather than an exception.

Same goes for other warnings in the engine of course, undefined variables
are just the biggest offender, because this particular warning can occur as
part of nearly any operation. The additional complexities that arise when
you combine this problem with a JIT compiler are left as an exercise to the
reader.

Regards,
Nikita

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

[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

[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()

[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

[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

[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

[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

[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

[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

[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

[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()

[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

[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

[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

[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

[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

[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()

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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()

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

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

[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

[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

[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

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

[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

[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

[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

[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