# Revert unapproved language change, was: Fix division by zero to throw exception (round 2) - Thread: https://externals.io/message/87026 - Original archive: https://news-web.php.net/php.internals/87026 - Started by Dan Ackroyd on 2015-07-05T10:29:38+00:00 - Messages: 6 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #87026 — Dan Ackroyd — 2015-07-05T10:29:38+00:00 URL: https://externals.io/message/87026#87026 > > I'm proposing that we reconsider removing the warning from floating point > division and here's why. Wait ....what? I don't remember an RFC about the behaviour changing. Did someone ninja commit a change to the language? Well it sure looks like it: https://github.com/php/php-src/commit/f9724b93f6592d2f77fa9165038a0ba0db3da0c6 This is absolutely a change that needs an RFC. As the change was done without one, please can it be reverted until an RFC is done? Bob Weinand wrote: > I’m just saying that warning in the worst we can have. You are wrong; it allows: * People who want it to be an exception to convert it to an exception in their error handler. * People who want to ignore it can suppress it with the squelch operator. * People who want to handle it on a case by case basis in their code can do. This is another case where you are ignoring other people's use-cases, and imagining that your use-case is the only valid use-case. But the technical discussion is one that we should have when someone is proposing this change - not when someone has committed it first and then is asking for approval. cheers Dan --- ## #87027 — Sherif Ramadan — 2015-07-05T11:01:14+00:00 URL: https://externals.io/message/87026#87027 · In reply to: #87026 > > > […] > > Wait ....what? I don't remember an RFC about the behaviour changing. > Did someone ninja commit a change to the language? > > Well it sure looks like it: > > https://github.com/php/php-src/commit/f9724b93f6592d2f77fa9165038a0ba0db3da0c6 > > This is absolutely a change that needs an RFC. As the change was done > without one, please can it be reverted until an RFC is done? > I actually fully agree to the IEEE 754 compliance part and I doubt anyone will disagree on that part as it only stands to benefit everyone. However, I completely disagree with removing the warning blind-sidedly and especially two days before the beta1 release like that. I do opt that it be reverted, however, until the matter is fully resolved. If that happens to take a day or a month it shouldn't result in releasing ad hoc changes that will be wish-washy between releases like that should something change. At the very least let's cherry pick it out of the beta 1 release to ensure we've fully resolved the matter. > > Bob Weinand wrote: > > […] > > You are wrong; it allows: > > * People who want it to be an exception to convert it to an exception > in their error handler. > * People who want to ignore it can suppress it with the squelch operator. > * People who want to handle it on a case by case basis in their code can > do. > > This is another case where you are ignoring other people's use-cases, > and imagining that your use-case is the only valid use-case. > > But the technical discussion is one that we should have when someone > is proposing this change - not when someone has committed it first and > then is asking for approval. > > > cheers > Dan --- ## #87028 — Ferenc Kovacs — 2015-07-05T17:02:00+00:00 URL: https://externals.io/message/87026#87028 · In reply to: #87027 On Sun, Jul 5, 2015 at 1:01 PM, Sherif Ramadan wrote: > On Sun, Jul 5, 2015 at 6:29 AM, Dan Ackroyd > wrote: > > > […] > point > > […] > https://github.com/php/php-src/commit/f9724b93f6592d2f77fa9165038a0ba0db3da0c6 > > […] > > I actually fully agree to the IEEE 754 compliance part and I doubt anyone > will disagree on that part as it only stands to benefit everyone. However, > I completely disagree with removing the warning blind-sidedly and > especially two days before the beta1 release like that. I do opt that it be > reverted, however, until the matter is fully resolved. If that happens to > take a day or a month it shouldn't result in releasing ad hoc changes that > will be wish-washy between releases like that should something change. At > the very least let's cherry pick it out of the beta 1 release to ensure > we've fully resolved the matter. > > hi, my 2 cents: the IEEE 754 related changes had enough discussion and while it would have been nice handling it under an rfc earlier instead of pushing it late calling it a bugfix I think we should keep it. however I see little point in removing the warning at this point, I can understand how from a purist point of view it makes little sense, but we have this warning forever and as people already pointed out at the very least can help with debugging bugs in your code. so I would keep the warning for 7.0 (as removing it after beta1 would be even worse than pushing the change 2 days before tagging beta1). -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #87030 — Anatol Belski — 2015-07-05T18:44:45+00:00 URL: https://externals.io/message/87026#87030 · In reply to: #87028 Hi, > -----Original Message----- > From: Ferenc Kovacs [mailto:tyra3l@gmail.com] > Sent: Sunday, July 5, 2015 7:02 PM > To: Sherif Ramadan > Cc: Dan Ackroyd; Jakub Kubíček; Dmitry Stogov; Bob Weinand; Andrea Faulds; > PHP Internals; Nikita Popov; Aaron Piotrowski; Levi Morrison > Subject: Re: [PHP-DEV] Re: Revert unapproved language change, was: Fix > division by zero to throw exception (round 2) > > On Sun, Jul 5, 2015 at 1:01 PM, Sherif Ramadan > wrote: > > > […] > wrote: > > […] > src/commit/f9724b93f6592d2f77fa9165038a0ba0 > > […] > hi, > > my 2 cents: > the IEEE 754 related changes had enough discussion and while it would have > been nice handling it under an rfc earlier instead of pushing it late calling it a > bugfix I think we should keep it. > however I see little point in removing the warning at this point, I can understand > how from a purist point of view it makes little sense, but we have this warning > forever and as people already pointed out at the very least can help with > debugging bugs in your code. > so I would keep the warning for 7.0 (as removing it after beta1 would be even > worse than pushing the change 2 days before tagging beta1). > The related changes are reverted as breaching BC. The issue reported by Dan is likely to prevent people from catching errors the ways they do it in PHP5. Furthermore, it's likely to break apps or make them misbehave. This is something we can't bear so short before beta1. Bob, I would kindly ask you again to not to commit such controversial changes without having a clear OK from the community. As for me - IMHO, the proper solution could be your first commit introducing exceptions for all (as in many other programming languages for div by zero) - but it was a BC breach as well, timed too short before feature freeze. So it is something that clearly needs an RFC to evaluate any possible risks and to make an informed collective decision. Many things are not perfect yet even in PHP7, but we have our timeline plan on which you was voting, too. Thanks Anatol --- ## #87034 — Levi Morrison — 2015-07-05T20:39:45+00:00 URL: https://externals.io/message/87026#87034 · In reply to: #87030 > Hi, > > > […] > The related changes are reverted as breaching BC. The issue reported by Dan is likely to prevent people from catching errors the ways they do it in PHP5. Furthermore, it's likely to break apps or make them misbehave. This is something we can't bear so short before beta1. > > Bob, I would kindly ask you again to not to commit such controversial changes without having a clear OK from the community. As for me - IMHO, the proper solution could be your first commit introducing exceptions for all (as in many other programming languages for div by zero) - but it was a BC breach as well, timed too short before feature freeze. So it is something that clearly needs an RFC to evaluate any possible risks and to make an informed collective decision. Many things are not perfect yet even in PHP7, but we have our timeline plan on which you was voting, too. > > Thanks > > Anatol I just want to chime in and say that I have a contributed to a popular application written in C that actually uses division by zero in a useful manner. I feel like the rest of you think this is solely a programmer error, and wanted to chime in to reiterate that this is not the case. I would be in favor of removal of the error/warning and *especially* would be against an exception for division by zero. Division by zero as defined by IEEE 754 is actually useful which is why it is often followed instead of sticking to a strict mathematical definition where it is an error. I understand that some people are just upset about the timing/discussion, which is possibly understandable - I have been too busy lately to follow anything so I don't know. I just wanted to jump in here and clarify the usefulness of what was actually changed. --- ## #87054 — Sherif Ramadan — 2015-07-06T16:43:44+00:00 URL: https://externals.io/message/87026#87054 · In reply to: #87034 > > > I just want to chime in and say that I have a contributed to a popular > application written in C that actually uses division by zero in a > useful manner. I feel like the rest of you think this is solely a > programmer error, and wanted to chime in to reiterate that this is not > the case. I would be in favor of removal of the error/warning and > *especially* would be against an exception for division by zero. > Division by zero as defined by IEEE 754 is actually useful which is > why it is often followed instead of sticking to a strict mathematical > definition where it is an error. > > I understand that some people are just upset about the > timing/discussion, which is possibly understandable - I have been too > busy lately to follow anything so I don't know. I just wanted to jump > in here and clarify the usefulness of what was actually changed. > IEEE 754 is useful, I agree. The warning is also useful, because it helps the programmer find their mistake more easily. It gives you a file name and line number to look for. We have to remember that PHP is being used by a wide array of people with varying skillsets and expertise. Not all of them know what IEEE 754 is or even how to prevent division by zero. When they don't debugging the code that results in a division by zero means having to learn those things first. We're lowering the bar by just handing them the useful debug information. So removing the warning makes no sense. Having a warning in your error log screams at you that you potentially have a bug here and PHP shines a light on it for you so that you can either fix it or refactor the code to not result in this potentially buggy behavior. I don't think the point of this discussion was to discuss which is more useful, the warning or the error/exception. My immediate concern for raising this issue, initially, was that we suddenly ended up in a place where we had neither a warning nor an error, thereby leaving the user with no useful debug information whatsoever. No indication that there is potentially buggy code. That's bad. That should not have happened.