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();
}
Am 06.08.2014 um 06:38 schrieb Sara Golemon:
Makes sense to me; +1.
yet another stupid implementation driven behavior :)
+1 for master.
Thanks. Dmitry.
On Wed, Aug 6, 2014 at 9:31 AM, Sebastian Bergmann sebastian@php.net
wrote:
Am 06.08.2014 um 06:38 schrieb Sara Golemon:
Makes sense to me; +1.
yet another stupid implementation driven behavior :)
+1 for master.
Thanks. Dmitry.
Yep, definitely yes +1.
Julien.Pauli
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
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]
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?
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
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
Lester Caine 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.
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]
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
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?
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
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.
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.
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.
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
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
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
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.
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
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.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
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
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/
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.
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
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.
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/
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.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.
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. :-)
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.
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/
On Wed, Aug 13, 2014 at 3:24 AM, Peter Cowburn petercowburn@gmail.com
wrote:
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. :-)
+1, my thoughts as well.
Best regards,
--Matt
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.
I agree with Andrea that this fix/change is arguably not a BC break. For
something where there is broad consensus that it's a bug (or at the very least
highly undesirable behavior) and the fix has very low risk of breaking existing
programs, it seems poor to update the spec to codify the bug. While the goal of
the spec is to describe PHP 5.6's current behavior as it is, I think it should
stop short of requiring that conforming implementations mimic existing bugs in
php.net PHP 5.6.
Making the spec codify bugs is bad for two reasons. First, it encourages other
implementations to mimic bugs causing these bugs to propagate further and
making them harder to ultimately fix. Second, it makes the spec more
complicated and more difficult to maintain without delivering any real value.
It's okay if php.net PHP is not 100% spec compliant as long as the all the
differences are considered bugs.
Going the RFC route for this issue felt a bit a like overkill to me, but I
respect the process :). The spirited discussion here was interesting to read
and informative. I don't have strong feelings about what version of php.net PHP
the fix should be applied to, or whether it should be allowed but E_DEPRECATED
for a period of time, but IMHO getting the fix in sooner rather than later
feels like the right way to go.
For future cases like this that are initially filed as bugs, I think discussion
should first focus exhaustively on determining whether or not the issue is a
bug before moving on to discussing whether the spec should be changed. I think
this will help reduce churn on the spec and will keep things simpler and more
light-weight for most cases like this.
-Drew
On Wed, Aug 13, 2014 at 3:24 AM, Peter Cowburn petercowburn@gmail.com
wrote: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. :-)+1, my thoughts as well.
Best regards,
--Matt
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.I agree with Andrea that this fix/change is arguably not a BC break. For
something where there is broad consensus that it's a bug (or at the very
least
highly undesirable behavior) and the fix has very low risk of breaking
existing
programs, it seems poor to update the spec to codify the bug. While the
goal of
the spec is to describe PHP 5.6's current behavior as it is, I think it
should
stop short of requiring that conforming implementations mimic existing
bugs in
php.net PHP 5.6.Making the spec codify bugs is bad for two reasons. First, it encourages
other
implementations to mimic bugs causing these bugs to propagate further and
making them harder to ultimately fix. Second, it makes the spec more
complicated and more difficult to maintain without delivering any real
value.
It's okay if php.net PHP is not 100% spec compliant as long as the all the
differences are considered bugs.Going the RFC route for this issue felt a bit a like overkill to me, but I
respect the process :). The spirited discussion here was interesting to
read
and informative. I don't have strong feelings about what version of
php.net PHP
the fix should be applied to, or whether it should be allowed but
E_DEPRECATED
for a period of time, but IMHO getting the fix in sooner rather than later
feels like the right way to go.For future cases like this that are initially filed as bugs, I think
discussion
should first focus exhaustively on determining whether or not the issue is
a
bug before moving on to discussing whether the spec should be changed. I
think
this will help reduce churn on the spec and will keep things simpler and
more
light-weight for most cases like this.-Drew
On Wed, Aug 13, 2014 at 9:22 AM, Matthew Fonda matthewfonda@gmail.com
wrote:On Wed, Aug 13, 2014 at 3:24 AM, Peter Cowburn petercowburn@gmail.com
wrote: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. :-)+1, my thoughts as well.
Best regards,
--Matt--
I do feel it necessary to point out at this juncture that the last 16 hours
of this thread are EXACTLY why the RFC process requires a minimum of two
weeks of discussion before voting is initiated on language-touching
proposals. I'll repeat my request that the author either cancel the
premature vote and wait the full two weeks or just cancel the RFC
altogether. If you don't feel an RFC is necessary, that's fine. But if
you do post an RFC, please follow the rules.
--Kris
From: Andrea Faulds [mailto:ajf@ajf.me]
Sent: Wednesday, August 13, 2014 11:43 AM
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.
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 ;-)
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.
not sure what do you mean here, multiple default cases can be reached the
same way as any other duplicated case branch(as shown in my snippet in the
mail you replied to).
even if those were always "silently ignored" as you falsely claim, it would
still be a BC break, as code working before this change would stop working
with a fatal error.
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.
it is a BC break, but I tend to agree that there isn't much code
intentionally using this behavior (but I'm also think that nobody really
looked for examples and just guessing about the usage of this feature), but
we had explicit phpt test for this behavior so I would call it a bad design
decision or lazy coding than an actual bug or hindsight.
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.
sure, and those which we consider bugs are usually tests which expect the
correct behavior and marked as XFAIL so we won't forget that they should be
fixed.
those which are testing and expecting the "bad" behavior is for us to not
change those unintentionally, alas introduce unintentional BC breaks.
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?
I think you forget the fall-through cases (eg. a case/default without a
break/continue), in those cases all of the matching cases will be executed
(even the duplicated ones as they are all matching)
This “feature” is completely and utterly useless, and I’ll eat my hat if
anyone intentionally relied on it.
not sure about the multiple defaults, but I'm fairly sure that there are
code out there which uses switch blocks as state-machines, and changes the
switch variable in the case blocks, and have duplicated case blocks for
executing code after the switch variable reached it's final form.
and I consider the default block as a glorified wildcard case pattern, so I
don't think that it is a good idea to prevent having multiple default
blocks while allowing the normal case blocks to check for the same value
multiple times.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
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.not sure what do you mean here, multiple default cases can be reached the
same way as any other duplicated case branch(as shown in my snippet in the
mail you replied to).
even if those were always "silently ignored" as you falsely claim, it would
still be a BC break, as code working before this change would stop working
with a fatal error.
It is a bug. No one should be using this, get rid of it.
However: not until the next major version. There are prob a few bits of code
that have this in there -- accidentally. People expect to perform minor upgrades
without breakage - so just leave it there. When there is a major upgrade it is
expected that a few things might not work - so do it when people a rebuilding
systems and expect such things.
--
Alain Williams
Linux/GNU Consultant - Mail systems, Web sites, Networking, Programmer, IT Lecturer.
+44 (0) 787 668 0256 http://www.phcomp.co.uk/
Parliament Hill Computers Ltd. Registration Information: http://www.phcomp.co.uk/contact.php
#include <std_disclaimer.h
Ferenc Kovacs wrote (on 13/08/2014):
not sure what do you mean here, multiple default cases can be reached the
same way as any other duplicated case branch(as shown in my snippet in the
mail you replied to).
If you're talking about http://3v4l.org/eZdPU then those duplicates are
definitely being silently ignored. Remember that the switch statement is
just a glorified goto, so execution starts at the first matching case
and then carries on until you hit a "break" or the end of the switch block.
In that example, it finds the first "case 'foo':", does a goto, and at
that point all the other labels are completely irrelevant; the 4 echo
statements are executed in order as though the other labels didn't exist.
If you feed it something other than 'foo', then the behaviour being
discussed kicks in: for whatever reason, the last default label is
selected as the target of the goto, so only the last echo is executed:
http://3v4l.org/TfYiQ
Interestingly, HHVM apparently behaves the same on that example,
although others mentioned it selecting the first rather than last
default label.
not sure about the multiple defaults, but I'm fairly sure that there are
code out there which uses switch blocks as state-machines, and changes the
switch variable in the case blocks, and have duplicated case blocks for
executing code after the switch variable reached it's final form.
Multiple cases do not make any difference here; the label to jump to is
selected only once, so if you want to change the state and jump to a
different label, you have to wrap the switch in a loop, and evaluate it
again. If you specify multiple labels matching the same state, the
second one can never be reached. http://3v4l.org/i3746
--
Rowan Collins
[IMSoP]
On Wed, Aug 13, 2014 at 2:52 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote (on 13/08/2014):
not sure what do you mean here, multiple default cases can be reached the
same way as any other duplicated case branch(as shown in my snippet in the
mail you replied to).If you're talking about http://3v4l.org/eZdPU then those duplicates are
definitely being silently ignored. Remember that the switch statement is
just a glorified goto, so execution starts at the first matching case and
then carries on until you hit a "break" or the end of the switch block.
oh, I thought Andrea was referring to ignoring the code from the case, not
the "label" of the case block.
yeah, that is how it should work except in the original bugreport, where we
jump to the last matching case but only when there is no previous
non-default matching case.
In that example, it finds the first "case 'foo':", does a goto, and at
that point all the other labels are completely irrelevant; the 4 echo
statements are executed in order as though the other labels didn't exist.If you feed it something other than 'foo', then the behaviour being
discussed kicks in: for whatever reason, the last default label is
selected as the target of the goto, so only the last echo is executed:
http://3v4l.org/TfYiQInterestingly, HHVM apparently behaves the same on that example, although
others mentioned it selecting the first rather than last default label.
yeah, I noticed this and mentioned in my comment to the bugreport.
not sure about the multiple defaults, but I'm fairly sure that there are
code out there which uses switch blocks as state-machines, and changes the
switch variable in the case blocks, and have duplicated case blocks for
executing code after the switch variable reached it's final form.Multiple cases do not make any difference here; the label to jump to is
selected only once, so if you want to change the state and jump to a
different label, you have to wrap the switch in a loop, and evaluate it
again. If you specify multiple labels matching the same state, the second
one can never be reached. http://3v4l.org/i3746
I think you misunderstood that part, I was thinking about a state machine
like this:
- function doStuff(){
- switch($state){
-
case OPENDOOR: -
if(!opendoor()){ -
break; -
} -
$state = SITDOWN; -
case SITDOWN: -
if(!sitdown()){ -
break; -
} -
$state = SIPWHISKEY; -
case SIPWHISKEY: -
sipwhiskey(); - }
- }
where you modify the switch variable in one case and fall through into
another.
as I mentioned I don't have a reasonable use-case for multiple defaults,
but I can see some for multiple case labels in general, and I don't think
that it is better to have different behavior for case and default in this
regard.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote (on 13/08/2014):
I think you misunderstood that part, I was thinking about a state
machine like this:
- function doStuff(){
- switch($state){
- case OPENDOOR:
- if(!opendoor()){
- break;
}$state = SITDOWN;- case SITDOWN:
- if(!sitdown()){
- break;
}$state = SIPWHISKEY;- case SIPWHISKEY:
sipwhiskey();- }
- }
where you modify the switch variable in one case and fall through into
another.
as I mentioned I don't have a reasonable use-case for multiple
defaults, but I can see some for multiple case labels in general, and
I don't think that it is better to have different behavior for case
and default in this regard.
The assignments to $state in that code do not make any difference to
execution, control simply flows forwards whenever there is no "break".
$state is only tested once, to select the initial label to jump to, so
no amount of reassigning it, or adding duplicate labels, can ever cause
a second jump.
I'm tempted to write a PHP script that emulates the switch execution by
building a set of if and goto statements, which might make some of this
behaviour clearer (not just for this discussion, but for other confusion
I've seen elsewhere), but I don't have time right now.
On Wed, Aug 13, 2014 at 3:55 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote (on 13/08/2014):
I think you misunderstood that part, I was thinking about a state machine
like this:
- function doStuff(){
- switch($state){
- case OPENDOOR:
- if(!opendoor()){
- break;
}$state = SITDOWN;- case SITDOWN:
- if(!sitdown()){
- break;
}$state = SIPWHISKEY;- case SIPWHISKEY:
sipwhiskey();- }
- }
where you modify the switch variable in one case and fall through into
another.
as I mentioned I don't have a reasonable use-case for multiple defaults,
but I can see some for multiple case labels in general, and I don't think
that it is better to have different behavior for case and default in this
regard.The assignments to $state in that code do not make any difference to
execution, control simply flows forwards whenever there is no "break".$state is only tested once, to select the initial label to jump to, so no
amount of reassigning it, or adding duplicate labels, can ever cause a
second jump.I'm tempted to write a PHP script that emulates the switch execution by
building a set of if and goto statements, which might make some of this
behaviour clearer (not just for this discussion, but for other confusion
I've seen elsewhere), but I don't have time right now.
yeah, and that is a documented behavior, I've just had a momentary
brain-fart here (which is funny considering that at one point I even wrote
a codesniffer rule for generating warnings for fall-through case statements
without an explicit comment).
thanks again for pointing that out. and sorry for the noise.
ps: I still hold onto my no vote, even thought it is a small BC break, it
is still a BC break, I wouldn't consider the current patch as a bugfix(and
even thought that the spec documented this as not a valid syntax hhvm still
supported it, ableit picking the first default when there are more than
one) , I would prefer having it E_DEPRECATED first in 5.next and we can
remove it in 7.0.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
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]
On Wed, Aug 13, 2014 at 12:22 PM, Rowan Collins rowan.collins@gmail.com
wrote:
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/eZdPUMultiple 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-statementIt 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:
}
agree, and I thought about mentioning that, but I left that out, because
I'm not proposing to remove that ability (and I see more use-cases for that
as mentioned in my previous email) but stating that this patch would make
an arbitrary distinction between the normal and the default cases.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote (on 13/08/2014):
agree, and I thought about mentioning that, but I left that out,
because I'm not proposing to remove that ability (and I see more
use-cases for that as mentioned in my previous email) but stating that
this patch would make an arbitrary distinction between the normal and
the default cases.
I'm not sure I agree that it's arbitrary. Having a switch with two
"default" clauses is like having an if with two "else" clauses;
specifying two case labels which can be true at the same time is like
having two "elseif" clauses chained together that can both be true at
the same time. One is clearly an error; the other is just poorly written
logic, and you can't make the parser outlaw every instance of poor logic.
--
Rowan Collins
[IMSoP]
On Wed, Aug 13, 2014 at 2:27 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote (on 13/08/2014):
agree, and I thought about mentioning that, but I left that out, because
I'm not proposing to remove that ability (and I see more use-cases for that
as mentioned in my previous email) but stating that this patch would make
an arbitrary distinction between the normal and the default cases.I'm not sure I agree that it's arbitrary. Having a switch with two
"default" clauses is like having an if with two "else" clauses; specifying
two case labels which can be true at the same time is like having two
"elseif" clauses chained together that can both be true at the same time.
One is clearly an error; the other is just poorly written logic, and you
can't make the parser outlaw every instance of poor logic.
Default is more of a catch all directive than an else (which will only
match if nothing else), so I don't think that having two default is more
"wrong" than having two "case 2:".
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
On Wed, Aug 13, 2014 at 2:27 PM, Rowan Collins rowan.collins@gmail.com
wrote:Ferenc Kovacs wrote (on 13/08/2014):
agree, and I thought about mentioning that, but I left that out, because
I'm not proposing to remove that ability (and I see more use-cases for that
as mentioned in my previous email) but stating that this patch would make
an arbitrary distinction between the normal and the default cases.I'm not sure I agree that it's arbitrary. Having a switch with two
"default" clauses is like having an if with two "else" clauses; specifying
two case labels which can be true at the same time is like having two
"elseif" clauses chained together that can both be true at the same time.
One is clearly an error; the other is just poorly written logic, and you
can't make the parser outlaw every instance of poor logic.Default is more of a catch all directive than an else (which will only
match if nothing else), so I don't think that having two default is more
"wrong" than having two "case 2:".--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Btw. I've added a comment to https://bugs.php.net/bug.php?id=67757 as I've
found some really weird behavior for multiple defaults and I think that it
would be better to try to figure out and fix why do we pick the last
default in the problematic snippet instead of trying to remove the multiple
default support to get rid of the setup which can make this bug to surface.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote (on 13/08/2014):
Btw. I've added a comment to https://bugs.php.net/bug.php?id=67757 as
I've found some really weird behavior for multiple defaults and I
think that it would be better to try to figure out and fix why do we
pick the last default in the problematic snippet instead of trying to
remove the multiple default support to get rid of the setup which can
make this bug to surface.
You're misinterpreting the behaviour of http://3v4l.org/r3poR
It is not selecting the first default, it just never needs to look for a
default, because it has already found a matching case label.
If you supply a value which requires the default label to be selected,
the last default label will be selected, as in all other examples to
date: http://3v4l.org/lQ7WE
On Wed, Aug 13, 2014 at 2:58 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote (on 13/08/2014):
Btw. I've added a comment to https://bugs.php.net/bug.php?id=67757 as
I've found some really weird behavior for multiple defaults and I think
that it would be better to try to figure out and fix why do we pick the
last default in the problematic snippet instead of trying to remove the
multiple default support to get rid of the setup which can make this bug to
surface.You're misinterpreting the behaviour of http://3v4l.org/r3poR
It is not selecting the first default, it just never needs to look for a
default, because it has already found a matching case label.
yes, and it continues the execution from the first matching case and
correctly evaluates the following defaults, as default matches everything,
which imo should be the correct behavior.
If you supply a value which requires the default label to be selected, the
last default label will be selected, as in all other examples to date:
http://3v4l.org/lQ7WE
having a different behavior for default when there is a preceeding matching
empty case is not the expected(from the userland point of view) behavior
imo.
ps: sorry for the imprecise/lousy definitions on my part, I hope I can get
my message through though.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote (on 13/08/2014):
It is not selecting the first default, it just never needs to look for a default, because it has already found a matching case label.yes, and it continues the execution from the first matching case and
correctly evaluates the following defaults, as default matches
everything, which imo should be the correct behavior.
No, it does not "evaluate the following defaults"; it evaluates all
following code. Once a label has been selected, you can think of all
subsequent labels being removed from the code completely.
having a different behavior for default when there is a preceeding
matching empty case is not the expected(from the userland point of
view) behavior imo.
There is no different behaviour. Labels which do not match the input
have no effect on execution, and default labels have no effect if any
other label matched.
On Wed, Aug 13, 2014 at 3:42 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote (on 13/08/2014):
It is not selecting the first default, it just never needs to look for a default, because it has already found a matching case label.yes, and it continues the execution from the first matching case and
correctly evaluates the following defaults, as default matches everything,
which imo should be the correct behavior.No, it does not "evaluate the following defaults"; it evaluates all
following code. Once a label has been selected, you can think of all
subsequent labels being removed from the code completely.
duh, that was embarrassing, thanks for bearing with me until I realized
that. :S
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Even if it is fall through that is causing the behavior it still is a
syntax that no longer will be considered valid in a minor release - and
seemingly in a point release as well if everyone had their way here. [:
Not that I consider code likehttp://3v4l.org/ http://3v4l.org/mtG4KmtG4K
http://3v4l.org/mtG4Ka shining jewel but I could see people using this
behavior. I also would like to second the notion that was brought up that
this is and has been in HHVM if 3v4l is to be believed. It even selects
the last default like PHP 5 does, not the first - again also brought up
before.
Hi
2014-08-13 13:02 GMT+02:00 Ferenc Kovacs tyra3l@gmail.com:
agree, and I thought about mentioning that, but I left that out, because
I'm not proposing to remove that ability (and I see more use-cases for that
as mentioned in my previous email) but stating that this patch would make
an arbitrary distinction between the normal and the default cases.
In PHP we already disallow overriding $this, like:
class A {
public function B() {
$this = 'hello'; // error
${'this'} = 'hello'; // error
${'t' . 'his'} = 'hello'; // works
}
}
so agreed too, we cannot disallow such cases that can be variable with
any gain, and it does make sense you cannot with $this, but if you
hack it like above, then you are asking for trouble anyway
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Hi
2014-08-13 13:02 GMT+02:00 Ferenc Kovacs tyra3l@gmail.com:
agree, and I thought about mentioning that, but I left that out, because
I'm not proposing to remove that ability (and I see more use-cases for
that
as mentioned in my previous email) but stating that this patch would make
an arbitrary distinction between the normal and the default cases.In PHP we already disallow overriding $this, like:
class A {
public function B() {
$this = 'hello'; // error
${'this'} = 'hello'; // error
${'t' . 'his'} = 'hello'; // works
}
}so agreed too, we cannot disallow such cases that can be variable with
any gain, and it does make sense you cannot with $this, but if you
hack it like above, then you are asking for trouble anyway
it is offtopic imo, but I also complained about that(
https://bugs.php.net/bug.php?id=52428) but I think that it is a bit
different beast, makes it harder for you to shot yourself to the foot, but
the we couldn't find a way to completelly prevent you without noticable
performance sacrifice if you go out your way to do so, so I think that the
current best-effort approach is ok.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
I woke up to this thread, and at first I thought:
Oh, some actual discussion, maybe we should stop the vote after all
and give it some time...
One or two minor points here...
And there's the bike shed.
For my own sanity, I'm going to leave this to the people who care
about the import things like what color you paint the bike shed.
When you're done, whoever wants to take over the rfc is welcome to.
Whoever wants to take my patch (simple as it is), is welcome to.
I just don't have time for this.
Plenty of love, but not enough patience,
-Sara
I woke up to this thread, and at first I thought:
Oh, some actual discussion, maybe we should stop the vote after all
and give it some time...
One or two minor points here...
And there's the bike shed.For my own sanity, I'm going to leave this to the people who care
about the import things like what color you paint the bike shed.
When you're done, whoever wants to take over the rfc is welcome to.
Whoever wants to take my patch (simple as it is), is welcome to.I just don't have time for this.
Plenty of love, but not enough patience,
Btw, I do not mind this exact change to be in 5.x but we should be
very careful for future ones. We somehow hijack your RFC thread with
the general discussions about specs fixes, necessary one :)
--
Pierre
@pierrejoye | http://www.libgd.org
Hi!
And this, which is completely undetectable at parse time:
$bar = 30;
// ... arbitrary amount of code
switch ( $foo ) {
case $bar:
case 30:
}
That's not all. You can also do:
switch (true) {
case foo($bar):
...
case baz($quz, $qux):
...
}
and this is a completely valid code which I've seen in the field.
--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/