Re: [PHP-DEV] [RFC][Vote] Property Hooks
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
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
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
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
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()
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
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
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
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
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 <http://www.php.net/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 <http://www.php.net/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 > <https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures>, > which are problematic for the reasons outlined in the “Deprecate functions > with overloaded signatures > <https://wiki.php.net/rfc/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
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)
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?
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?
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
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
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]
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
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
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
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
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
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
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
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
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?
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
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
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
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
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
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()
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 > > <https://github.com/libgd/libgd/pull/671> to libgd > > <https://github.com/libgd/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 > > <https://github.com/php/php-src/pull/7026> into PHP's bundled gd fork, > > and adding > > AVIF awareness <https://github.com/php/php-src/pull/7091> 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 > > <https://github.com/php/php-src/pull/5127#issuecomment-799825277> 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 > > <https://github.com/php/php-src/pull/5127#issuecomment-976241397> to use > > getimagesize() to return the actual size. So a colleague has kindly > > created standalone > > code <https://aomedia-review.googlesource.com/c/libavifinfo/+/148321> > (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] <https://github.com/php/php-src/pull/7711> > 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
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
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
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
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
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
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
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()
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
Re: [PHP-DEV] Add ReflectionFunctionAbstract::isAnonymous()
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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
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
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
ibrary 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 mechanism to warn you about upcoming changes without requiring immediate action, in reality open-source libraries have to treat deprecations as immediate breakage. One open-source project with a slightly different attitude towards deprecations is Symfony, where deprecation notices during test runs are aggregated and considered quite ordinary. Of course Symfony does address PHP-related deprecations before release, but this happens gradually. We were able to run the Symfony test suite just fine during most of the PHP 8.1 development phase, much unlike most other projects, which would crash and burn on encountering the first "tentative return types" deprecation. The reason why I'm going off on this tangent: I believe that even if PHP followed semver to the letter, I don't think the migration burden for open-source libraries would change to any appreciable degree. If we dropped all non-deprecation changes from PHP 8.1, the upgrade would be a bit easier, but not by much. This is not where the main cost is. I'm not sure what we can do about that though. Sure, we could stop with the (runtime) deprecations, only document the change and then directly implement it at the next major version. That would be much simpler for us (generating runtime deprecations is actually a major implementation pain, as well as a big performance concern) and would make minor version upgrades for libraries much simpler. However, it also removes the ability to address problems before they turn into fatal errors, e.g. by recording any stray deprecation warnings in production. There was certainly a big outcry over the handful of backwards-incompatible changes in PHP 8.0 that did not previously trigger deprecation warnings. Regards, Nikita
Re: [PHP-DEV] run-tests SKIPIF caching can be problematic for third-party extensions
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
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
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
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
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?
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
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
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
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?
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
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
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
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