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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Rowan and Larry, > Isn't that exactly what a deprecation period is for? > Yes, sure, but I wrote my arguments with the "Short deprecation period" in mind so that the removal of these functions causes the least pain. > If we want to give people longer, just leave the functionality deprecated > for longer before removing it. If we want to phase that in gradually, start > with a documentation-only deprecation, and add the deprecation notice > later. > If the plan is to keep the current function name, we can't get any of the > (very small) benefits of removing the extra signature until the final > removal anyway. > I don't completely understand this sentence, but my current plan is to go with the original function name as this causes the least impact and at the same time makes the scope of the RFC smaller. I'm tired of the debates (even if they were useful), and I don't want to add any tangentially related things in the RFC anymore. Anyway, adding an alias (session_set_handler()) and/or deprecating the original function name later is trivial, if someone wants to pursue this. > I'd propose a doc-only deprecation now (with session_set_handler() added > for the object version), an E_DEPRECATED in 9.0, and full removal in 10.0. Assuming the expected > release schedule, that gives people ~2 years before they see even an E_DEPRECATED, and 6-7 > years before they are forced to change. That should be ample time for anyone that still needs > to make the switch. I want to keep the original voting choices (the short and long path). Then votes can decide how much notice period they want to give. Since I don't insist on changing the function name anymore, the impact of this change is a lower, as only people using the callback signature would be affected. The fortunate thing is that session handling is used less in library code so at least open source maintainers rarely have to deal with the deprecation. And proprietary code maintainers can mostly ignore or suppress it for a while. Regards, Máté
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Wed, Jun 21, 2023, at 11:57 AM, Rowan Tommins wrote: > On Wed, 21 Jun 2023 at 09:43, Máté Kocsis wrote: > >> The reason why I think it's a good approach to have an intermediate state >> is to give >> these people the possibility to defer the actual migration until the >> very end. >> > > > Isn't that exactly what a deprecation period is for? > > If we want to give people longer, just leave the functionality deprecated > for longer before removing it. If we want to phase that in gradually, start > with a documentation-only deprecation, and add the deprecation notice later. > > If the plan is to keep the current function name, we can't get any of the > (very small) benefits of removing the extra signature until the final > removal anyway. > > Regards, > -- > Rowan Tommins > [IMSoP] I'm inclined to agree here, I think. I am all for a very-long grace period to give people lots of time for this change, but if the end state is just the object version being supported, then adding functions in the mean time doesn't make sense to me. I'd propose a doc-only deprecation now (with session_set_handler() added for the object version), an E_DEPRECATED in 9.0, and full removal in 10.0. Assuming the expected release schedule, that gives people ~2 years before they see even an E_DEPRECATED, and 6-7 years before they are forced to change. That should be ample time for anyone that still needs to make the switch. (A generic "functions to object wrapper" class is probably a not-too-hard composer package for someone to write, either, but that's not a task for internals.) --Larry Garfield -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Wed, 21 Jun 2023 at 09:43, Máté Kocsis wrote: > The reason why I think it's a good approach to have an intermediate state > is to give > these people the possibility to defer the actual migration until the > very end. > Isn't that exactly what a deprecation period is for? If we want to give people longer, just leave the functionality deprecated for longer before removing it. If we want to phase that in gradually, start with a documentation-only deprecation, and add the deprecation notice later. If the plan is to keep the current function name, we can't get any of the (very small) benefits of removing the extra signature until the final removal anyway. Regards, -- Rowan Tommins [IMSoP]
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Hi Bruce, For those who use callbacks now, how is this in any way better? They will > eventually end up using an OOP approach anyway (as that's the strategic > goal). > Migrating from `session_set_handler_callbacks()` would still be > (potentially) > non-trivial. And what's the point migrating *to* > `session_set_handler_callbacks()` > if we already know it will be deprecated and removed soon enough? The reason why I think it's a good approach to have an intermediate state is to give these people the possibility to defer the actual migration until the very end. So they can choose what to do when the 6+ parameter version of session_set_save_handler() becomes deprecated: they can either go straight to session_set_handler() *if they are ready* or they can also choose gaining time with a very straightforward change and use session_set_handler_callbacks() for at least a couple of years more. The important thing is that people are not yet forced to make a bigger effort, unless they are already ready for that. Regards, Máté
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Wed, Jun 21, 2023 at 12:54 AM Máté Kocsis wrote: > * In order to finally achieve the strategic goal, we deprecate the old > function along with session_set_handler_callbacks() > as soon as the right time comes. That may be PHP 9.x or 10.y or 11.z, > whatever. For those who use callbacks now, how is this in any way better? They will eventually end up using an OOP approach anyway (as that's the strategic goal). Migrating from `session_set_handler_callbacks()` would still be (potentially) non-trivial. And what's the point migrating *to* `session_set_handler_callbacks()` if we already know it will be deprecated and removed soon enough? -- Best regards, Bruce Weirdan mailto:weir...@gmail.com -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
HI Nicolas and Rowan, Mate, WDYT of 2)? Your sentence about better typing rang a bell with me: I think that's the best argument for removing the signature accepting callables. But before coming to a conclusion, let me answer Rowan's question first: This is where I don't understand what you or Maté consider to be the > alternative. If we create any function with a new name, then we have two > names for the user to choose between, regardless of whether that's one old > name and one new, or two new names. That's why I'm saying that if we have > two names at all, they should both be chosen with new users in mind, not > fudged based on a combination of current and new naming styles. > > Once the deprecation period is over, we should be left with a pair of names > that obviously signals the difference; or, we should be left with only one > function. > Of course, the best situation would be to have only the new function name with a single signature. Personally, I also prefer the OO style, but this doesn't really matter now. However, the sad truth is that we have a very widespread function with a slightly incorrect name supporting 2 signatures for more than a decade... On top of that, while most of the other deprecations in the RFC either affect rare functionality, or are suppressable via a simple polyfill or some automation/search+replace, only supporting the OO style signature of session_set_save_handler() would mean that people would have to wrap the callbacks in a class. This is not automatable, and requires some possibly non-trivial manual work. This means that we have to change anything really carefully here. With all these in mind, I'd say that the strategic goal should be to have a single signature + the new function name only. Reaching this state would affect every single proprietary codebase in some way or another, so I don't think it's feasible or fair with users to try it in one single step. So in order to reduce the damage, my plan is the following: * Let's keep one of the two signatures which are compatible with older PHP versions so that at least a fraction of all users (maybe 50-70%?) are not impacted at all. That's why session_set_save_handler() should not be removed just yet. Since the OO version seems more future-proof based on Nicolas' great argument (it accepts an interface, while callbacks don't have a formal signature), we should keep this form. * People who use the signature to be deprecated should not be forced to do non-trivial changes. So introducing a new function for them will keep their code run with a minimal effort (search + replace), while we managed to eliminate the overloaded signature. At the same time, we try to educate them (mostly in the manual) that the best way forward is to migrate their code to use session_set_handler(). There's no deprecation just yet, and everyone can do it voluntarily at their own pace. * In order to finally achieve the strategic goal, we deprecate the old function along with session_set_handler_callbacks() as soon as the right time comes. That may be PHP 9.x or 10.y or 11.z, whatever. This process may sound complicated, but in my opinion, this is the least disruptive upgrade path for achieving the goal I outlined above. Sometimes going full ninja mode is possible, but I'd prefer being careful in this very case, especially because I guess some people do not consider the motivation of the RFC particularly important, so I would like if their cost-benefit analysis would stay positive (I don't only mean voters but end users as well, since their perception also matters when they are fixing the incompatibilities of their code). Máté
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 20 Jun 2023 at 09:22, Nicolas Grekas wrote: > > So are you saying we should deprecate the function-based version > > completely, and not provide any new names at all? I think I would prefer > > that to the confusing set of aliases the current RFC text proposes. > > > > I was not but I would be fine with this solution also :) > Then I don't understand the reasoning of picking a "best" - is the "best" one the one that keeps the old name, which is unclear but requires no changes; or is the "best" one the one that gets a clearer name, and immediately has a non-overloaded signature that everyone can use? > > 1) Do nothing. I don't see the current situation as being that big a > > problem. > > > > The problem is the overloaded signature, which Mate explained in the RFC. > Sure; I just don't see any of the things mentioned as big enough problems to justify significant disruption to users. > THIS would create confusion to me, because ppl would have > two names to choose from, and many won't take the time to figure out the > difference. > This is where I don't understand what you or Maté consider to be the alternative. If we create any function with a new name, then we have two names for the user to choose between, regardless of whether that's one old name and one new, or two new names. That's why I'm saying that if we have two names at all, they should both be chosen with new users in mind, not fudged based on a combination of current and new naming styles. Once the deprecation period is over, we should be left with a pair of names that obviously signals the difference; or, we should be left with only one function. Regards, -- Rowan Tommins [IMSoP]
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> Then, among both options, we need to select the one with the best future > > proofness, and that's definitely the OOP one to me, because it comes with > > more guarantees (type checks). > > > So are you saying we should deprecate the function-based version > completely, and not provide any new names at all? I think I would prefer > that to the confusing set of aliases the current RFC text proposes. > I was not but I would be fine with this solution also :) > I think I would support any of these options: > > 1) Do nothing. I don't see the current situation as being that big a > problem. > The problem is the overloaded signature, which Mate explained in the RFC. > 2) Deprecate the function-based signature, provide no alternative but to > switch to the interface-based one. Potentially non-trivial upgrade for > those affected, but leaves us with a single clear function. > Would work for me! The OOP way can be implemented since more than 10 years. > 3) Create new names for both signatures, treating neither of them as > more "default" than the other, so that users have a clear choice between > the two. Deprecate the existing function completely. > Deprecating in the same version that provides the alternative would create a lot of friction (even if in this case, it's possible to polyfill, which makes this a bit less rough). So to me, this path requires providing an alias without deprecating the OOP version first, and later on deprecating the current name. THIS would create confusion to me, because ppl would have two names to choose from, and many won't take the time to figure out the difference. Mate, WDYT of 2)? Nicolas
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 20/06/2023 07:26, Nicolas Grekas wrote: Then, among both options, we need to select the one with the best future proofness, and that's definitely the OOP one to me, because it comes with more guarantees (type checks). So are you saying we should deprecate the function-based version completely, and not provide any new names at all? I think I would prefer that to the confusing set of aliases the current RFC text proposes. I think I would support any of these options: 1) Do nothing. I don't see the current situation as being that big a problem. 2) Deprecate the function-based signature, provide no alternative but to switch to the interface-based one. Potentially non-trivial upgrade for those affected, but leaves us with a single clear function. 3) Create new names for both signatures, treating neither of them as more "default" than the other, so that users have a clear choice between the two. Deprecate the existing function completely. All the other suggestions seem likely to create a lot of confusion, and not actually leave us much better off in the long term. 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 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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Le lun. 19 juin 2023 à 22:33, Rowan Tommins a écrit : > On 19/06/2023 21:17, Nicolas Grekas wrote: > > I think we must account for a bit of history/legacy on the topic. > > I think session_set_save_handler(SessionHandlerInterface) is the best > BC/FC > > path we can provide. > > > Can you elaborate? The SessionHandlerInterface is the *newer* of the two > current signatures, so what does making it the preferred signature (with > users of the other having to change their code) have to do with > "history/legacy"? Sure : SessionHandlerInterface is around 5.4. It's been there since long enough to not care about this aspect when considering both signatures. In my experience, supporting 3 major versions is way enough: previous major, current major and next major. Which means 7+8+9 in this case. It's enough because an app that runs on < 5.4 doesn't need to prepare to move to 9. It first has many more steps to do. Then, among both options, we need to select the one with the best future proofness, and that's definitely the OOP one to me, because it comes with more guarantees (type checks). By doing so, we allow apps that are still on 7 but are also actively planning to upgrade to 8+ to make the future-proof choice of choosing SessionHandlerInterface. Nicolas
Re: [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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 19/06/2023 21:17, Nicolas Grekas wrote: I think we must account for a bit of history/legacy on the topic. I think session_set_save_handler(SessionHandlerInterface) is the best BC/FC path we can provide. Can you elaborate? The SessionHandlerInterface is the *newer* of the two current signatures, so what does making it the preferred signature (with users of the other having to change their code) have to do with "history/legacy"? Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
Le mer. 14 juin 2023 à 23:51, Máté Kocsis a écrit : > > > > Given that you've agreed that neither signature is "primary" or "better", > > doesn't that argument apply equally to both signatures? If it's OK to > force > > anyone using individual callbacks to change their code, why is it not > > equally OK to force anyone using an object to change their code? > > > > As far as I remember, my reasoning was two-fold: > - naming: passing *multiple* callbacks to a function name in singular > (session_set_save_handler()) > is definitely not ideal > - a decision had to be made and I thought that keeping the OO approach in > place had less BC impact, > especially because previously I got feedback from Nicolas that they used > this version. > > > > I do wonder if this change is too disruptive relative to the benefit it > > brings, but in that case, we should just leave the whole function as it > is. > > > > In my opinion, phasing out session_set_save_handler() completely would be > worth the effort if this > was the only option to fix the overridden signature. However, it's not the > case, since strictly speaking, > only one of the two signatures has to be removed in order to achieve the > desired goal of the RFC. > The whole discussion about also deprecating the other one started only > because of improving naming: > it is also a nice thing to pursue but fails the cost-benefit analysis. All > in all, I think neither not doing > anything, nor deprecating the whole function is a good choice. But at least > I definitely want the question > to be settled with putting this to vote. > > I don't think "handler" particularly implies "object". The "handler" passed > > to set_error_handler is a function, and your original suggestion for a > new > > function was "session_set_save_handlers", where "handler" would also mean > > "function". > > > > Without any suffix at all, it seems like "set a session handler the normal > > way" vs "set a session a special different way"; like how > > "array_merge_recursive" is a special version of "array_merge". > > > > I think my reasoning is easier to understand if we go back to my original > suggestion: > session_set_save_handler() and session_set_save_handlers(), which would be > session_set_handler() and session_set_handlers() now. That was the starting > point, > but you didn't like that they only differ by an "s". That's why I swapped > the "s" with "callbacks". > > According to your deduction, session_set_handler() is not the most correct > name indeed, but I don't think > that we always have to choose the most correct and the entirely descriptive > one. After all, neither the > parameters of session_set_handler() are called "$open_callback", > "$close_callback" etc., they are just > "$open", "$close" etc. and I guess they are still clear enough, given that > their type is declared which > completes the missing context. > > Similarly, we all would know that the session_set_handler() function > expects an object of > SessionHandlerInterface type, while its sibling, > session_set_handler_callbacks() expects some > callbacks. Yes, having the _object suffix is the most pedantic name, but > personally, I don't really like it > because I find it ugly (and I was pretty much content with > session_set_handlers()). I'm curious about > the point of view of other people though, because if it turns out that > people generally also favor some > other name then I'm ok with a compromise. > I think we must account for a bit of history/legacy on the topic. I think session_set_save_handler(SessionHandlerInterface) is the best BC/FC path we can provide. And I don't think we should alias this function to a new name because that would require users to make a choice, and will lead to confusion, because in this case, the choice is purely cosmetic, but people will still have to get it. Nicolas
Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > > 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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 14/06/2023 22:51, Máté Kocsis wrote: The whole discussion about also deprecating the other one started only because of improving naming: it is also a nice thing to pursue but fails the cost-benefit analysis. In my mind, making two new names, both unambiguous, gives a greater benefit (a clearer final state) without that much extra cost (more people will have to change their code, but fewer will be confused about whether they need to or not). All in all, I think neither not doing anything, nor deprecating the whole function is a good choice. The more I think about it, the more I'm leaning towards not doing anything being the best choice. The benefit seems small, and the cost high. Regards, -- Rowan Tommins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > Given that you've agreed that neither signature is "primary" or "better", > doesn't that argument apply equally to both signatures? If it's OK to force > anyone using individual callbacks to change their code, why is it not > equally OK to force anyone using an object to change their code? > As far as I remember, my reasoning was two-fold: - naming: passing *multiple* callbacks to a function name in singular (session_set_save_handler()) is definitely not ideal - a decision had to be made and I thought that keeping the OO approach in place had less BC impact, especially because previously I got feedback from Nicolas that they used this version. > I do wonder if this change is too disruptive relative to the benefit it > brings, but in that case, we should just leave the whole function as it is. > In my opinion, phasing out session_set_save_handler() completely would be worth the effort if this was the only option to fix the overridden signature. However, it's not the case, since strictly speaking, only one of the two signatures has to be removed in order to achieve the desired goal of the RFC. The whole discussion about also deprecating the other one started only because of improving naming: it is also a nice thing to pursue but fails the cost-benefit analysis. All in all, I think neither not doing anything, nor deprecating the whole function is a good choice. But at least I definitely want the question to be settled with putting this to vote. I don't think "handler" particularly implies "object". The "handler" passed > to set_error_handler is a function, and your original suggestion for a new > function was "session_set_save_handlers", where "handler" would also mean > "function". > Without any suffix at all, it seems like "set a session handler the normal > way" vs "set a session a special different way"; like how > "array_merge_recursive" is a special version of "array_merge". > I think my reasoning is easier to understand if we go back to my original suggestion: session_set_save_handler() and session_set_save_handlers(), which would be session_set_handler() and session_set_handlers() now. That was the starting point, but you didn't like that they only differ by an "s". That's why I swapped the "s" with "callbacks". According to your deduction, session_set_handler() is not the most correct name indeed, but I don't think that we always have to choose the most correct and the entirely descriptive one. After all, neither the parameters of session_set_handler() are called "$open_callback", "$close_callback" etc., they are just "$open", "$close" etc. and I guess they are still clear enough, given that their type is declared which completes the missing context. Similarly, we all would know that the session_set_handler() function expects an object of SessionHandlerInterface type, while its sibling, session_set_handler_callbacks() expects some callbacks. Yes, having the _object suffix is the most pedantic name, but personally, I don't really like it because I find it ugly (and I was pretty much content with session_set_handlers()). I'm curious about the point of view of other people though, because if it turns out that people generally also favor some other name then I'm ok with a compromise.
Re: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On Tue, 13 Jun 2023 at 15:49, Máté Kocsis wrote: > I understand that communication is much simpler with your suggested > approach, however, deprecating something which everyone surely uses partly > undermines the efforts we have made for ensuring as much backward > compatibility as reasonably possible. > Given that you've agreed that neither signature is "primary" or "better", doesn't that argument apply equally to both signatures? If it's OK to force anyone using individual callbacks to change their code, why is it not equally OK to force anyone using an object to change their code? I do wonder if this change is too disruptive relative to the benefit it brings, but in that case, we should just leave the whole function as it is. > In my opinion, the "_object" suffix is superfluous. There would be two > session_set_handler*() functions: the first one accepts a session handler > (instance) (session_set_handler()) and the second one accepts session > handler callbacks (session_set_handler_callbacks()). They are not primary > or secondary, I just don't like the "_object" suffix, and I couldn't come > up with a better alternative. > I don't think "handler" particularly implies "object". The "handler" passed to set_error_handler is a function, and your original suggestion for a new function was "session_set_save_handlers", where "handler" would also mean "function". My interpretation of the names is that both versions of the function "set a session handler", but they differ in what type of handler - one "sets a session handler using function callbacks", the other "sets a session handler using an object instance". Thus, the first is "session_set_handler_functions" or "session_set_handler_callbacks" or so, and the second is "session_set_handler_object" or "session_set_handler_instance" or so. Without any suffix at all, it seems like "set a session handler the normal way" vs "set a session a special different way"; like how "array_merge_recursive" is a special version of "array_merge". Regards, -- Rowan Tommins [IMSoP]
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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
> > In my mind, the only reason to change anything about this function is that > we can't properly overload a function based on its argument types. There's > nothing particularly "primary" or "better" about either of the two > signatures, it's just hard to document and use named parameters while > they're both part of one function. > Yes, that's the case exactly. > My suggestion was very explicitly that everyone using sessions should have > to change their code, to a name that explicitly mentions the parameter type > in its name (because that's the difference between the two). As I said, I > think it's much simpler to communicate "this function is deprecated, choose > the appropriate from these two names", rather than "this function is > deprecated in one format, but not the other, and if you are using the > deprecated format, you can use this instead". > I understand that communication is much simpler with your suggested approach, however, deprecating something which everyone surely uses partly undermines the efforts we have made for ensuring as much backward compatibility as reasonably possible. That's why I'm not in favor of officially deprecating the object oriented version of session_set_save_handler() just yet. I think soft deprecating it for now results in a smoother upgrade path by letting people optionally use its alias first (session_set_handler()), and then we can start nagging them later on. Otherwise, I would question why we had to have such a long discussion focusing mainly on the BC aspects of the proposal. I also understand that polyfilling session_set_save_handler() would also be easy to do, but the migration is even easier if we leave the 2-parameter version alone for a while. Both the "_object" suffix (or some equivalent) and deprecating the original > regardless of signature are inherent in my suggestion. You seem to have > interpreted it as something else, and I'm not entirely sure what you're > actually proposing - how does an alias fit in to something that's about > splitting a function into two? > In my opinion, the "_object" suffix is superfluous. There would be two session_set_handler*() functions: the first one accepts a session handler (instance) (session_set_handler()) and the second one accepts session handler callbacks (session_set_handler_callbacks()). They are not primary or secondary, I just don't like the "_object" suffix, and I couldn't come up with a better alternative. Regards, Máté
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: Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
On 10 June 2023 08:31:24 BST, "Máté Kocsis" wrote: >Hmm, that's also a very good idea, and I support this. However, I'm >hesitant to deprecate the 2 parameter version of session_set_save_handler() >just yet, >since doing so would mean that everyone using sessions has to use a new >function... Instead, I opted for aliasing it to the new >session_set_handler() function (I'm not fond >of the "_object" postfix, because I regard it unnecessary as it operates on >SessionHandlerInterface instances). And we could maybe deprecate >session_set_save_handler() >altogether in a few years. Hm, I think we're not quite on the same page here. In my mind, the only reason to change anything about this function is that we can't properly overload a function based on its argument types. There's nothing particularly "primary" or "better" about either of the two signatures, it's just hard to document and use named parameters while they're both part of one function. My suggestion was very explicitly that everyone using sessions should have to change their code, to a name that explicitly mentions the parameter type in its name (because that's the difference between the two). As I said, I think it's much simpler to communicate "this function is deprecated, choose the appropriate from these two names", rather than "this function is deprecated in one format, but not the other, and if you are using the deprecated format, you can use this instead". Both the "_object" suffix (or some equivalent) and deprecating the original regardless of signature are inherent in my suggestion. You seem to have interpreted it as something else, and I'm not entirely sure what you're actually proposing - how does an alias fit in to something that's about splitting a function into two? 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
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é
Fwd: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
I'm forwarding my mails which I managed to only address for Rowan: Hi Rowan, > 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. > Hmm, that's also a very good idea, and I support this. However, I'm hesitant to deprecate the 2 parameter version of session_set_save_handler() just yet, since doing so would mean that everyone using sessions has to use a new function... Instead, I opted for aliasing it to the new session_set_handler() function (I'm not fond of the "_object" postfix, because I regard it unnecessary as it operates on SessionHandlerInterface instances). And we could maybe deprecate session_set_save_handler() altogether in a few years. 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. > I'm not 100% sure an official static analyser would be needed. I think it's already a good situation to have very well usable and maintained userland implementations, which we do have. After all, we neither bundle Composer or PHPUnit. To expand on my previous statement in my other main: I don't say that everyone uses static analysers I know many people don't... My message would have rather been that bugs don't cost enough for these projects if they don't have to actively prevent the introduction of bugs via static analysis. I have realized in the meanwhile though that there may be many other reasons why someone doesn't use these tools, so I think this argument of mine is invalidated. 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. > However, as George highlighted (I also tried, but most probably I couldn't convey my message), declaring true return types is an intermediary step before changing them to void. So replacing the old IntlCalendar methods having "incorrect" return types with new ones having the "correct" return types is a very nice by-product of this RFC, since we (and users) don't have to deal with the return type migration separately at a later point of time. I still think that the benefit of immediately using void for the new methods outweighs the risks. Regards: 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
[PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
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é
