Re: [VOTE] Reclassifying engine warnings

This is only part of a thread. view whole thread
  107322
September 26, 2019 07:41 nikita.ppv@gmail.com (Nikita Popov)
On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov ppv@gmail.com> wrote:

> Hi internals, > > I've opened the vote on //wiki.php.net/rfc/engine_warnings. > > There are 4 votes, all of them independent. The first 3 are for specific > cases that were controversial during the discussion, the last one is for > the remainder of the proposal. > > Voting closes 2019-09-26. >
Voting has closed with the final outcome being: * Undefined variables: 36 exception, 18 warning, 10 notice. Exception declined with 56% in favor. Warning accepted with 84% combined majority. * Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3 majority. * Division by zero: 52 exception, 8 warning. Exception accepted with 87% majority. * Remainder: 54 yes, 3 no. Accepted with 95% majority. Regards, Nikita
  107325
September 26, 2019 08:31 cschneid@cschneid.com (Christian Schneider)
Am 26.09.2019 um 09:41 schrieb Nikita Popov ppv@gmail.com>:
> * Remainder: 54 yes, 3 no. Accepted with 95% majority.
Just for the record: The one I'm most concerned about here is the foreach on undefined variables throwing an exception. While this was already promoted to a warning at an earlier stage it still allowed a program to finish with a well-defined result (looping over nothing does nothing). Now this code will stop with an Exception and possibly won't produce its output. I noticed this before the vote but as the mailing list was already flooded with the 'undefined variables' discussion I feared that there was no place to bring this up without getting personally attacked like I was for the undefined variables stuff. Anyway, the vote is done, I said my piece and will shut up about it now, - Chris
  107335
September 26, 2019 14:23 chasepeeler@gmail.com (Chase Peeler)
On Thu, Sep 26, 2019 at 4:31 AM Christian Schneider <cschneid@cschneid.com>
wrote:

> Am 26.09.2019 um 09:41 schrieb Nikita Popov ppv@gmail.com>: > > * Remainder: 54 yes, 3 no. Accepted with 95% majority. > > Just for the record: > The one I'm most concerned about here is the foreach on undefined > variables throwing an exception. > While this was already promoted to a warning at an earlier stage it still > allowed a program to finish with a well-defined result (looping over > nothing does nothing). > Now this code will stop with an Exception and possibly won't produce its > output. > > I agree. At the very least, I'd propose that null (and ideally false) NOT throw an exception. There are enough instances of methods that return one
of those values when there is no data (so, not an error situation) but an array when there is.
> I noticed this before the vote but as the mailing list was already flooded > with the 'undefined variables' discussion I feared that there was no place > to bring this up without getting personally attacked like I was for the > undefined variables stuff. > > Thanks for bringing this up. I think this should be noted in relation to the other RFC currently out there. Those of us speaking out against this
RFC are the ones being held up as examples of why that RFC is needed, which this statement actually shows that it was those in favor of the RFC that were behaving in a way that intimidated others and made them feel reluctant to contribute.
> Anyway, the vote is done, I said my piece and will shut up about it now, > - Chris > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
-- Chase Peeler chasepeeler@gmail.com
  107327
September 26, 2019 09:48 petercowburn@gmail.com (Peter Cowburn)
On Thu, 26 Sep 2019 at 08:42, Nikita Popov ppv@gmail.com> wrote:

> On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov ppv@gmail.com> wrote: > > > Hi internals, > > > > I've opened the vote on //wiki.php.net/rfc/engine_warnings. > > > > There are 4 votes, all of them independent. The first 3 are for specific > > cases that were controversial during the discussion, the last one is for > > the remainder of the proposal. > > > > Voting closes 2019-09-26. > > > > Voting has closed with the final outcome being: > > * Undefined variables: 36 exception, 18 warning, 10 notice. Exception > declined with 56% in favor. Warning accepted with 84% combined majority. >
I just want to go on the record in saying that I am very, very disappointed that a choice that only got 28% of the overall votes, and only 33% of votes in the "we want change" scenario, is being taken as the will of the overwhelming majority, which is the bar that is needed to be crossed for RFC votes. This is wholly irresponsible.
> * Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3 > majority. > * Division by zero: 52 exception, 8 warning. Exception accepted with 87% > majority. > * Remainder: 54 yes, 3 no. Accepted with 95% majority. > > Regards, > Nikita >
  107334
September 26, 2019 12:20 rowan.collins@gmail.com (Rowan Tommins)
On Thu, 26 Sep 2019 at 10:48, Peter Cowburn <petercowburn@gmail.com> wrote:

> I just want to go on the record in saying that I am very, very disappointed > that a choice that only got 28% of the overall votes, and only 33% of votes > in the "we want change" scenario, is being taken as the will of the > overwhelming majority, which is the bar that is needed to be crossed for > RFC votes. This is wholly irresponsible. >
Three-way votes are always tricky in this respect, but I think in this case Nikita has taken a very sensible approach. Firstly, the interpretation of the three-way vote was laid out very clearly on the page, and I'm not aware of anyone objecting to it prior to this point. Secondly, it makes sense intuitively: it seems unlikely that someone who would vote yes to the question "Should undefined variables give an Error instead of a Notice?" would vote no to the question "Should undefined variables give a Warning instead of a Notice?" Thirdly, the options are not mutually exclusive in the way that, say, a syntax decision would be. Raising the level to Warning now doesn't prevent a future proposal to raise it to Error (e.g. on a different timescale). Finally, and perhaps most importantly, RFC votes are intended to be measures of consensus. Taken alongside the discussion, the result strongly suggests that there is a consensus (but not a unanimous one) to change the error level, but there is some concern about raising it as high as Error. Regards, -- Rowan Tommins [IMSoP]
  107362
October 2, 2019 13:23 nikita.ppv@gmail.com (Nikita Popov)
On Thu, Sep 26, 2019 at 9:41 AM Nikita Popov ppv@gmail.com> wrote:

> On Thu, Sep 12, 2019 at 2:17 PM Nikita Popov ppv@gmail.com> wrote: > >> Hi internals, >> >> I've opened the vote on //wiki.php.net/rfc/engine_warnings. >> >> There are 4 votes, all of them independent. The first 3 are for specific >> cases that were controversial during the discussion, the last one is for >> the remainder of the proposal. >> >> Voting closes 2019-09-26. >> > > Voting has closed with the final outcome being: > > * Undefined variables: 36 exception, 18 warning, 10 notice. Exception > declined with 56% in favor. Warning accepted with 84% combined majority. > * Undefined array index: 42 warning, 21 notice. Warning accepted with 2/3 > majority. > * Division by zero: 52 exception, 8 warning. Exception accepted with 87% > majority. > * Remainder: 54 yes, 3 no. Accepted with 95% majority. > > Regards, > Nikita >
This RFC is now mostly implemented. The parts that are still missing are: * Switching to DivisionByZeroError. I would like to add an fdiv() function before doing this and have started a separate thread on the topic. * The "Invalid argument supplied for foreach()" case. Christian Schneider has requested that this stay as a warning, and personally I'm okay with doing that. Unlike most of the other exception promotions, this one won't result in any VM simplifications/optimizations and I don't have a very strong reason to make it an exception. Does anyone else feel strongly about this? * The "Undefined array index" case. This one passed the vote with an exact 2/3 majority, so I'm a bit uncomfortable making changes here. This is also the only case where convincing usability concerns have been brought forward, primarily around the $array[$key]++ example. I'm not yet decided on what to do here and whether we should pursue additional library or language features first. Nikita
  107363
October 2, 2019 15:45 pollita@php.net (Sara Golemon)
On Wed, Oct 2, 2019 at 8:24 AM Nikita Popov ppv@gmail.com> wrote:

> * The "Undefined array index" case. This one passed the vote with an exact > 2/3 majority, so I'm a bit uncomfortable making changes here. This is also > the only case where convincing usability concerns have been brought > forward, primarily around the $array[$key]++ example. I'm not yet decided > on what to do here and whether we should pursue additional library or > language features first. > > We raised margins to 2/3 to avoid having to feel bad about narrow
majorities. I say this as someone who was quite tempted to vote "Keep Notice" which on retrospect would have changed the entire outcome based on that one vote. If this had been a simple majority vote and it had pass by like.... 52%-48%, then obviously that's not a mandate worth making potentially catastrophic changes over, but this is 67% to 33%, it feels like you're okay here. -Sara
  107364
October 2, 2019 16:11 bishop@php.net (Bishop Bettini)
On Wed, Oct 2, 2019 at 11:45 AM Sara Golemon <pollita@php.net> wrote:

> On Wed, Oct 2, 2019 at 8:24 AM Nikita Popov ppv@gmail.com> wrote: > > > * The "Undefined array index" case. This one passed the vote with an > exact > > 2/3 majority, so I'm a bit uncomfortable making changes here. This is > also > > the only case where convincing usability concerns have been brought > > forward, primarily around the $array[$key]++ example. I'm not yet decided > > on what to do here and whether we should pursue additional library or > > language features first. > > > > > We raised margins to 2/3 to avoid having to feel bad about narrow > majorities. I say this as someone who was quite tempted to vote "Keep > Notice" which on retrospect would have changed the entire outcome based on > that one vote. > > If this had been a simple majority vote and it had pass by like.... > 52%-48%, then obviously that's not a mandate worth making potentially > catastrophic changes over, but this is 67% to 33%, it feels like you're > okay here. >
I'd say it's ok, also: with 63 votes cast, I think we have a quorum and therefore representative opinion. (Per [1], this vote is second only in participation to typed properties 2.0, with 71 votes cast.) I personally abstained, seeing both sides of the argument. [1]:https://php-rfc-watch.beberlei.de/
  107365
October 3, 2019 19:53 zeev@php.net (Zeev Suraski)
On Wed, 2 Oct 2019 at 15:24 Nikita Popov ppv@gmail.com> wrote:

> > * The "Undefined array index" case. This one passed the vote with an exact > 2/3 majority, so I'm a bit uncomfortable making changes here.
I think that making this change on the edge of a single vote is less than ideal... Even if I’m known for my pro-BC bias 🙂 If you’re looking for an excuse for why we might consider not going through with it even though it did clear the needed majority (if barely) - the vote ended roughly 5 hours before two weeks passed from the point the vote was announced. It’s not outside the realm of possibility that given the zero margin - the vote might have ended differently if these 5hrs were available... But I’d say it’s up to you. Zeev