Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Derick, In any case, I don't mind this — I'm actually going to suggest to change > the constructor to: > > public function __construct(DateTimeInterface $start, DateInterval > $interval, DateTimeInterface|int $end, int $options = 0) {} > > And then *only* add: > > public static function createFromISO8601String(string $specification, int > $options = 0): static {} > > This solves the original problem of not being able to define the > signatures in stubs, and also extracts the most problematic of methods > into a factory method where it should always have belonged. > Thanks for your response, I've just updated the RFC accordingly. It solves the main issue indeed, and we still have the possibility to add a second factory method using the $recurrences parameter later if we deem it useful at some point. With all that said, I don't want to update the RFC anymore, so I plan to start the vote on Monday. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Mon, 19 Jun 2023, Nicolas Grekas wrote: > > > On my side, I'd very much prefer keeping the constructor of > > > DatePeriod and thus making it non-overloaded with this signature: > > > > > > public function __construct(DateTimeInterface $start, DateInterval > > > $interval, DateTimeInterface|int $end, int $options = 0) {} > > > > That still has an overloaded third argument — DateTimeInterface|int. > > > > Where you trying to suggest to change the current existing > > __construct() to: > > > > public function __construct(DateTimeInterface $start, DateInterval > > $interval, DateTimeInterface $end, int $options = 0) > > > > and then add these two new factory methods? > > > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > > int $recurrences, int $options = 0) > > > > createFromISO8601String(string $specification, int $options = 0) > > > > I really meant DateTimeInterface|int, not an overloaded signature but a > union type. You may call it a union type, but that's still an overloaded signature :-) > My concern is about providing the best path forward. Both deprecating > and providing the alternative in the same version creates a lot of > friction for end users. > > After a quick chat with Mate, I think we should do this union type *in > addition* to adding the new static constructors. > > Then, later on, if future us think it's worth it, we might want to > consider deprecating passing an integer. > > This would provide a path for BC/FC I would fully support. In any case, I don't mind this — I'm actually going to suggest to change the constructor to: public function __construct(DateTimeInterface $start, DateInterval $interval, DateTimeInterface|int $end, int $options = 0) {} And then *only* add: public static function createFromISO8601String(string $specification, int $options = 0): static {} This solves the original problem of not being able to define the signatures in stubs, and also extracts the most problematic of methods into a factory method where it should always have belonged. There would be no additional benefit in creating the other two factory methods. cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Juliette, Respectfully though, in my opinion selectively leaving out impact analysis > without mentioning why they are missing in the RFC, reeks of trying to hide > information which could influence the vote. > Maybe just mentioning why the impact analysis is missing in these cases in > the RFC could take that stench away ? > I respectfully disagree that it would affect the vote result, but I added the clarification you suggested nevertheless. :) Doing so resulted in a very nice side-effect: I noticed that the array_keys_search() function name is slightly inconsistent/misleading, since what it really does is to filter keys based on their *value* (thus the $filter_value param name). So I renamed it to array_keys_filter() as well as came up with a new suggested replacement which is compatible with at least PHP 4: using the combination of array_keys() and array_filter() retains the original behavior (example is added to the RFC). I know that it's less than ideal performance-wise, but it works anyway (and not all code is performance sensitive). Thanks, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 19-6-2023 23:16, Máté Kocsis wrote: Hi, The impact analysis on userland code seems to be missing for some of the proposals, most notably for the proposals which I expect will have the highest impact. I'd like to ask for an impact analysis to be added to each of these: * array_keys() * ReflectionProperty::setValue() These are intentionally left out due to different reasons: * Even though the 1 parameter version of array_keys() is used a lot, its 2+ parameter signature is much less known. Personally, I've never used it. Of course, this doesn't mean no one uses it, but I don't think this deprecation will really matter in practice, so I didn't care to write a script for this (the rest of the deprecations were easy to analyze via regexes). * ReflectionProperty::setValue() would be a lot of work to analyze since it's very difficult to track whether the setValue() method is called on a ReflectionProperty instance. Not to mention the fact that the deprecation only affects function invocations where either a single parameter is provided or static properties where the first parameter is not null or an object instance. Since the suggested alternative is basically available since forever, I think it's OK not to do an exhaustive analysis in this case. For the `get*_class()` deprecation, I wonder if an additional impact analysis is needed for packages which may not have removed usages of `get_class( null )`, which would now be double-impacted (and not caught by the current analysis). I'm not sure I can follow you, but get_class(null) is a TypeError since 8.0 (and have been warning since 7.2). If a package didn't feel the need to migrate its get_class() usages then it is likely a dead project. And if someone tries to upgrade their (proprietary) code from PHP 7.1 to PHP 8.3 directly then they will notice the exception first and also - if they followed the upgrading notes carelly - will learn the relevant suggestions regarding the deprecation. But realistically speaking, these projects will most probably be bound by lots of other more severe issues, so I don't think the above is a viable upgrade path... Regards, Máté Thanks for the reply Máté. I understand what you are saying and appreciate it may be difficult to get an accurate impact analysis for `ReflectionProperty::setValue()`. Respectfully though, in my opinion selectively leaving out impact analysis without mentioning why they are missing in the RFC, reeks of trying to hide information which could influence the vote. Maybe just mentioning why the impact analysis is missing in these cases in the RFC could take that stench away ? Smile, Juliette
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi, The impact analysis on userland code seems to be missing for some of the > proposals, most notably for the proposals which I expect will have the > highest impact. I'd like to ask for an impact analysis to be added to > each of these: > * array_keys() > * ReflectionProperty::setValue() > These are intentionally left out due to different reasons: * Even though the 1 parameter version of array_keys() is used a lot, its 2+ parameter signature is much less known. Personally, I've never used it. Of course, this doesn't mean no one uses it, but I don't think this deprecation will really matter in practice, so I didn't care to write a script for this (the rest of the deprecations were easy to analyze via regexes). * ReflectionProperty::setValue() would be a lot of work to analyze since it's very difficult to track whether the setValue() method is called on a ReflectionProperty instance. Not to mention the fact that the deprecation only affects function invocations where either a single parameter is provided or static properties where the first parameter is not null or an object instance. Since the suggested alternative is basically available since forever, I think it's OK not to do an exhaustive analysis in this case. > For the `get*_class()` deprecation, I wonder if an additional impact > analysis is needed for packages which may not have removed usages of > `get_class( null )`, which would now be double-impacted (and not caught > by the current analysis). > I'm not sure I can follow you, but get_class(null) is a TypeError since 8.0 (and have been warning since 7.2). If a package didn't feel the need to migrate its get_class() usages then it is likely a dead project. And if someone tries to upgrade their (proprietary) code from PHP 7.1 to PHP 8.3 directly then they will notice the exception first and also - if they followed the upgrading notes carelly - will learn the relevant suggestions regarding the deprecation. But realistically speaking, these projects will most probably be bound by lots of other more severe issues, so I don't think the above is a viable upgrade path... Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > > I'm going to nitpick on the newly suggested names and argument order > for > > > the > > > DatePeriod factory methods — althoughI do agree that they need to get > > > created: > > > > > > createFromInterval(DateTimeInterface $start, DateInterval $interval, > > > DateTimeInterface $end, int $options = 0) > > > → createWithRange(DateTimeInterface $begin, DateTimeInterface $end, > > > DateTimeInterface $int, int $options = 0) > > > > > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > > > int $recurrences, int $options = 0) > > > → createWithRecurrences(DateInterval $begin, int $recurrences, > > > DateInterval $interval, int $options = 0) > > > > > > We also should fix the argument names. Either $start/$finish, or > > > $begin/$end. I > > > prefer the latter. > > > > > > createFromIso8601(string $specification, int $options = 0) > > > -> createFromISO8601String > > > > > > I am open to bike shedding about this :-) > > > > > > > On my side, I'd very much prefer keeping the constructor of DatePeriod > and > > thus making it non-overloaded with this signature: > > > > public function __construct(DateTimeInterface $start, DateInterval > > $interval, DateTimeInterface|int $end, int $options = 0) {} > > That still has an overloaded third argument — DateTimeInterface|int. > Where you trying to suggest to change the current existing __construct() > to: > > public function __construct(DateTimeInterface $start, DateInterval > $interval, DateTimeInterface $end, int $options = 0) > > and then add these two new factory methods? > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > int $recurrences, int $options = 0) > > createFromISO8601String(string $specification, int $options = 0) > I really meant DateTimeInterface|int, not an overloaded signature but a union type. My concern is about providing the best path forward. Both deprecating and providing the alternative in the same version creates a lot of friction for end users. After a quick chat with Mate, I think we should do this union type *in addition* to adding the new static constructors. Then, later on, if future us think it's worth it, we might want to consider deprecating passing an integer. This would provide a path for BC/FC I would fully support. Nicolas
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 27-4-2023 23:28, Máté Kocsis wrote: Hi Internals, As you have possibly already experienced, overloaded signatures cause various smaller and bigger issues, while the concept is not natively supported by PHP. That's why I drafted an RFC which intends to phase out the majority of overloaded function/method signatures and also forbid the introduction of such functions in the future: https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures I'm not sure at all what the best solution would be to get rid of the overloaded FFI methods, so I hope that someone comes up with a better idea. Currently, the RFC suggests deprecating and then removing the static methods in question, but I hope that we can find a more elegant approach. So this section is incomplete and it's just for starting the discussion. Regards, Máté The impact analysis on userland code seems to be missing for some of the proposals, most notably for the proposals which I expect will have the highest impact. I'd like to ask for an impact analysis to be added to each of these: * array_keys() * ReflectionProperty::setValue() For the `get*_class()` deprecation, I wonder if an additional impact analysis is needed for packages which may not have removed usages of `get_class( null )`, which would now be double-impacted (and not caught by the current analysis). Smile, Juliette
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Wed, Jun 14, 2023 at 4:06 AM Derick Rethans wrote: > > On Tue, 13 Jun 2023, Nicolas Grekas wrote: > > > > I'm going to nitpick on the newly suggested names and argument order for > > > the > > > DatePeriod factory methods — althoughI do agree that they need to get > > > created: > > > > > > createFromInterval(DateTimeInterface $start, DateInterval $interval, > > > DateTimeInterface $end, int $options = 0) > > > → createWithRange(DateTimeInterface $begin, DateTimeInterface $end, > > > DateTimeInterface $int, int $options = 0) > > > > > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > > > int $recurrences, int $options = 0) > > > → createWithRecurrences(DateInterval $begin, int $recurrences, > > > DateInterval $interval, int $options = 0) > > > > > > We also should fix the argument names. Either $start/$finish, or > > > $begin/$end. I > > > prefer the latter. > > > > > > createFromIso8601(string $specification, int $options = 0) > > > -> createFromISO8601String > > > > > > I am open to bike shedding about this :-) > > > > > > > On my side, I'd very much prefer keeping the constructor of DatePeriod and > > thus making it non-overloaded with this signature: > > > > public function __construct(DateTimeInterface $start, DateInterval > > $interval, DateTimeInterface|int $end, int $options = 0) {} > > That still has an overloaded third argument — DateTimeInterface|int. This specific case is not that problematic. Since things need untangling anyway, I would feel free to improve it if something is done, but it's not much of a problem. The problem is when there's a wildly different signature, or if args change the return type structure, etc. So in this case, the problem is really: public function __construct(string $isostr, int $options = 0) {} Which can't really be unified with the others very well. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 13 Jun 2023, Nicolas Grekas wrote: > > I'm going to nitpick on the newly suggested names and argument order for > > the > > DatePeriod factory methods — althoughI do agree that they need to get > > created: > > > > createFromInterval(DateTimeInterface $start, DateInterval $interval, > > DateTimeInterface $end, int $options = 0) > > → createWithRange(DateTimeInterface $begin, DateTimeInterface $end, > > DateTimeInterface $int, int $options = 0) > > > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > > int $recurrences, int $options = 0) > > → createWithRecurrences(DateInterval $begin, int $recurrences, > > DateInterval $interval, int $options = 0) > > > > We also should fix the argument names. Either $start/$finish, or > > $begin/$end. I > > prefer the latter. > > > > createFromIso8601(string $specification, int $options = 0) > > -> createFromISO8601String > > > > I am open to bike shedding about this :-) > > > > On my side, I'd very much prefer keeping the constructor of DatePeriod and > thus making it non-overloaded with this signature: > > public function __construct(DateTimeInterface $start, DateInterval > $interval, DateTimeInterface|int $end, int $options = 0) {} That still has an overloaded third argument — DateTimeInterface|int. Where you trying to suggest to change the current existing __construct() to: public function __construct(DateTimeInterface $start, DateInterval $interval, DateTimeInterface $end, int $options = 0) and then add these two new factory methods? createFromRecurrences(DateTimeInterface $start, DateInterval $interval, int $recurrences, int $options = 0) createFromISO8601String(string $specification, int $options = 0) That works for me. cheers, Derick -- https://derickrethans.nl | https://xdebug.org | https://dram.io Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support Host of PHP Internals News: https://phpinternals.news mastodon: @derickr@phpc.social @xdebug@phpc.social twitter: @derickr and @xdebug -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Nicolas, On my side, I'd very much prefer keeping the constructor of DatePeriod and > thus making it non-overloaded with this signature: > > public function __construct(DateTimeInterface $start, DateInterval > $interval, DateTimeInterface|int $end, int $options = 0) {} > > That'd help a lot with the migration/BC path and this signature also just > makes sense. > I completely agree with this (more so since this used to be the original proposal), so I'll ask Derick again about his stance on this. Unrelated: I don't think adding "ReflectionMethod::createFromMethodName" is > useful. Using explode is just fine for those that need this style. And > since this is the recommended migration path, better not give a reason to > change twice the code. > I don't really mind removing ReflectionMethod::createFromMethodName, so if anyone has a use-case which heavily relies on creating the reflection method from a callable name ("Class::method" format), now is the time to speak up for this method. As far as I see the relevant impact analysis file ( https://gist.github.com/kocsismate/b457f8fef074039b58fbee9121d05e92), Laminas, Symfony and Nette DI would also be impacted by the removal. I don't think that it causes a big problem to explode the string instead, but I'm wondering if performance sensitive code may be negatively affected (?). Thank you, Nicolas, for your insights! Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > > As you have possibly already experienced, overloaded signatures cause > > various smaller and bigger issues, while the concept is not natively > > supported by PHP. That's why I drafted an RFC which intends to phase > > out the majority of overloaded function/method signatures and also > > forbid the introduction of such functions in the future: > > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures > > I'm going to nitpick on the newly suggested names and argument order for > the > DatePeriod factory methods — althoughI do agree that they need to get > created: > > createFromInterval(DateTimeInterface $start, DateInterval $interval, > DateTimeInterface $end, int $options = 0) > → createWithRange(DateTimeInterface $begin, DateTimeInterface $end, > DateTimeInterface $int, int $options = 0) > > createFromRecurrences(DateTimeInterface $start, DateInterval $interval, > int $recurrences, int $options = 0) > → createWithRecurrences(DateInterval $begin, int $recurrences, > DateInterval $interval, int $options = 0) > > We also should fix the argument names. Either $start/$finish, or > $begin/$end. I > prefer the latter. > > createFromIso8601(string $specification, int $options = 0) > -> createFromISO8601String > > I am open to bike shedding about this :-) > On my side, I'd very much prefer keeping the constructor of DatePeriod and thus making it non-overloaded with this signature: public function __construct(DateTimeInterface $start, DateInterval $interval, DateTimeInterface|int $end, int $options = 0) {} That'd help a lot with the migration/BC path and this signature also just makes sense. Unrelated: I don't think adding "ReflectionMethod::createFromMethodName" is useful. Using explode is just fine for those that need this style. And since this is the recommended migration path, better not give a reason to change twice the code. I like everything else in the RFC. I'm going to go with the "short path" of deprecating things because the migration paths look smooth enough, thanks for taking the time to explain them all (I just have a concern with the DatePeriod migration path, which would be fixed with my suggestion bove.) Thanks! Nicolas
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > It looks like the current preferred signature was only introduced in PHP > 8.2. Previously, the signatures were dba_fetch($key, $handle) and > dba_fetch($key, $skip, $handle) - it effectively had a non-final optional > parameter. > Yes, that's the case! Thanks for reminding me again, I clarified in the RFC that PHP 8.2 is the first version which supports the preferred signature. > Whether that makes it too early to deprecate the older 3-parameter form, > I'm not sure. As you say, it's probably pretty rare, particularly because > according to the docs that parameter is ignored for most file formats the > function supports anyway. > Normally, I wouldn't do this, but I'm certain that deprecating this signature won't cause a big havoc in the PHP ecosystem, so that's the only reason I went ahead. >
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Sat, 10 Jun 2023 at 08:40, Máté Kocsis wrote: > In the meanwhile, I noticed yet another overloaded function (dba_fetch()) > and I included it into the proposal. In my opinion, it's a no-brainer > to deprecate due to its low adoption rate and very odd behavior - to say at > least. > It looks like the current preferred signature was only introduced in PHP 8.2. Previously, the signatures were dba_fetch($key, $handle) and dba_fetch($key, $skip, $handle) - it effectively had a non-final optional parameter. Whether that makes it too early to deprecate the older 3-parameter form, I'm not sure. As you say, it's probably pretty rare, particularly because according to the docs that parameter is ignored for most file formats the function supports anyway. Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hey Everyone, In the meanwhile, I noticed yet another overloaded function (dba_fetch()) and I included it into the proposal. In my opinion, it's a no-brainer to deprecate due to its low adoption rate and very odd behavior - to say at least. The RFC evolved a lot during the discussion phase, thanks for everyone who was involved! I believe my proposal is ready now, as I don't intend to change anything in it anymore. This means that I'll start the vote mid-next week, unless new concerns emerge. Thanks, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 30 May 2023 at 09:54, Claude Pache wrote: > Any change that introduce a BC break or a migration burden must be > justified. (There has been enough rant around justified BC breaks, so let’s > not introduce unjustified ones.) > > What is the purpose of changing the return convention of IntlCalendar > methods, and is it worth? > We are talking about adding a new method here, so I don't really see how this is a BC break. One would hope that people do not just blindly change function names, especially considering that one would need to look at the migration guide, where it can be explicitly noted that the return type is different from the previous method. Moreover, the return convention of IntlCalendar (and most other Intl classes) has for the most par always been wrong. Prior to PHP 8.0, passing invalid types to functions/methods was considered undefined behaviour, and in general the value returned when a TypeError wasn't thrown was NULL. However, Intl (and some other extensions) didn't follow this and returned false on ZPP errors and always true otherwise. As such, the change to *always* throw TypeErrors on ZPP violations has brought to light those sorts of extensions. This was one of the main motivation to add the true singleton type other than consistency, as this allows tools like static analysis, but not limited to them, to detect dead code and bring this up to people's attention. As effectively, there is no point in storing and/or comparing the value of a method/function that always returns the same value (be that null(/void), true, false, or other). Considering this fact, that one should not compare the return value of most (if not all) of these methods, using void as the type seems very much appropriate rather than using true. Best regards, George P. Banyard
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 2 June 2023 14:21:49 BST, "Máté Kocsis" wrote: >I hope that I don't sound elitist, but codebases not using static >analysis... are kind of hopeless... Then I guess we should just pack up and go home, because right now PHP doesn't even have an official static analyser, let alone a mandatory one; and the evolution of Hack shows just how much the language would need to change for such a tool to guarantee full coverage. The needs of new users being introduced to best practices are different from those maintaining and modernising existing codebases. Sometimes, they're in direct opposition, but I think we have a duty to try to support both where we can. > The purpose is to have the "correct" return type. Is it worth it? Well, it > home > depends... For me, what's important is that PHP becomes a more and > more predictable and accessible language, and I care less about minimizing > backward compatibility breaks. I think Claude is taking the same premise, and reaching a different conclusion: returning true is consistent with the other methods on the class, and that consistency makes it more predictable and more accessible. Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 17 May 2023 08:35:18 BST, "Máté Kocsis" wrote: >Particularly, I have been wondering for a long time, why the original >function includes "save_handler" in its name? >The passed in handlers are not just "save", but also other kinds of >handlers (e.g. "read"). So I'm considering to use >something like "session_set_handlers()" or >"session_set_handler_callbacks()". What do you think about these names? I think that's a very good point, and I like the explicitness of "session_set_handler_callbacks" - this isn't a name people are going to need to type often, so there's not much reason to keep it short. If we go down that route, perhaps we should come up with a corresponding name for the object based version - "session_set_handler_object" perhaps? That would also mean the deprecation messages can be much simpler: if you're using the old name, you need to do something. (There's another minor fringe benefit: once the old name is removed, it becomes available for users to polyfill if for some reason they're struggling to change code that calls it.) I'm still in two minds on the general concept of this RFC, as it is placing a burden on users for the mostly minor convenience of maintainers; but I think you've done a good job responding to concerns and improving the proposal. Regards, Hi Máté, Sorry I didn't get round to replying to this sooner, particularly this point: -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Claude, The very risk is that the programmer must be aware that the return > convention has changed. Recall that not everyone run static analysers, > especially for such a trivial migration, and that not every code is > statically analysable (e.g.. `$foo->$bar()`). > I hope that I don't sound elitist, but codebases not using static analysis... are kind of hopeless... That is, in my opinion, there's no other option to run a larger PHP project successfully than to either use static analysis or to have programmers who don't commit any mistakes. For the rest of the less valuable projects, the cost of messing up with the return value check is not much. What is the purpose of changing the return convention of IntlCalendar > methods, and is it worth? > The purpose is to have the "correct" return type. Is it worth it? Well, it depends... For me, what's important is that PHP becomes a more and more predictable and accessible language, and I care less about minimizing backward compatibility breaks. The introduction of the "true" type was useful, since we now have more correct type information, but adding new functions with this return type would be unfortunate, as many people (especially the ones coming from other languages) would be puzzled about the meaning of the "true" type more than if the return type was "void". I'm pretty sure that George - the author of the true type - agrees that the main purpose of this type is to eliminate its declarations from internal functions and methods. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Máté, > Le 30 mai 2023 à 01:41, Máté Kocsis a écrit : > > Hi Claude, > >> The replacement methods for IntlCalendar::set() (namely >> IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a >> return type of `void`, but `true`, like the original method, for the two >> following reasons: >> >> 1. By changing the returned value, you are introducing an unnecessary hazard >> when migrating code. >> >> 2. Per https://www.php.net/IntlCalendar, all modification methods of that >> class (clear(), roll(), setTime(), etc.) consistently return `true` on >> success and `false` on failure, even when the method is infallible (and thus >> would always return `true`). Don’t break consistency. > > 1. I don't see much risk with this change, since we clearly indicate at the > very beginning that the new methods return void, so programmers migrating > their code > should pay attention to the return types. IDEs and static analysers can > surely warn them in case of inattention. The very risk is that the programmer must be aware that the return convention has changed. Recall that not everyone run static analysers, especially for such a trivial migration, and that not every code is statically analysable (e.g.. `$foo->$bar()`). Of course, the benefice of the change might be worth the risk; but that leads to the second point: > > 2. Some of the methods you mentioned have a "true" return type for a few > weeks now. The biggest motivation for introducing these was to at least have > some chance to > make them void in the future. Doing so is a risky move indeed, so should be > carried out very carefully, if it's possible at all... That's why I consider > it beneficial to spare the > effort and start with a clean slate by adding the new methods in question > with their correct return type. Concretely, if you change the return value (from `true` to `void/null`), you will break the following pattern: setBar(...)) { // error handling (might be dead code, it doesn’t matter) } ?> Any change that introduce a BC break or a migration burden must be justified. (There has been enough rant around justified BC breaks, so let’s not introduce unjustified ones.) What is the purpose of changing the return convention of IntlCalendar methods, and is it worth? —Claude
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Claude, The replacement methods for IntlCalendar::set() (namely > IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a > return type of `void`, but `true`, like the original method, for the two > following reasons: > > 1. By changing the returned value, you are introducing an unnecessary > hazard when migrating code. > > 2. Per https://www.php.net/IntlCalendar, all modification methods of that > class (clear(), roll(), setTime(), etc.) consistently return `true` on > success and `false` on failure, even when the method is infallible (and > thus would always return `true`). Don’t break consistency. > 1. I don't see much risk with this change, since we clearly indicate at the very beginning that the new methods return void, so programmers migrating their code should pay attention to the return types. IDEs and static analysers can surely warn them in case of inattention. 2. Some of the methods you mentioned have a "true" return type for a few weeks now. The biggest motivation for introducing these was to at least have some chance to make them void in the future. Doing so is a risky move indeed, so should be carried out very carefully, if it's possible at all... That's why I consider it beneficial to spare the effort and start with a clean slate by adding the new methods in question with their correct return type. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> Le 27 avr. 2023 à 23:28, Máté Kocsis a écrit : > > Hi Internals, > > As you have possibly already experienced, overloaded signatures cause > various smaller and bigger issues, while the concept is not natively > supported by PHP. That's why I drafted an RFC which intends to phase out > the majority of overloaded function/method signatures and also forbid > the introduction of such functions in the future: > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures > > Hi Máté, The replacement methods for IntlCalendar::set() (namely IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a return type of `void`, but `true`, like the original method, for the two following reasons: 1. By changing the returned value, you are introducing an unnecessary hazard when migrating code. 2. Per https://www.php.net/IntlCalendar, all modification methods of that class (clear(), roll(), setTime(), etc.) consistently return `true` on success and `false` on failure, even when the method is infallible (and thus would always return `true`). Don’t break consistency. —Claude
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Everyone, Yesterday, I updated the voting choices based on Rowan's suggestions. I also picked a few signatures where only one migration path is proposed (the short one). Now, I'd be curious what to do with the FFI methods in question ( https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures#fficast_ffinew_and_ffitype). .. Especially, it would be very nice to hear about FFI users' opinions. Also, there was feedback about the session_set_save_handlers() function name that it shouldn't resemble this much to the original session_set_save_handler(). Rowan suggested using the session_set_save_handler_functions() name. While I'm not a big fan of this choice, I'd be interested in if there is any other idea? Particularly, I have been wondering for a long time, why the original function includes "save_handler" in its name? The passed in handlers are not just "save", but also other kinds of handlers (e.g. "read"). So I'm considering to use something like "session_set_handlers()" or "session_set_handler_callbacks()". What do you think about these names? If we manage to answer these questions then I'd like to start the vote not long afterwards. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, May 16, 2023, at 2:26 PM, Derick Rethans wrote: > On 15 May 2023 13:38:56 GMT-05:00, Larry Garfield > wrote: >>On Mon, May 15, 2023, at 6:36 PM, Rowan Tommins wrote: >>> On 15 May 2023 09:54:41 BST, "G. P. B." wrote: >>> Why are we assuming that PHP 9.0 is going to come after PHP 8.4? >>> >>> Historically, PHP has had a major release roughly every five years. The >>> main exception is PHP 6, which was never released - but whose major >>> features became PHP 5.3, five years after 5.0, and six before 7.0 >>> >>> I think planning a rough timeline is more useful to users and >>> contributors than waiting until there's some exciting headline feature. >>> Otherwise, it becomes tempting to sneak in breaking changes in 8.x >>> because "we don't know how soon 9.0 is", or to have a rush of changes >>> because "we've only just decided 9.0 is soon". >>> >>> It also helps avoid putting a release number on an experimental feature >>> that might never arrive, as with Unicode strings in 6.0; or that might >>> turn out to be less important to most users than other changes, like >>> the JIT in 8.0. >> >>I agree entirely. Setting reasonable expectations for users to plan around, >>such as a known 5-years-per-major cycle, helps end users far more than >>"whelp, we did something big, version number time!" >> >>Tangent: If I were to put together an RFC that set out such a 5 year cycle >>expectation with reasonable guidelines around when things could be >>deprecated, would anyone actually support it? >> >>--Larry Garfield >> > > I would. With some remarks. > > I think that having a 5 year cycle for major releases is beneficial. > > First to users as they know when all the deprecations are being rolled > up into behavioural changes and (potential) breakage. > > But also to us, as maintainers, as it could signal *when* it makes > sense to work on deprecations, behaviour changes, and code refactoring > (that's more likely to break APIs and extensions). > > IMO it would make sense to tweak what sort of user land deprecations > can be done in the .3 and .4 releases (probably being more strict) > compared to the .1-.2 releases. We'd want to enlarge thr time frame for > significant changes more ahead of time compared to more minor ones. > > But I do think we ought to be weary of making these rules, in when to > allow to deprecate what, too strict. A rough set of guidelines makes > IMO more sense. > > cheers > Derick What would be a reasonable heuristic for "gotta do in .1 or .2 to give people time" deprecations vs "this is minor enough that it's OK to do in .3 or .4" deprecations? "Amount of code impacted" is not great, as that's extremely hard to determine (as we've seen). But I'm not sure of a better metric. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 15 May 2023 13:38:56 GMT-05:00, Larry Garfield wrote: >On Mon, May 15, 2023, at 6:36 PM, Rowan Tommins wrote: >> On 15 May 2023 09:54:41 BST, "G. P. B." wrote: >> >>>Why are we assuming that PHP 9.0 is going to come after PHP 8.4? >> >> Historically, PHP has had a major release roughly every five years. The >> main exception is PHP 6, which was never released - but whose major >> features became PHP 5.3, five years after 5.0, and six before 7.0 >> >> I think planning a rough timeline is more useful to users and >> contributors than waiting until there's some exciting headline feature. >> Otherwise, it becomes tempting to sneak in breaking changes in 8.x >> because "we don't know how soon 9.0 is", or to have a rush of changes >> because "we've only just decided 9.0 is soon". >> >> It also helps avoid putting a release number on an experimental feature >> that might never arrive, as with Unicode strings in 6.0; or that might >> turn out to be less important to most users than other changes, like >> the JIT in 8.0. > >I agree entirely. Setting reasonable expectations for users to plan around, >such as a known 5-years-per-major cycle, helps end users far more than "whelp, >we did something big, version number time!" > >Tangent: If I were to put together an RFC that set out such a 5 year cycle >expectation with reasonable guidelines around when things could be deprecated, >would anyone actually support it? > >--Larry Garfield > I would. With some remarks. I think that having a 5 year cycle for major releases is beneficial. First to users as they know when all the deprecations are being rolled up into behavioural changes and (potential) breakage. But also to us, as maintainers, as it could signal *when* it makes sense to work on deprecations, behaviour changes, and code refactoring (that's more likely to break APIs and extensions). IMO it would make sense to tweak what sort of user land deprecations can be done in the .3 and .4 releases (probably being more strict) compared to the .1-.2 releases. We'd want to enlarge thr time frame for significant changes more ahead of time compared to more minor ones. But I do think we ought to be weary of making these rules, in when to allow to deprecate what, too strict. A rough set of guidelines makes IMO more sense. cheers Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Sat, 13 May 2023 at 00:08, Larry Garfield wrote: > > That means it's impossible to write code that works from 8.2 to 9.0 without > version checks. The overlap period is only 2 releases (8.3 and 8.4). That's > too short of a window. That it is too short, is an opinion. Having one version where both the old version of the function, and the new version work is the minimum (imo) as it allows people to run the same code on both. But anything more than that is a nicety, not a requirement. Also, in my opinion. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 16 May 2023 at 12:03, G. P. B. wrote: > > Because this proposal of "let's do majors every 5 years" now reduces what > we can do, as we cannot (or should not) deprecate stuff in X.0, as it makes > it harder for people to upgrade to the new major (see how we postponed the > 8.0 deprecation RFC to 8.1 after various comments). > Again, the solution to "people don't like deprecation messages" is not "don't deprecate things", it's "improve how deprecation messages work". There should be absolutely zero reason not to deprecate something in an X.0 release, or even in an X.0.15 release, because a deprecation message form the code should serve the same purpose as, and have similar impact to, a note in the documentation. > But apparently we cannot deprecate anything in X.3 or X.4 because there > will "never" be a X.5 again and this is "too short" of a time frame. > I don't think that's what anyone's saying. They're saying that if we have a predictable release cycle, we can decide for each case between a one-to-two-year deprecation, and a five-to-six-year deprecation. If we don't have any idea when PHP 9.0 will come out, then we can't even begin to discuss that, we have to say "the deprecation period will last anywhere between one year and ten years, depending when we get round to a major release". I don't think that helps anybody. > Considering, this was partially the argument used against Max Kellerman > with the header changes: "this should have been done for PHP 7.0 or PHP > 8.0". > Having a fixed release cycle actually allows this to be turned around to something more productive: "we could consider this for PHP 9.0, which will happen in 2 years time". Without it, all we can say is "we could consider this for PHP 9.0, but you have to regularly read the internals mailing list for several years to get a hint of when that might be". Note that prior to PHP 5.4, we had this same "when things are ready" approach to tagging *any* release. Since then, we've had an annual release cycle, which I think has worked well in terms of both contributors and users being able to plan ahead. My understanding as to *why* the JIT caused the major bump was because of > major changes within the Zend Engine, not necessarily the feature in itself. > I mentioned JIT, because around this time in the 7.x life cycle, I was regularly having this same debate, and people used "everyone knows PHP 8 will be whenever the JIT is ready" as though it was more important than any other planning we might want to do. I disagreed then, and I disagree now. > And again, the only "good" reason I see for a major bump is something like converting streams from resources to objects, as this is a massive shift and has the possibility to cause issues for a lot of codebases. Given that we changed a whole list of resources to objects in PHP 8.1, it's clearly not considered that massive a shift to most people https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.resource2object Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Mon, 15 May 2023 at 19:39, Larry Garfield wrote: > Tangent: If I were to put together an RFC that set out such a 5 year cycle > expectation with reasonable guidelines around when things could be > deprecated, would anyone actually support it? > No, as this doesn't solve the problem. Best regards, George P. Banyard
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Mon, 15 May 2023 at 19:36, Rowan Tommins wrote: > On 15 May 2023 09:54:41 BST, "G. P. B." wrote: > > >Why are we assuming that PHP 9.0 is going to come after PHP 8.4? > > Historically, PHP has had a major release roughly every five years. The > main exception is PHP 6, which was never released - but whose major > features became PHP 5.3, five years after 5.0, and six before 7.0 > > I think planning a rough timeline is more useful to users and contributors > than waiting until there's some exciting headline feature. Otherwise, it > becomes tempting to sneak in breaking changes in 8.x because "we don't know > how soon 9.0 is", or to have a rush of changes because "we've only just > decided 9.0 is soon". > > It also helps avoid putting a release number on an experimental feature > that might never arrive, as with Unicode strings in 6.0; or that might turn > out to be less important to most users than other changes, like the JIT in > 8.0. > I totally agree that we shouldn't use a major number as an exciting feature flag. However, if what we want is a consistent timeline for how long something gets deprecated before being removed, then let's just do like Python and state that the deprecation exists for two consecutive releases and will be removed in the third one. Because this proposal of "let's do majors every 5 years" now reduces what we can do, as we cannot (or should not) deprecate stuff in X.0, as it makes it harder for people to upgrade to the new major (see how we postponed the 8.0 deprecation RFC to 8.1 after various comments). But apparently we cannot deprecate anything in X.3 or X.4 because there will "never" be a X.5 again and this is "too short" of a time frame. This, IMHO, is terrible for the project's maintenance. As this basically says to someone who wants to start contributing to the project by deprecating some weird behaviour that they may be told off with "you should have done this X years ago" or "wait another Y years". People cannot magically know when they should shove aside any other priorities in life just to be able to make *some* change to the language. Considering, this was partially the argument used against Max Kellerman with the header changes: "this should have been done for PHP 7.0 or PHP 8.0". Those sorts of statements alienate new (and old) contributors, and looking at the state of the projects where we are still very much thin on maintainers, anything that does this is from my POV a no-go. My understanding as to *why* the JIT caused the major bump was because of major changes within the Zend Engine, not necessarily the feature in itself. And again, the only "good" reason I see for a major bump is something like converting streams from resources to objects, as this is a massive shift and has the possibility to cause issues for a lot of codebases. Best regards, George P. Banyard
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> On 15 May 2023, at 19:51, Rowan Tommins wrote: > > On 15 May 2023 19:38:56 BST, Larry Garfield wrote: > >> I agree entirely. Setting reasonable expectations for users to plan around, >> such as a known 5-years-per-major cycle, helps end users far more than >> "whelp, we did something big, version number time!" > >> Tangent: If I were to put together an RFC that set out such a 5 year cycle >> expectation with reasonable guidelines around when things could be >> deprecated, would anyone actually support it? > > A big yes from me, but I've not had the most promising responses when banging > that drum in the past. > Hi, As an end user this would be very useful and address a lot of queries when planning and budgeting for PHP developments, especially if major breaking changes were also in .0 versions as I saw suggested elsewhere in a recent thread (also from Larry I think). I've been spending a lot of time reading around and thinking after the "Future Stability of PHP Thread" last month and think that this level of predictability would do a lot to mitigate issues (though appreciate it would have other consequences). Best wishes, Matt
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 15 May 2023 19:38:56 BST, Larry Garfield wrote: >Tangent: If I were to put together an RFC that set out such a 5 year cycle >expectation with reasonable guidelines around when things could be deprecated, >would anyone actually support it? A big yes from me, but I've not had the most promising responses when banging that drum in the past. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Mon, May 15, 2023, at 6:36 PM, Rowan Tommins wrote: > On 15 May 2023 09:54:41 BST, "G. P. B." wrote: > >>Why are we assuming that PHP 9.0 is going to come after PHP 8.4? > > Historically, PHP has had a major release roughly every five years. The > main exception is PHP 6, which was never released - but whose major > features became PHP 5.3, five years after 5.0, and six before 7.0 > > I think planning a rough timeline is more useful to users and > contributors than waiting until there's some exciting headline feature. > Otherwise, it becomes tempting to sneak in breaking changes in 8.x > because "we don't know how soon 9.0 is", or to have a rush of changes > because "we've only just decided 9.0 is soon". > > It also helps avoid putting a release number on an experimental feature > that might never arrive, as with Unicode strings in 6.0; or that might > turn out to be less important to most users than other changes, like > the JIT in 8.0. I agree entirely. Setting reasonable expectations for users to plan around, such as a known 5-years-per-major cycle, helps end users far more than "whelp, we did something big, version number time!" Tangent: If I were to put together an RFC that set out such a 5 year cycle expectation with reasonable guidelines around when things could be deprecated, would anyone actually support it? --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 15 May 2023 09:54:41 BST, "G. P. B." wrote: >Why are we assuming that PHP 9.0 is going to come after PHP 8.4? Historically, PHP has had a major release roughly every five years. The main exception is PHP 6, which was never released - but whose major features became PHP 5.3, five years after 5.0, and six before 7.0 I think planning a rough timeline is more useful to users and contributors than waiting until there's some exciting headline feature. Otherwise, it becomes tempting to sneak in breaking changes in 8.x because "we don't know how soon 9.0 is", or to have a rush of changes because "we've only just decided 9.0 is soon". It also helps avoid putting a release number on an experimental feature that might never arrive, as with Unicode strings in 6.0; or that might turn out to be less important to most users than other changes, like the JIT in 8.0. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Sat, 13 May 2023 at 00:08, Larry Garfield wrote: > That means it's impossible to write code that works from 8.2 to 9.0 > without version checks. The overlap period is only 2 releases (8.3 and > 8.4). That's too short of a window. > Why are we assuming that PHP 9.0 is going to come after PHP 8.4? There has been no decision as to when the new major is going to be released. And as far as I'm concerned, the only good reason to move from 8.x to 9.0 is when we convert streams from resources to objects, which is going to be a large enough BC that those sorts of minor BC breaks should frankly be a non-issue. So could we not speculate on an arbitrary timeline of the new major version being tagged and go on the merit of this proposal in isolation? I don't see much of an issue of, introduce the new functions/methods in PHP 8.3, deprecate them in PHP 8.3+x (for x > 0) and remove in PHP 9.0. Best regards, George P. Banyard
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 13 May 2023 00:08:22 BST, Larry Garfield >A better argument, I think: > >The old function exists in 8.2, the new one does not. >The new one exists in 8.3. >The old one ceases to exist in 9.0. > >That means it's impossible to write code that works from 8.2 to 9.0 without >version checks. The overlap period is only 2 releases (8.3 and 8.4). That's >too short of a window. Yes, that line of reasoning I find more persuasive. It's why I was working out earlier which changes could be polyfilled: it effectively extends the overlap period if it's trivial to include a definition somewhere wrapped in function_exists. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Fri, May 12, 2023, at 9:59 PM, Rowan Tommins wrote: > On 12 May 2023 19:17:20 BST, "Máté Kocsis" wrote: >>Libraries have to >>get rid of deprecated calls in any case, otherwise >>their users will surely start to complain. > > I've said it before, and I'll say it again: the solution to this is not > fewer deprecation messages; it's better documentation to stop people > confusing deprecations for errors, and better functionality for users > to choose which messages they see. A better argument, I think: The old function exists in 8.2, the new one does not. The new one exists in 8.3. The old one ceases to exist in 9.0. That means it's impossible to write code that works from 8.2 to 9.0 without version checks. The overlap period is only 2 releases (8.3 and 8.4). That's too short of a window. That's mitigated by these all being very uncommon cases, yes, but as a principle we should give people more warning than that. I am actually tempted to propose that we do deprecations at the very start of a release, 9.0 and 9.1 only, and then not allow them for the rest of that major, for that exact reason. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 12 May 2023 19:17:20 BST, "Máté Kocsis" wrote: >Libraries have to >get rid of deprecated calls in any case, otherwise >their users will surely start to complain. I've said it before, and I'll say it again: the solution to this is not fewer deprecation messages; it's better documentation to stop people confusing deprecations for errors, and better functionality for users to choose which messages they see. -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > So I would suggest rewording the options slightly: > > a) Deprecate in 8.3, remove in either 9.0 or 10.0 > b) Deprecate in 8.3, remove in 10.0 > c) Do not deprecate > > Now if the votes are a:5, b:4, c:4, we can say: > > - 9 people voted for deprecation in 8.3, vs 4 against > - only 5 voted for removal in 9.0, vs 8 against > - 9 voted for removal in 10.0, vs 4 against > > So we conclude that we should deprecate in 8.3, and remove in 10.0 > While this vote format looks a bit confusing for me at the first sight, I'm OK to apply it. Although, I don't fully agree with the "deprecate in 8.3 and remove in 10.0" part due to the reason I wrote about in my previous email. In my opinion, if we want to keep a signature for around 5-7 more years then we should really wait ~2 years with the deprecation so that people can react voluntarily first. Libraries have to get rid of deprecated calls in any case, otherwise their users will surely start to complain. If they do fix these calls in their PHP 8.x compatible code then there's much less reason to postpone the removal with 5 more years. The suggestion to narrow it down to a yes/no proposal in the discussion > phase is probably even better, though. > I agree with this, but so far there was no feedback which functions/methods people want to deprecate slower or faster, so I'm eager to hear about your and everyone else's opinion! Replying to Larry: That these particular functions and uses are quite rare doesn't really > change any of that; if anything it strengthens it, that we're willing to be > graceful about it even in cases where we could probably get away with not > being so. I don't think this "generosity" is really necessary, even though I appreciate your intention. For example, in case of ldap_connect(), the signature to be deprecated is not even documented at php.net... and it's only available on a very specific platform. I see no reason to keep this signature for 5+ years. The same goes for IntlGregorianCalendar::__construct() which seems to be a very underrepresented class as well. As I already said, most of the functionality to be removed is easy to replace, and it could be done via automation: i.e. you add Symfony Polyfill to your project and then Rector swaps the problematic function calls to the good ones. Sometimes not even tooling is needed, a simple search + replace is enough. With all that said, I think only a smaller subset of functions may deserve the longer depreciation period. Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Fri, May 12, 2023, at 6:52 AM, Rowan Tommins wrote: > I think it's actually very likely that voters would want to express > "deprecate, but do not remove before 10.0". Treating those votes as > "generally in favour, so enough to approve removal in 9.0" doesn't seem > appropriate. > > The other way around - "deprecate, but do not remove later than 9.0" - > seems less likely. I also don't see any reason to delay the deprecation > - the whole point of the longer period is to give people more notice. > > So I would suggest rewording the options slightly: > > a) Deprecate in 8.3, remove in either 9.0 or 10.0 > b) Deprecate in 8.3, remove in 10.0 > c) Do not deprecate > > Now if the votes are a:5, b:4, c:4, we can say: > > - 9 people voted for deprecation in 8.3, vs 4 against > - only 5 voted for removal in 9.0, vs 8 against > - 9 voted for removal in 10.0, vs 4 against > > So we conclude that we should deprecate in 8.3, and remove in 10.0 > > > The suggestion to narrow it down to a yes/no proposal in the discussion > phase is probably even better, though. Personally I'm on team "add new functions now, deprecate old ones in 9, remove old ones in 10." 1. As noted, it gives people a clean window in which to use the old or new ones without getting a deprecation warning, and without cutting off support for old but still supported PHP versions. 2. It gives people ample time to fix their code. (Around 7-8 years.) 3. Perhaps most importantly, it's a good-faith gesture to the people who have objected (validly, if sometimes impolitely and crudely) to shorter deprecation periods in the past. There is definitely a relationship building to be done with the broader user-space community, and offering a very wide grace period is a good way to help with that. That these particular functions and uses are quite rare doesn't really change any of that; if anything it strengthens it, that we're willing to be graceful about it even in cases where we could probably get away with not being so. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 11 May 2023 18:06:44 BST, Dan Ackroyd wrote: >On Thu, 11 May 2023 at 10:36, Tim Düsterhus wrote: >> >> I believe this vote format ("three options") is not really compatible >> with the voting rules (https://wiki.php.net/rfc/voting). >> >> For example it's not entirely clear what would happen here: >> >> 5 votes to deprecate in 8.3 / remove 9.0 >> 4 votes to deprecate in 9.0 / remove 10.0 >> 4 votes to not deprecate. > >The RFC author could just say that yes votes for deprecation and >eventual removal will be added together, with the timescale being a >preference vote. > >I think the only people who would object to that are people who would >vote yes to "deprecate and remove" but only if it matches their >preferred timescale, and would otherwise vote no. Which probably isn't >a thing. I think it's actually very likely that voters would want to express "deprecate, but do not remove before 10.0". Treating those votes as "generally in favour, so enough to approve removal in 9.0" doesn't seem appropriate. The other way around - "deprecate, but do not remove later than 9.0" - seems less likely. I also don't see any reason to delay the deprecation - the whole point of the longer period is to give people more notice. So I would suggest rewording the options slightly: a) Deprecate in 8.3, remove in either 9.0 or 10.0 b) Deprecate in 8.3, remove in 10.0 c) Do not deprecate Now if the votes are a:5, b:4, c:4, we can say: - 9 people voted for deprecation in 8.3, vs 4 against - only 5 voted for removal in 9.0, vs 8 against - 9 voted for removal in 10.0, vs 4 against So we conclude that we should deprecate in 8.3, and remove in 10.0 The suggestion to narrow it down to a yes/no proposal in the discussion phase is probably even better, though. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Dan and Tim, > The RFC author could just say that yes votes for deprecation and > eventual removal will be added together, with the timescale being a > preference vote. > Thank you, Dan, for the clarification, that was indeed what I meant. I'll also make this clear in the RFC, unfortunately I forgot it so far. … the following: I wanted to know if > "Deprecate in 8.x + not remove in 9.0, but only remove in 10.x or 11.x" Yes, this is also a possible choice. Actually, I'm sure there were multiple things deprecated in 5.x, which only got removed in PHP 8.0. At least this is what I remember. :) The reason why I still opted for the possibility of "deprecate in 9.0 and remove in 10.0", because in my opinion this choice would suit better some of the most widespread functionalities. Not deprecating them formally straightaway in 8.x versions, but only providing a suggested alternative instead of them, would first of all buy some time for people who don't yet want to deal with this problem at all, but still allow them to voluntarily migrate when they are ready without nagging them. Then they had a relatively long time period when we would warn them a bit louder. A full major release cycle should be more than enough time to perform the migration. With all that said, personally I don't think any of the affected functionalities (maybe with the exception of only one of them) warrant such a long phasing out period. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi On 5/11/23 18:58, Dan Ackroyd wrote: On Thu, 11 May 2023 at 10:36, Tim Düsterhus wrote: Would it be an option to just deprecate everything in PHP 8.3 As I said before, having at least one version where the new versions of the functions are available, and the old versions aren't giving deprecation notices, is the 'cleanest' way of refactoring functions. Right. Replace "8.3" by "the first version after the replacement is available" in my previous email. My main question was … Deprecation notices only deliver their value when the old versions of the functions are removed, and before then are a cost. It doesn't really feel right to decide on a deprecation for 9.0 / removal for 10.0 at the current time, but deprecation in 8.3 + removal when it's ready seems to be okay. I think I don't understand your concern. If a problem is discovered with a planned removal of something, people on this project are mostly reasonable*, and a small RFC to adjust to the new circumstances would be very likely to pass. … the following: I wanted to know if "Deprecate in 8.x + not remove in 9.0, but only remove in 10.x or 11.x" would be an option, because I feel that it is more useful than "If we deprecate in 8.x then we MUST remove in 9.0, that's why we're already making deprecation plans for 9.x, so we are able to remove in 10.0" All the discussion so far had "Deprecate in x.y and remove in x+1.0", but not "Deprecate in x.y and remove in x+n.0 with n > 1". Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Thu, 11 May 2023 at 10:36, Tim Düsterhus wrote: > > I believe this vote format ("three options") is not really compatible > with the voting rules (https://wiki.php.net/rfc/voting). > > For example it's not entirely clear what would happen here: > > 5 votes to deprecate in 8.3 / remove 9.0 > 4 votes to deprecate in 9.0 / remove 10.0 > 4 votes to not deprecate. The RFC author could just say that yes votes for deprecation and eventual removal will be added together, with the timescale being a preference vote. I think the only people who would object to that are people who would vote yes to "deprecate and remove" but only if it matches their preferred timescale, and would otherwise vote no. Which probably isn't a thing. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Thu, 11 May 2023 at 10:36, Tim Düsterhus wrote: > > Would it be an option to just deprecate everything in PHP 8.3 As I said before, having at least one version where the new versions of the functions are available, and the old versions aren't giving deprecation notices, is the 'cleanest' way of refactoring functions. Deprecation notices only deliver their value when the old versions of the functions are removed, and before then are a cost. > It doesn't really feel right to decide on > a deprecation for 9.0 / removal for 10.0 at the current time, but > deprecation in 8.3 + removal when it's ready seems to be okay. I think I don't understand your concern. If a problem is discovered with a planned removal of something, people on this project are mostly reasonable*, and a small RFC to adjust to the new circumstances would be very likely to pass. cheers Dan Ack * though sometimes they base their decisions on experiences/value different from my own, which makes it not feel like that. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi On 5/11/23 00:10, Máté Kocsis wrote: If not, then we could just move really slowly on deprecating them. We could deprecate them at some point in the 9.x release cycle and remove them in 10.0. Although I would love to get rid of as many overloaded signatures as possible due to the above mentioned fact, I admit that there might be some signatures which other internals would prefer to phase out slower than me. So thanks for the idea! Now, the individual votes in the RFC contain 2 options for removing a signature: a shorter path (deprecation in PHP 8.4, removal in PHP 9.0) and a longer one (deprecation in PHP 9.0 and removal in PHP 10.0). I believe this vote format ("three options") is not really compatible with the voting rules (https://wiki.php.net/rfc/voting). For example it's not entirely clear what would happen here: 5 votes to deprecate in 8.3 / remove 9.0 4 votes to deprecate in 9.0 / remove 10.0 4 votes to not deprecate. Neither option has a 2/3 majority over any other option. The two deprecation options together have a 2/3 majority over not deprecating, which might indicate that a deprecation should happen. But in which version? This would likely require ranked choice voting which feels complex for something that should be a simple vote. I've had a similar issue with my "Improve unserialize() error handling RFC" (https://externals.io/message/118566) and opted to make an executive decision as the RFC author and not offer one of the possible version targets to avoid that problem entirely. -- Repeating my previous question: Does it necessarily follow that a function that is deprecated in major X must be removed in major X+1? Otherwise deprecating with 8.3 and removing in 10 would be an option that could be evaluated when it's time for 9 and the migration did not yet "sufficiently" happen in userland code. Would it be an option to just deprecate everything in PHP 8.3 and defer the vote for the removal target version to a later date? This would give users the deprecation notice as early as possible and also gives them appropriate time to migrate if migration for a specific set of functions is (much) slower than others. It doesn't really feel right to decide on a deprecation for 9.0 / removal for 10.0 at the current time, but deprecation in 8.3 + removal when it's ready seems to be okay. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Derick and Tim, Derick, I incorporated most of your suggestions into the RFC with the sole exception of the argument names ($start/$finish,$begi/$end): it would feel odd if these parameters and their related properties were called differently: https://www.php.net/manual/en/class.dateperiod.php#dateperiod.props.start Actually, when we renamed the majority of parameters together with Nikita, this was one of few rules that we had: parameters should be called the same way as the property they initialize. > Please no consecutive uppercase letters in the function name. I find > createFromIso8601 (or createFromIso8601String if find the 'String' > suffix useful) much more readable than createFromISO8601String. > Uppercase acronyms are especially bad if followed by another ucfirst > word, because the word boundary is not immediately visible there. Not > the case here (it's digits), but still bad. Yes, generally, I wholeheartedly agree, but unfortunately I still "had to" go with Derick's suggestion of using consecutive uppercase letters because there is already a DateTime::setISODate() method with the same casing and I want to follow the established naming convention of ext/date. Personally, I like the "String" suffix, probably it makes the name better, but since I don't really care about this problem too much, I'd leave it for Derick to decide about it. Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Dan (and Larry and Tim who responded to the same topic), Would leaving the current versions in place and working actually cause > any maintenance issues? Yes, unfortunately they cause some maintenance issues: since they don't have a single signature, gen_stub.php generates the wrong signature for them in the manual. For example https://www.php.net/manual/en/function.stream-context-set-option.php displays two distinct signatures, however gen_stub.php only knows one signature, so obviously generation won't work well: function stream_context_set_option($context, array|string $wrapper_or_options, ?string $option_name = null, mixed $value = UNKNOWN): bool {} I use gen_stub.php for automatically detecting the outdated function/method/class signatures which I can then commit with minimal effort. Actually my doc-en PRs ( https://github.com/php/doc-en/pulls?page=1=is%3Apr+is%3Aclosed+author%3Akocsismate ) are either fixing outdated information in the manual, or ensuring that gen_stub.php can generate the same information that we display in the docs. Now, there are only ~50-60 function/method signatures in master which are not in line with the output of gen_stub.php, and most of them are due to overloaded signatures (the rest is due to PHP 8.3 changes which shouldn't be documented yet). For reference, the number of outdated function signatures used to be many hundreds or probably 1000+ when the project started, just before PHP 8.0 came out. If not, then we could just move really slowly on deprecating them. We > could deprecate them at some point in the 9.x release cycle and remove > them in 10.0. Although I would love to get rid of as many overloaded signatures as possible due to the above mentioned fact, I admit that there might be some signatures which other internals would prefer to phase out slower than me. So thanks for the idea! Now, the individual votes in the RFC contain 2 options for removing a signature: a shorter path (deprecation in PHP 8.4, removal in PHP 9.0) and a longer one (deprecation in PHP 9.0 and removal in PHP 10.0). Additionally, I wrote a very explicit and detailed list about the suggested BC compatible alternatives for each deprecation so that now it should be very clear how difficult it is to replace a function/method invocation. The RFC says: Impact analysis: 1 out of the 2000 most popular PHP packages rely on > calling session_set_save_handler() with 6 or more arguments. > I doubt analysing github is going to give a useful measure of the > impact of this RFC. Functions like session_set_save_handler are going > to be used in custom code written for a company than in shared > libraries. Yes, I'm aware of this glitch, but that's the best I could provide for estimating the impact of the deprecations. Cheers, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Rowan, > As an aside, I don't think "other deprecations are already more difficult" > is a good argument - it's like saying "yes, I punched him, but not as hard > as someone else already had". I think the change can be defended from other > angles, but wanted to call that out. > I admit that my argument was not a good one, but I intended to mean that most of the suggested alternatives are not too difficult to implement: since numerous other proposals were accepted in the past with a much more difficult upgrade path, I think this aspect of my RFC shouldn't be a deal breaker considering the benefits. Maybe because the function names are so similar? In general, I hate > variable and function names that differ only by an "s", it's far too easy > to misread or mistype them. Maybe "session_set_save_handler_functions"? To be honest, I'm not a fan of using the longer names (session_set_save_handler_functions and stream_context_set_option_array), especially the latter one looks weird to me, and it doesn't go well with stream_context_get_options(). Besides, a most interesting development is that I've just discovered the stream_context_get_params() and stream_context_set_params() functions which do nearly the same what the stream_context_*et_option functions do: the only difference between them is that the latter functions wrap the options inside the "options" key and also has an additional "notification" key. It *is* strictly related, though: the current function has two purposes: > get an option, and set an option; the RFC proposes to split that into two > functions, and there are three ways we can do that: Thanks for the idea, your explanation made sense to me, so finally, I removed assert_options() from my proposal and the deprecation of assert_options() relies on the PHP 8.3 deprecations RFC. Regards: Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tuesday 02 May 2023 15:07:21 (+02:00), Rowan Tommins wrote: > On Tue, 2 May 2023 at 13:20, Máté Kocsis wrote: > > > Yes, I agree that the assert_options() name is at least weird but I > > wouldn't like to > > include changes into this RFC which are not strictly related to overloaded > > signatures. Just like in case of implode(), the 1-parameter version of > > assert_options() > > could be added to the PHP 8.3/8.4 deprecations RFC though. > > > > > It *is* strictly related, though: the current function has two purposes: > get an option, and set an option; the RFC proposes to split that into two > functions, and there are three ways we can do that: > > 1) Keep the current name for get, come up with a new name for set > 2) Come up with a new name for get, keep the current name for set > 3) Come up with new names for both get and set > > I then looked further, and suggested: > > 4) Deprecate the existing function, but do not create any new functions; > instead, recommend ini_get for get, and ini_set for set > > All four options are direct remedies to the overloaded signature, and I > think due to the current unclear naming, options 3 and 4 are superior to > options 1 and 2. Do you have a specific reason to prefer option 1? I can't answer that question but checked especially option 4) as it looked promising to me. In the overall context, let's not forget this note [1]: > While assert_options() can still be used to control behaviour as described > above for backward compatibility reasons, PHP 7 only code should use the two > new configuration directives to control the behaviour of assert() and not > call assert_options(). That is zend.assertions and assert.exception only (of which only the second has a counterpart in ASSERT_EXCEPTION as the first can not be set at runtime). That means: Before investing too much thought which of all the options, IMHO the general route should be to deprecate assert_options() overall to prepare its removal (and with it a good share of the assert.options). >From signature / types perspective, ASSERT_CALLBACK option is not 100% >compatible w/ ini_get()/ini_set() as they don't support all types of the >callback pseudotype. but have not double checked this as I ran over the above >mentioned remark which I believe should be leading (the deprecation of the pre >PHP 7.0 assert/options alltogether). With the PHP 8.0 release this got some traction (upgrade of the assert callback signature and ASSERT_QUIET_EVAL option seems to be gone (perhaps in alignment with the improvements in the error handler callback signature on the level parameter that is now the error type regardless of error_level())). So perhaps this for the current RFC: 4) Deprecate the existing function, but do not create any new functions; instead, recommend the two configuration settings zend.assertions and assert.exception only, for those that can be set at runtime recommend ini_get()/ini_set(), for callback further investigate (does it still work?), for the moment I see no alternative to assert_options() for callback values not of type null or string (e.g. callabe array or callable object) as those are not supported by ini_get()/in_set(). -- hakre p.s. thank you for creating this RFC and taking care. [1]: https://www.php.net/manual/en/function.assert.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi On 5/5/23 18:12, Derick Rethans wrote: createFromIso8601(string $specification, int $options = 0) -> createFromISO8601String I am open to bike shedding about this :-) Okay, I'd like a different color of my bikeshed: Please no consecutive uppercase letters in the function name. I find createFromIso8601 (or createFromIso8601String if find the 'String' suffix useful) much more readable than createFromISO8601String. Uppercase acronyms are especially bad if followed by another ucfirst word, because the word boundary is not immediately visible there. Not the case here (it's digits), but still bad. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Thu, 27 Apr 2023, Máté Kocsis wrote: > As you have possibly already experienced, overloaded signatures cause > various smaller and bigger issues, while the concept is not natively > supported by PHP. That's why I drafted an RFC which intends to phase > out the majority of overloaded function/method signatures and also > forbid the introduction of such functions in the future: > https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures I'm going to nitpick on the newly suggested names and argument order for the DatePeriod factory methods — althoughI do agree that they need to get created: createFromInterval(DateTimeInterface $start, DateInterval $interval, DateTimeInterface $end, int $options = 0) → createWithRange(DateTimeInterface $begin, DateTimeInterface $end, DateTimeInterface $int, int $options = 0) createFromRecurrences(DateTimeInterface $start, DateInterval $interval, int $recurrences, int $options = 0) → createWithRecurrences(DateInterval $begin, int $recurrences, DateInterval $interval, int $options = 0) We also should fix the argument names. Either $start/$finish, or $begin/$end. I prefer the latter. createFromIso8601(string $specification, int $options = 0) -> createFromISO8601String I am open to bike shedding about this :-) cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, May 2, 2023, at 5:05 PM, Tim Düsterhus wrote: > Hi > > On 5/2/23 13:37, Dan Ackroyd wrote: >> For the functions that are having new separate methods added, not >> deprecating them immediately makes upgrading easier. When upgrading >> from one version of PHP to the next, it is best if you can run the >> same code on both versions, without any deprecation notices going off. > > Agree here, I consider it a good thing if there's at least one version > where both alternatives exist and are fully supported (i.e. without > emitting a deprecation notice). > > As an example, with PHP 8.3's addition of Randomizer::getFloat() and > Randomizer::getBytesFromString() there are useful replacements for > lcg_value() and uniqid() [1] respectively, but I'm intentionally not > proposing deprecating either of them before whatever comes after PHP 8.3. > > [1] The latter of which is likely one of the most misused functions in PHP. > >> Would leaving the current versions in place and working actually cause >> any maintenance issues? >> >> If not, then we could just move really slowly on deprecating them. We >> could deprecate them at some point in the 9.x release cycle and remove >> them in 10.0. > > Does it necessarily follow that a function that is deprecated in major X > must be removed in major X+1? Otherwise deprecating with 8.3 and > removing in 10 would be an option that could be evaluated when it's time > for 9 and the migration did not yet "sufficiently" happen in userland code. > > I can also see how that would be useful for the planned deprecation of > rand() and mt_rand(). Both of them have overloaded signatures, but will > be part of the other deprecation RFC, as they also have a myriad of > other issues that I consider to be worse than the signature. > Unfortunately they are pretty ubiquitous, but luckily do not cause > "internal" maintenance problems. So it would likely be feasible to keep > them until 10, as long as they emit the deprecation notice to slowly nag > developers away from them towards better alternatives. > > Best regards > Tim Düsterhus Given past pushback on changes that we've gotten, a low-cost/no-cost long deprecation period seems like it would be very appreciated. I've actually been considering proposing an RFC to mandate a minimum time length between deprecation and actual removal/change for that reason. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi On 5/2/23 13:37, Dan Ackroyd wrote: For the functions that are having new separate methods added, not deprecating them immediately makes upgrading easier. When upgrading from one version of PHP to the next, it is best if you can run the same code on both versions, without any deprecation notices going off. Agree here, I consider it a good thing if there's at least one version where both alternatives exist and are fully supported (i.e. without emitting a deprecation notice). As an example, with PHP 8.3's addition of Randomizer::getFloat() and Randomizer::getBytesFromString() there are useful replacements for lcg_value() and uniqid() [1] respectively, but I'm intentionally not proposing deprecating either of them before whatever comes after PHP 8.3. [1] The latter of which is likely one of the most misused functions in PHP. Would leaving the current versions in place and working actually cause any maintenance issues? If not, then we could just move really slowly on deprecating them. We could deprecate them at some point in the 9.x release cycle and remove them in 10.0. Does it necessarily follow that a function that is deprecated in major X must be removed in major X+1? Otherwise deprecating with 8.3 and removing in 10 would be an option that could be evaluated when it's time for 9 and the migration did not yet "sufficiently" happen in userland code. I can also see how that would be useful for the planned deprecation of rand() and mt_rand(). Both of them have overloaded signatures, but will be part of the other deprecation RFC, as they also have a myriad of other issues that I consider to be worse than the signature. Unfortunately they are pretty ubiquitous, but luckily do not cause "internal" maintenance problems. So it would likely be feasible to keep them until 10, as long as they emit the deprecation notice to slowly nag developers away from them towards better alternatives. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, May 2, 2023, at 12:20 PM, Máté Kocsis wrote: > As far as I see there are more deprecations which are easy to fix. In order > to see the whole picture > better, let's group the functions based on how a possible upgrade path > would look like: > > - With existing suggested alternative: > - get_class() and get_parent_class() > - pg_fetch_result(), pg_field_prtlen(), and pg_field_is_null() > - Phar::setStub() > - ReflectionMethod::__construct() > - ReflectionProperty::setValue() > - session_set_save_handler() (because the OO and procedural style are > interchangeable) > - stream_context_set_option() > - Polyfillable: > - assert_options() > - ldap_connect() > - ldap_exop() > - Requires conditional version check: > - DatePeriod::__construct() > - IntlCalendar::set() > - IntlGregorianCalendar::__construct() > > (I intentionally did not include the FFI related methods, since this > section is TBD) > > As it can be seen, most deprecations have an existing alternative. The 2nd > category (polyfillable ones) > mostly contains very little used functions, especially because the > deprecation of ldap_connect() only > affects PHP compiled with OracleLDAP. Thanks for the breakdown. I am also generally in favor of the changes here, but am concerned about the migration path. The key factor, IMO, is that it should be possible to run code without version checks across as wide a range of versions as feasible. Needing version checks to run from 8.2 to 9.0 is going to be problematic. Were we earlier in the 8.x cycle it would be less of an issue, but assuming we stick to the usual schedule it feels too tight on those. As Rowan asked, how much would it hurt to leave the date-related methods available but deprecated until 10.x? I'm also not entirely clear how you'd polyfill functions that exist, but change behavior. --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 2 May 2023 at 13:20, Máté Kocsis wrote: > Yes, but changing session_set_save_handler() to session_set_save_handlers() > should be a reasonably small effort, isn't it? I understand that it's > going to affect > lots of codebases, however other PHP 8.x deprecations are much more > difficult > to fix than this one would be. > Oh, did that proposal change, or did I just misread it? Maybe because the function names are so similar? In general, I hate variable and function names that differ only by an "s", it's far too easy to misread or mistype them. Maybe "session_set_save_handler_functions"? For the same reason, I much prefer Kamil's suggestion of " stream_context_set_option_array" over " stream_context_set_options". As an aside, I don't think "other deprecations are already more difficult" is a good argument - it's like saying "yes, I punched him, but not as hard as someone else already had". I think the change can be defended from other angles, but wanted to call that out. > Yes, I agree that the assert_options() name is at least weird but I > wouldn't like to > include changes into this RFC which are not strictly related to overloaded > signatures. Just like in case of implode(), the 1-parameter version of > assert_options() > could be added to the PHP 8.3/8.4 deprecations RFC though. > It *is* strictly related, though: the current function has two purposes: get an option, and set an option; the RFC proposes to split that into two functions, and there are three ways we can do that: 1) Keep the current name for get, come up with a new name for set 2) Come up with a new name for get, keep the current name for set 3) Come up with new names for both get and set I then looked further, and suggested: 4) Deprecate the existing function, but do not create any new functions; instead, recommend ini_get for get, and ini_set for set All four options are direct remedies to the overloaded signature, and I think due to the current unclear naming, options 3 and 4 are superior to options 1 and 2. Do you have a specific reason to prefer option 1? Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Rowan, As Kamil says, a potential 1-year deprecation is probably not enough, > and we need to keep in mind that many libraries and applications need to > support multiple versions of PHP at once. If they are to become errors > in 9.0, there should be some way to write code that will run on both 8.0 > and 9.0. > Yes, I agree that we should make the upgrade path of libraries as easy as possible, and I also agree that probably a 1-year deprecation is not enough.. although the suggested alternatives would be available 2 years in advance of the removal so this would allow a 2 year window for fixing code using the deprecated functionality. > The only ones in this list I can see that being easy for are > assert_options (you can write a polyfill for the new assert_set_option, > and switch all code to use that), and get_class() / get_parent_class() > (you can start passing $this right away). > As far as I see there are more deprecations which are easy to fix. In order to see the whole picture better, let's group the functions based on how a possible upgrade path would look like: - With existing suggested alternative: - get_class() and get_parent_class() - pg_fetch_result(), pg_field_prtlen(), and pg_field_is_null() - Phar::setStub() - ReflectionMethod::__construct() - ReflectionProperty::setValue() - session_set_save_handler() (because the OO and procedural style are interchangeable) - stream_context_set_option() - Polyfillable: - assert_options() - ldap_connect() - ldap_exop() - Requires conditional version check: - DatePeriod::__construct() - IntlCalendar::set() - IntlGregorianCalendar::__construct() (I intentionally did not include the FFI related methods, since this section is TBD) As it can be seen, most deprecations have an existing alternative. The 2nd category (polyfillable ones) mostly contains very little used functions, especially because the deprecation of ldap_connect() only affects PHP compiled with OracleLDAP. The category which requires conditional PHP version checks is indeed the most problematic one. Probably it's not a surprise that it only includes methods because it's not possible to polyfill methods according to my knowledge. The good news is that IntlCalendar::set() and IntlGregorianCalendar::__construct() seem to be very little used functionality. So in my opinion, DatePeriod::__construct() has the worst upgrade path, the rest are either easy or shouldn't cause much issues. Since the constructor is set to be removed completely based on Derick's preference, we should discuss with him whether it's worth to leave the public function __construct(DateTimeInterface $start, DateInterval $interval, DateTimeInterface $end, int $options = 0) {} signature alive so that people can change their code to use this instead of the other two signatures. I have particular concerns about session_set_save_handler. The impact is > hard to estimate, because it will be used directly in applications more > often than in libraries. The multiple parameter signature is much older, > dating back to PHP 4, with the object form added only in PHP 5.4. That > may seem a long time ago, but there is currently no incentive to change > existing code between the styles, and doing so requires a reasonable > amount of work. > Yes, but changing session_set_save_handler() to session_set_save_handlers() should be a reasonably small effort, isn't it? I understand that it's going to affect lots of codebases, however other PHP 8.x deprecations are much more difficult to fix than this one would be. > On a different point, I think "assert_options" is a peculiar name for > either setting or getting a single option, and would suggest it be > replaced with two new functions, assert_get_option and > assert_set_option. Replacing both variants also makes it easier for > users to find everywhere they've used it, and polyfill both variants, > rather than having to examine each. > Yes, I agree that the assert_options() name is at least weird but I wouldn't like to include changes into this RFC which are not strictly related to overloaded signatures. Just like in case of implode(), the 1-parameter version of assert_options() could be added to the PHP 8.3/8.4 deprecations RFC though. Regards, Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi On 5/2/23 13:56, Dan Ackroyd wrote: And unless I'm missing something, the second option doesn't appear usable with static methods, which is also a problem with get_parent_class() static::class or self::class appear to do the right thing depending on what exactly you want to retrieve. while code which invokes get_parent_class() without parameters should be modified to use get_parent_class($this) instead. What would the equivalent code get_parent_class() for static methods? e.g. parent::class appears to do the right thing. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 27.04.2023 23:28, Máté Kocsis wrote: https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures stream_context_set_option() should be removed from the Future Scope as it already is being changed by the RFC. -- Aleksander Machniak Kolab Groupware Developer[https://kolab.org] Roundcube Webmail Developer [https://roundcube.net] PGP: 19359DC1 # Blog: https://kolabian.wordpress.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
>From the RFC: > > Code which invokes get_class() without parameters should be modified to > use __CLASS__ or get_class($this) instead, I don't think the first option is equivalent: class C { function printType() { echo "__CLASS__ is " . __CLASS__ . "\n"; echo "get_class is " . get_class($this) . "\n"; } } class D extends C { function printTypeInChild() { echo "__CLASS__ is " . __CLASS__ . "\n"; echo "get_class is " . get_class($this) . "\n"; } } (new D)->printType(); (new D)->printTypeInChild(); // Output is: __CLASS__ is C get_class is D __CLASS__ is D get_class is D And unless I'm missing something, the second option doesn't appear usable with static methods, which is also a problem with get_parent_class() > while code which invokes > get_parent_class() without parameters should be modified to use > get_parent_class($this) instead. What would the equivalent code get_parent_class() for static methods? e.g. class A {} class B extends A { public static function foo() { echo get_parent_class() . " \n"; } } b::foo(); cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Kamil Tekiela wrote: > > I think one year of deprecation is not enough... It doesn't make > > sense to me to add a replacement but not deprecate the old variant. Máté Kocsis wrote: > Yes, this upgrade path also makes sense to me, and I'm happy to go with > it if others don't disagree! For the functions that are having new separate methods added, not deprecating them immediately makes upgrading easier. When upgrading from one version of PHP to the next, it is best if you can run the same code on both versions, without any deprecation notices going off. Would leaving the current versions in place and working actually cause any maintenance issues? If not, then we could just move really slowly on deprecating them. We could deprecate them at some point in the 9.x release cycle and remove them in 10.0. The RFC says: > Impact analysis: 1 out of the 2000 most popular PHP packages rely on calling > session_set_save_handler() with 6 or more arguments. I doubt analysing github is going to give a useful measure of the impact of this RFC. Functions like session_set_save_handler are going to be used in custom code written for a company than in shared libraries. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Fri, 28 Apr 2023 at 09:16, Rowan Tommins wrote: > On a different point, I think "assert_options" is a peculiar name for > either setting or getting a single option, and would suggest it be > replaced with two new functions, assert_get_option and > assert_set_option. > Further to this, I've realised that the manual is currently a bit confusing regarding the interaction of ini_set('zend.assertions', ...), ini_set('assert.active', ...), and assert_options(ASSERT_ACTIVE, ...) Either way, all the options to assert_options are documented as having an equivalent ini setting, so is this function actually needed at all, or should users be encouraged to use ini_get / ini_set directly? https://www.php.net/assert_options Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Kamil, Thank you for your reply, it is pretty much appreciated! I think one year of deprecation is not enough. I believe the functions > that get replacements should be deprecated immediately in PHP 8.3 to > give people two years of deprecation notice. It doesn't make sense to > me to add a replacement but not deprecate the old variant. > Yes, this upgrade path also makes sense to me, and I'm happy to go with it if others don't disagree! Probably my approach would be advantageous if we were earlier in the 8.x lifecycle. > `($rm = new ReflectionMethod(explode(“::”, $string));) ` I think you > forgot a splat operator here. > Fixed! > ReflectionProperty::setValue could have an alternative called > ReflectionProperty::setStaticValue. Then there would be no need for > the unused parameter anymore. > Originally, my proposal had this method, but at last I removed it based on Nicolas' suggestions. I think both ways make sense, a single setValue() is probably much easier from the migration point of view, since the suggested alternative is already there in older PHP versions. And it doesn't mean we cannot have a ReflectionProperty::setStaticValue() later on. :) > The overload of array_keys should be replaced with a new function > array_search_all. This overload is little known and very confusing. > Thanks for pointing this out, I'll look into this soon! > > The overload of ldap_exop should just be deprecated. An alternative > already exists and is the preferred way. > I've just included ldap_exop() in the RFC, albeit slightly differently to your suggestion: since this function performs the extended operation synchronously or asynchronously based on whether the $response_data parameter is provided, we cannot simply deprecate and remove the latter one, because as far as I can understand, synchronous communication is also a valid use-case. So I went with adding an ldap_exop_sync() function. > I wasn't aware that pg_execute has an overload apart from the default > connection one. Could you explain what that overload is? > You are right, as it turned out for me, there is no other overload indeed, so I removed it from the RFC. > stream_context_set_option should get alternative > stream_context_set_option_array. > Thanks for the suggestion! I incorporated this suggestion into the RFC (using the stream_context_set_options name though). What is the last item in Future scope supposed to be, because I think > ReflectionProperty is a typo. > Right, it's now removed. > Why is implode() not mentioned in this RFC? > Because it's not overloaded anymore. :) Even though the manual lists multiple signatures for it, its exact signature is the following: function implode(string|array $separator, ?array $array = null): string {} I'm not sure how much people would miss the possibility to use implode with a single argument ("implode($array)"), but if that's the case, we can add it to the usual PHP 8.3 or PHP 8.4 Deprecations RFC. I wouldn't like to add it to mine, because it's already way too long. Regards: Máté
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 27/04/2023 22:28, Máté Kocsis wrote: As you have possibly already experienced, overloaded signatures cause various smaller and bigger issues, while the concept is not natively supported by PHP. That's why I drafted an RFC which intends to phase out the majority of overloaded function/method signatures and also forbid the introduction of such functions in the future: https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures Hi Máté, While I broadly agree with the sentiment behind this, I think the upgrade path needs to be examined a bit more closely. As Kamil says, a potential 1-year deprecation is probably not enough, and we need to keep in mind that many libraries and applications need to support multiple versions of PHP at once. If they are to become errors in 9.0, there should be some way to write code that will run on both 8.0 and 9.0. The only ones in this list I can see that being easy for are assert_options (you can write a polyfill for the new assert_set_option, and switch all code to use that), and get_class() / get_parent_class() (you can start passing $this right away). A lot of the others introduce new methods, which can't easily polyfilled, or directly change the signature. For all of those, users would have to write an additional wrapper, with explicit detection of the PHP version to make the call in one form or the other. I have particular concerns about session_set_save_handler. The impact is hard to estimate, because it will be used directly in applications more often than in libraries. The multiple parameter signature is much older, dating back to PHP 4, with the object form added only in PHP 5.4. That may seem a long time ago, but there is currently no incentive to change existing code between the styles, and doing so requires a reasonable amount of work. On a different point, I think "assert_options" is a peculiar name for either setting or getting a single option, and would suggest it be replaced with two new functions, assert_get_option and assert_set_option. Replacing both variants also makes it easier for users to find everywhere they've used it, and polyfill both variants, rather than having to examine each. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Máté, I agree with this proposal, and I will be voting yes. The function overloads are not a big issue for PHP developers, and some are very much in use, but the reasons you listed are convincing and the manner in which you propose to do it should create an easy upgrade path. I think one year of deprecation is not enough. I believe the functions that get replacements should be deprecated immediately in PHP 8.3 to give people two years of deprecation notice. It doesn't make sense to me to add a replacement but not deprecate the old variant. `($rm = new ReflectionMethod(explode(“::”, $string));) ` I think you forgot a splat operator here. ReflectionProperty::setValue could have an alternative called ReflectionProperty::setStaticValue. Then there would be no need for the unused parameter anymore. The overload of array_keys should be replaced with a new function array_search_all. This overload is little known and very confusing. The overload of ldap_exop should just be deprecated. An alternative already exists and is the preferred way. I wasn't aware that pg_execute has an overload apart from the default connection one. Could you explain what that overload is? stream_context_set_option should get alternative stream_context_set_option_array. What is the last item in Future scope supposed to be, because I think ReflectionProperty is a typo. Why is implode() not mentioned in this RFC? Regards, Kamil -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php