# [RFC] Disallow multiple default blocks in a single switch statement - Thread: https://externals.io/message/76368 - Original archive: https://news-web.php.net/php.internals/76368 - Started by Sara Golemon on 2014-08-06T04:38:51+00:00 - Messages: 48 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/76368.md?page=2 - 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 --- ## #76368 — Sara Golemon — 2014-08-06T04:38:51+00:00 URL: https://externals.io/message/76368#76368 https://wiki.php.net/rfc/switch.default.multiple 'Cause this code is silly (even if it had case blocks), but we allow it: switch ($expr) { default: notExecuted(); break; default: executed(); } --- ## #76369 — Sebastian Bergmann — 2014-08-06T05:31:18+00:00 URL: https://externals.io/message/76368#76369 · In reply to: #76368 Am 06.08.2014 um 06:38 schrieb Sara Golemon: > https://wiki.php.net/rfc/switch.default.multiple Makes sense to me; +1. --- ## #76370 — Dmitry Stogov — 2014-08-06T08:57:34+00:00 URL: https://externals.io/message/76368#76370 · In reply to: #76369 yet another stupid implementation driven behavior :) +1 for master. Thanks. Dmitry. On Wed, Aug 6, 2014 at 9:31 AM, Sebastian Bergmann wrote: > Am 06.08.2014 um 06:38 schrieb Sara Golemon: > > […] > > Makes sense to me; +1. --- ## #76371 — Julien Pauli — 2014-08-06T08:59:37+00:00 URL: https://externals.io/message/76368#76371 · In reply to: #76370 > yet another stupid implementation driven behavior :) > > +1 for master. > > Thanks. Dmitry. > Yep, definitely yes +1. Julien.Pauli --- ## #76372 — Ferenc Kovacs — 2014-08-06T10:00:25+00:00 URL: https://externals.io/message/76368#76372 · In reply to: #76368 > https://wiki.php.net/rfc/switch.default.multiple > > 'Cause this code is silly (even if it had case blocks), but we allow it: > > switch ($expr) { > default: > notExecuted(); > break; > default: > executed(); > } > > -- > > > > Hi, I think dropping this behavior is a good idea, but I'm confused by the reasoning related to the langspec. This rfc targets php.next (which is a safe move as this has BC break albeit would require some questionable code), but the langspec was agreed to be based on 5.6 and document how that works. So even if we accept this rfc, and remove the multiple default case, wouldn't we still need to document the current behavior in the spec (maybe mentioning that it will go away in php-net)? -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #76373 — Rowan Collins — 2014-08-06T10:16:14+00:00 URL: https://externals.io/message/76368#76373 · In reply to: #76372 Ferenc Kovacs wrote (on 06/08/2014): > Hi, > > I think dropping this behavior is a good idea, but I'm confused by the > reasoning related to the langspec. > This rfc targets php.next (which is a safe move as this has BC break albeit > would require some questionable code), but the langspec was agreed to be > based on 5.6 and document how that works. > So even if we accept this rfc, and remove the multiple default case, > wouldn't we still need to document the current behavior in the spec (maybe > mentioning that it will go away in php-net)? According to the bug report, HHVM also accepts multiple default blocks, but uses the first rather than the last. It's probably not worth implementing specific code there to take the last default label just in order to adhere to a 5.6 spec, but is worth making it detect multiple labels to bring it in line with the proposed change. So either we declare the php.net implementation to be in violation of the spec (which isn't completely insane - the spec shouldn't mirror behaviour to the point of incorporating the entire bug list); or we document the php.net behaviour as "correct", but accept that nobody is going to implement that part of the spec as written, and fix it in the next release of both the implementation and the spec. Having the 5.6 spec match behaviour, and starting a 5.7 draft immediately for implementations to actually target might be a compromise? -- Rowan Collins [IMSoP] --- ## #76374 — Pierre Joye — 2014-08-06T10:44:49+00:00 URL: https://externals.io/message/76368#76374 · In reply to: #76373 > Ferenc Kovacs wrote (on 06/08/2014): > > > […] > > > According to the bug report, HHVM also accepts multiple default blocks, but > uses the first rather than the last. It's probably not worth implementing > specific code there to take the last default label just in order to adhere > to a 5.6 spec, but is worth making it detect multiple labels to bring it in > line with the proposed change. > > So either we declare the php.net implementation to be in violation of the > spec (which isn't completely insane - the spec shouldn't mirror behaviour to > the point of incorporating the entire bug list); or we document the php.net > behaviour as "correct", but accept that nobody is going to implement that > part of the spec as written, and fix it in the next release of both the > implementation and the spec. > > Having the 5.6 spec match behaviour, and starting a 5.7 draft immediately > for implementations to actually target might be a compromise? Too late for 5.6 but it would have been one of these acceptable BC and sanity change. I did not even know it was possible and I do not think anyone would be insane enough to rely on that :) We will need a 5.7 anyway, so yes, 5.7 sounds like a reasonable target. And I indeed like this RFC, specs work is paying off. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76375 — Lester Caine — 2014-08-06T12:00:06+00:00 URL: https://externals.io/message/76368#76375 · In reply to: #76373 > According to the bug report, HHVM also accepts multiple default blocks, > but uses the first rather than the last. It's probably not worth > implementing specific code there to take the last default label just in > order to adhere to a 5.6 spec, but is worth making it detect multiple > labels to bring it in line with the proposed change. > > So either we declare the php.net implementation to be in violation of > the spec (which isn't completely insane - the spec shouldn't mirror > behaviour to the point of incorporating the entire bug list); or we > document the php.net behaviour as "correct", but accept that nobody is > going to implement that part of the spec as written, and fix it in the > next release of both the implementation and the spec. It is perhaps interesting that this problem has not turned up before? As I indicated in another thread I was surprised to see 'default' as the first item in an example. I'm sure I'm not alone in always putting it at the end, and I quite often will have a previous block which does not have a 'break' so that the default element is added to other actions on some keys for that reason. How much of the core PHP is still 'normal practice' rather than 'by design' :) -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #76378 — Rowan Collins — 2014-08-06T12:40:16+00:00 URL: https://externals.io/message/76368#76378 · In reply to: #76375 Lester Caine wrote (on 06/08/2014): > > > […] > It is perhaps interesting that this problem has not turned up before? As > I indicated in another thread I was surprised to see 'default' as the > first item in an example. I'm sure I'm not alone in always putting it at > the end, and I quite often will have a previous block which does not > have a 'break' so that the default element is added to other actions on > some keys for that reason. How much of the core PHP is still 'normal > practice' rather than 'by design' :) > You could equally have the default case run some extra logic *before* one of the steps, since it doesn't have to be last. The goto-like behaviour of case and default labels originates in C, which makes this answer on SO worth mentioning: http://stackoverflow.com/a/3110478/157957 The reason some languages require default to be the last label is presumably that they treat it as equivalent to a case statement that always matches, and so never test lower labels. "Normal practice" is often stricter than what is technically possible in a language, because it is aimed at writing readable and maintainable code. Which in turn has no bearing on the ability to specify multiple defaults (which never makes sense, except as a result of a lazy code generator). -- Rowan Collins [IMSoP] --- ## #76402 — Sara Golemon — 2014-08-06T18:35:44+00:00 URL: https://externals.io/message/76368#76402 · In reply to: #76372 > I think dropping this behavior is a good idea, but I'm confused by the > reasoning related to the langspec. > This rfc targets php.next (which is a safe move as this has BC break albeit > would require some questionable code), but the langspec was agreed to be > based on 5.6 and document how that works. > Did we agree on that? The lang spec was originally written to 5.6 to have a relatively stable target, but (in my mind at least) was meant to track master as we move the language forward. Was there a discussion about branching the langspec repo for versions? Rowan Collins wrote: > According to the bug report, HHVM also accepts multiple default blocks, > but uses the first rather than the last. It's probably not worth implementing > specific code there to take the last default label just in order to adhere to > a 5.6 spec, but is worth making it detect multiple labels to bring it in line > with the proposed change. > Yeah, HHVM needs to be fixed as well, but I left that out of the scope of php-lang versions php-implementation issue. -Sara --- ## #76403 — Ferenc Kovacs — 2014-08-06T19:32:04+00:00 URL: https://externals.io/message/76368#76403 · In reply to: #76402 > > > […] > albeit > > […] > Did we agree on that? The lang spec was originally written to 5.6 to > have a relatively stable target, but (in my mind at least) was meant > to track master as we move the language forward. Was there a > discussion about branching the langspec repo for versions? maybe that was just my impression. I'm not sure what would be the best solution, but if we don't version the spec, then when we introduce BC breaks or simply new features in a new version which is in turn get's added to the spec, that would make the older php version's(from any implementation) not being compliant with the spec. would be nice checking out how other spec-driven languages manage this problem (I know that at least java has separate spec for each major version), but I don't think that a single spec can exists which allows alternative implementations to say that they comform me spec while the reference implementation and the spec can still change/evolve after the initial release. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #76404 — Adam Harvey — 2014-08-06T19:36:05+00:00 URL: https://externals.io/message/76368#76404 · In reply to: #76403 > > > […] > > > maybe that was just my impression. > I'm not sure what would be the best solution, but if we don't version the > spec, then when we introduce BC breaks or simply new features in a new > version which is in turn get's added to the spec, that would make the older > php version's(from any implementation) not being compliant with the spec. > would be nice checking out how other spec-driven languages manage this > problem (I know that at least java has separate spec for each major > version), but I don't think that a single spec can exists which allows > alternative implementations to say that they comform me spec while the > reference implementation and the spec can still change/evolve after the > initial release. I guess I (possibly) misinterpreted that too, which is why I changed the original bug report to be a spec bug rather than a ZE bug. I too was under the impression that the spec right now would document 5.6 as it actually is (rather than being aspirational), and then future versions would be separate documents (branches, presumably) that would be updated as part of the RFC process when changes were made. Adam, who has just come to the horrible realisation that this is going to require someone to write an RFC. Dibs not me. --- ## #76407 — Sara Golemon — 2014-08-07T16:50:56+00:00 URL: https://externals.io/message/76368#76407 · In reply to: #76368 > https://wiki.php.net/rfc/switch.default.multiple > Added an implementation: https://github.com/sgolemon/php-src/compare/switch.default.multiple -Sara P.S. - For who care, a similar fix will be landing in HHVM shortly. --- ## #76461 — Ferenc Kovacs — 2014-08-13T07:47:38+00:00 URL: https://externals.io/message/76368#76461 · In reply to: #76368 > https://wiki.php.net/rfc/switch.default.multiple > > 'Cause this code is silly (even if it had case blocks), but we allow it: > > switch ($expr) { > default: > notExecuted(); > break; > default: > executed(); > } > > -- > > > > Hi, sorry to jump in this late, but I'm not sure that it is a good idea to only reject the multiple default blocks but keep the ability to have the same case multiple times: http://3v4l.org/eZdPU in this regard I think the current status is more consistent as it would after merging this patch. and I also think that this isn't an important enough issue to warrant a BC break (albeit this is the better kind of BC: probably doesn't effect too many people, and they will be clearly notified about the error at compile time) so I voted no based on this two thing. -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #76462 — James — 2014-08-13T08:13:57+00:00 URL: https://externals.io/message/76368#76462 · In reply to: #76461 Just saw the RFC today so I hadn't been able to comment on it until now.. If we are getting rid of it wouldn't it be better to emit an `E_DEPRECATED` per https://github.com/imnotjames/php-src/compare/switch.default.multiple-deprecated and then remove it in the next major release? This has been known to at least one person for many years if 034.phpt is to be believed. I could see where people would use it - there are reasons to, even if they are poor in choice to do so. I entirely believe this behavior is weird and should be removed. However, breaking backwards compatibility in a minor release because the incomplete spec says so is kind of odd. A BC break is a BC break, which doesn't belong in a minor revision. Regards, James > > > > […] > Hi, > > sorry to jump in this late, but I'm not sure that it is a good idea to only > reject the multiple default blocks but keep the ability to have the same > case multiple times: > http://3v4l.org/eZdPU > in this regard I think the current status is more consistent as it would > after merging this patch. > and I also think that this isn't an important enough issue to warrant a BC > break (albeit this is the better kind of BC: probably doesn't effect too > many people, and they will be clearly notified about the error at compile > time) so I voted no based on this two thing. > > > -- > Ferenc Kovács > @Tyr43l - http://tyrael.hu --- ## #76463 — Pierre Joye — 2014-08-13T08:38:50+00:00 URL: https://externals.io/message/76368#76463 · In reply to: #76461 > > > > […] > Hi, > > sorry to jump in this late, but I'm not sure that it is a good idea to only > reject the multiple default blocks but keep the ability to have the same > case multiple times: > http://3v4l.org/eZdPU > in this regard I think the current status is more consistent as it would > after merging this patch. > and I also think that this isn't an important enough issue to warrant a BC > break (albeit this is the better kind of BC: probably doesn't effect too > many people, and they will be clearly notified about the error at compile > time) so I voted no based on this two thing. I have a mixed feeling too about doing this change in 5.x. Without nitpicking, it is still a BC break. I also think that making PHP somehow consistent, less confusing, etc. while working on the PHP specs, we should really target php7 for any of these changes. As of the discussions about the discussion phase, what is one week more anyway? Especially in summer when the lucky ones are in holidays? ;) Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76464 — Ferenc Kovacs — 2014-08-13T08:48:37+00:00 URL: https://externals.io/message/76368#76464 · In reply to: #76463 > > > […] > only > > […] > BC > > […] > > I have a mixed feeling too about doing this change in 5.x. Without > nitpicking, it is still a BC break. I also think that making PHP > somehow consistent, less confusing, etc. while working on the PHP > specs, we should really target php7 for any of these changes. > > As of the discussions about the discussion phase, what is one week > more anyway? Especially in summer when the lucky ones are in holidays? > ;) > > after re-reading my email, I realized that instead of "and I also think that this isn't an important enough issue to warrant a BC break" I wanted to say that and I also think that this isn't an important enough issue to warrant a BC break in a minor version -- Ferenc Kovács @Tyr43l - http://tyrael.hu --- ## #76465 — Lester Caine — 2014-08-13T09:04:19+00:00 URL: https://externals.io/message/76368#76465 · In reply to: #76464 > I wanted to say that > and I also think that this isn't an important enough issue to warrant a BC > break in a minor version Reading between the lines this does seem to be more a problem for HHVM which has not copied the current operation properly. The question of multiple case blocks has been flagged, and thinking about it, I quite often copy, paste and forget to update the label ;) Even the IDE does not flag that ... it's only when it does not work ... -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #76467 — Andrea Faulds — 2014-08-13T09:42:30+00:00 URL: https://externals.io/message/76368#76467 · In reply to: #76461 Hi! > and I also think that this isn't an important enough issue to warrant a BC > break (albeit this is the better kind of BC: probably doesn't effect too > many people, and they will be clearly notified about the error at compile > time) so I voted no based on this two thing. This isn’t really a BC break. Multiple default blocks didn’t actually work anyway, we just silently ignored extra ones. > I entirely believe this behavior is weird and should be removed. However, > breaking backwards > compatibility in a minor release because the incomplete spec says so is > kind of odd. A BC break > is a BC break, which doesn't belong in a minor revision. It isn’t a BC break that will affect anyone. It fixes a parser bug. > This has been known to at least one person for many years if 034.phpt is to > be believed. Just because it’s tested doesn’t mean anyone relies on it. We have plenty of tests which ensure PHP contains bugs and will error if they don’t. > I could see > where people would use it - there are reasons to, even if they are poor in > choice to do so. How, exactly, could there ever be a use for having multiple default: sections and ignoring all but one? This “feature” is completely and utterly useless, and I’ll eat my hat if anyone intentionally relied on it. Thanks!! -- Andrea Faulds http://ajf.me/ --- ## #76468 — Pierre Joye — 2014-08-13T09:52:43+00:00 URL: https://externals.io/message/76368#76468 · In reply to: #76467 > > […] > > How, exactly, could there ever be a use for having multiple default: sections and ignoring all but one? This “feature” is completely and utterly useless, and I’ll eat my hat if anyone intentionally relied on it. The question is more about how and when do we fix issues found while working on the specs? That one is pretty easy and the impact for users will be rather small, if at all. However, I tend to think that we should simply target php7 for any of these fixes. No need of endless arguing and keeps everyone happy. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #76469 — Andrea Faulds — 2014-08-13T09:56:32+00:00 URL: https://externals.io/message/76368#76469 · In reply to: #76468 > > > > […] > > The question is more about how and when do we fix issues found while > working on the specs? That one is pretty easy and the impact for users > will be rather small, if at all. > > However, I tend to think that we should simply target php7 for any of > these fixes. No need of endless arguing and keeps everyone happy. I’d rather we fix these in 5.7, which I’m a proponent of and would come out sooner. This way we don’t have nonsenses like this specified for three years. -- Andrea Faulds http://ajf.me/ --- ## #76470 — Rowan Collins — 2014-08-13T10:22:34+00:00 URL: https://externals.io/message/76368#76470 · In reply to: #76461 Ferenc Kovacs wrote (on 13/08/2014): > sorry to jump in this late, but I'm not sure that it is a good idea to only > reject the multiple default blocks but keep the ability to have the same > case multiple times: > http://3v4l.org/eZdPU Multiple cases with the same value are a lot harder to prevent (if not impossible), because there is no requirement for the cases to be static, or of the same type. The spec actually calls this out with a couple of examples: https://github.com/php/php-langspec/blob/master/spec/11-statements.md#the-switch-statement It would be a bit awkward if this was a syntax error: switch ( $foo ) { case 30: case 30: } But this was fine: switch ( $foo ) { case 30.0: case 30: case 10 * 3: } And this, which is completely undetectable at parse time: $bar = 30; // ... arbitrary amount of code switch ( $foo ) { case $bar: case 30: } -- Rowan Collins [IMSoP] --- ## #76471 — Peter Cowburn — 2014-08-13T10:24:29+00:00 URL: https://externals.io/message/76368#76471 · In reply to: #76469 > > > > > […] > poor in > > […] > sections and ignoring all but one? This “feature” is completely and utterly > useless, and I’ll eat my hat if anyone intentionally relied on it. > > […] > > I’d rather we fix these in 5.7, which I’m a proponent of and would come > out sooner. This way we don’t have nonsenses like this specified for three > years. > Since the spec is targeting 5.6 [citation needed], this behaviour will need to be in there regardless of in what version it gets fixed. > -- > Andrea Faulds > http://ajf.me/ > > > My thoughts on the topic? I think we're in danger of letting "process" get in our way here. It's a bug fix which IMHO should even be thrown into 5.6 (this is a bug fix!). Going through the RFC process, being forced to wait for 5.7 or 7, extended discussions on the list... it all just seems unnecessary. But, that's just the opinion of a docs guy. :-) --- ## #76472 — Andrea Faulds — 2014-08-13T10:34:32+00:00 URL: https://externals.io/message/76368#76472 · In reply to: #76471 > > > > > […] > > I’d rather we fix these in 5.7, which I’m a proponent of and would come out sooner. This way we don’t have nonsenses like this specified for three years. You might be misunderstanding me. The thing is we have to specify the behaviour of 5.6 now, probably this year. PHP 7 will likely take at least 2 years (I know some think differently, but realistically 7 will not just be phpng and even that has problems), but 5.7 would come out presumably next year. In which case, that means we’d be able to fix the language in just a year and hence make a 5.7 spec with the 5.6 issues fixed. -- Andrea Faulds http://ajf.me/ --- ## #76473 — Christian Stoller — 2014-08-13T10:34:59+00:00 URL: https://externals.io/message/76368#76473 · In reply to: #76467 From: Andrea Faulds [mailto:ajf@ajf.me] Sent: Wednesday, August 13, 2014 11:43 AM > > Hi! > > > > > […] > > This isn’t really a BC break. Multiple default blocks didn’t actually > work anyway, we just silently ignored extra ones. > > > > > […] > > It isn’t a BC break that will affect anyone. It fixes a parser bug. > If somebody unintentionally has multiple default blocks in his code and PHP is upgraded, he will get a parser error and his application will be broken. This definitely IS a BC break. But one could argue if this BC break will maybe help someone to find bugs in his code ;-) --- Next page: https://externals.io/message/76368.md?page=2