Hi All,
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in PHP 7.
I'm happy to expand this into a proper RFC, and work on patches for the
engine, tests, and spec, but want to see if initial reception to the
idea is favourable first.
Current behaviour: any bare word which is not a defined constant is
treated as though it were a quoted string, with an E_NOTICE
issued.
Proposed behaviour: an E_RECOVERABLE_ERROR
is issued; if the error is
caught, either the existing string behaviour could be retained, or the
value could be treated as NULL. (I'm open to other alternatives.)
PROS
- Consistency.
An undefined class constant (e.g. Foo::BAR) gives a fatal error, but a
normal global constant gives only a notice. Class constants being "more
modern" made this seem reasonable, but with namespaces, it's even worse
- a qualified namespace constant (e.g. Foo\BAR) gives a fatal error, as
does a global constant in namespace notation (e.g. \BAR); but a
namespace constant referenced relative to the current namespace is a
bare word, and gives only a notice (e.g. namespace Foo; const BAR=42;
echo BAAR;).
Functions and classes also give fatal errors if used without declaring;
variables do not, but this is more useful, because they can meaningfully
be created as null, and there isn't a syntax to declare a local
variable, only to initialise it.
- Inherent severity.
It's probably relatively common to turn off E_NOTICE
in error_reporting,
or to miss one notice among many others. But an undefined constant
magically turning into a string could cause relatively major problems.
Firstly, a constant is likely to have some value which needs to be used
or matched somewhere else. A READ_ONLY flag mis-typed as REED_ONLY could
be interpreted as int(0) (i.e. intval('REED_ONLY')) and cause a
catastrophic error.
Secondly, keywords are also bare words, and any expression can be a
statement. Thus the following is an infinite loop, no matter what
blahblah() returns:
while(true) {
if ( blahblah() ) {
brek;
}
}
Obviously, there are other mistakes which could cause such errors, but
this is one that the engine could easily pick up on and protect the user.
CONS
Like most changes, the downside is a break in compatibility.
As far as I can make out, the current behaviour is actually for
compatibility back as far as PHP 3 (correct me if I'm wrong, I didn't
dig far).
I'm not aware of any notice officially deprecating barewords-as-strings,
but nor can I find many references to it in the manual at all. Its use
for array keys has been explicitly discouraged in the manual since some
time in 2001. [1]
Outside code golf, I'd be very surprised if any code written or updated
in the last 10 years deliberately uses this "feature", and I think
changing it in PHP 7 is justifiable given the benefits to maintainers of
even the simplest non-OO, non-namespaced code.
What do people think?
Regards,
--
Rowan Collins
[IMSoP]
Hey Rowan,
I actually was thinking about doing this myself, if function/constant autoloading passed. That would mean that if you really needed this behaviour for some reason, you could write a constant autoloader that reimplements it.
One concern I’d have is the use of constants in constant scalar expressions. In implementation, we’d need to make sure those produce an error, not just normal constant usage.
Anyway, I’m very much in favour of this. Please go ahead!
Thanks.
Andrea Faulds
http://ajf.me/
De : Rowan Collins [mailto:rowan.collins@gmail.com]
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in
PHP 7.
+1
2015.01.28. 6:04 ezt írta ("François Laupretre" francois@tekwire.net):
De : Rowan Collins [mailto:rowan.collins@gmail.com]
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in
PHP 7.+1
I don't like that.
Depending if we want to keep the behavior on the long term I would either
go with E_WARNING
or E_DEPRECATED
Turning it into E_RECOVERABLE_ERROR
would be for most users just as bad as
removing it while we still have to support the behavior so no real gain for
us.
Ferenc Kovacs wrote on 28/01/2015 07:20:
2015.01.28. 6:04 ezt írta ("François Laupretre" <francois@tekwire.net
mailto:francois@tekwire.net>):De : Rowan Collins [mailto:rowan.collins@gmail.com
mailto:rowan.collins@gmail.com]
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in
PHP 7.+1
I don't like that.
Depending if we want to keep the behavior on the long term I would
either go withE_WARNING
orE_DEPRECATED
Turning it intoE_RECOVERABLE_ERROR
would be for most users just as
bad as removing it while we still have to support the behavior so no
real gain for us.
E_DEPRECATED
is likely to be even more ignored than E_NOTICE, so would
be a step backwards; my whole assumption is that most people getting the
message are not using a deprecated feature, but accidentally mis-typing
a constant or keyword. I'd be happy with raising it to E_WARNING
to make
it more noticeable, though, if we don't think removing the functionality
is acceptable.
On the other hand, if the concern is having to support it, then it could
just be E_ERROR. The reason I chose E_RECOVERABLE_ERROR
is because that
had recently been adopted for other things, such as invalid method
calls. As I mentioned in my original e-mail, the recovery behaviour
needn't be the existing string interpretation, but could just be to
treat the value as NULL, which would probably be simpler to implement.
Personally, I've never actually recovered an E_RECOVERABLE_ERROR, so am
not sure what use cases we should be supporting.
Regards,
Rowan Collins
[IMSoP]
Hi Rowan,
On Wed, Jan 28, 2015 at 7:48 PM, Rowan Collins rowan.collins@gmail.com
wrote:
On the other hand, if the concern is having to support it, then it could
just be E_ERROR. The reason I choseE_RECOVERABLE_ERROR
is because that had
recently been adopted for other things, such as invalid method calls. As I
mentioned in my original e-mail, the recovery behaviour needn't be the
existing string interpretation, but could just be to treat the value as
NULL, which would probably be simpler to implement. Personally, I've never
actually recovered an E_RECOVERABLE_ERROR, so am not sure what use cases we
should be supporting.
Please don't use E_ERROR. It cannot be catched by user error handler now.
If user error handler can catch it, then I don't mind it. E_RECOVERBLE_ERROR
is better than E_ERROR. I use user defined error handler always for
immediate
notification. I use log analyzer also, but it has time lag.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
On Wed, Jan 28, 2015 at 11:48 AM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote on 28/01/2015 07:20:
2015.01.28. 6:04 ezt írta ("François Laupretre" <francois@tekwire.net
mailto:francois@tekwire.net>):De : Rowan Collins [mailto:rowan.collins@gmail.com <mailto:
rowan.collins@gmail.com>]
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in
PHP 7.+1
I don't like that.
Depending if we want to keep the behavior on the long term I would either
go withE_WARNING
orE_DEPRECATED
Turning it intoE_RECOVERABLE_ERROR
would be for most users just as bad
as removing it while we still have to support the behavior so no real gain
for us.
E_DEPRECATED
is likely to be even more ignored than E_NOTICE, so would be
a step backwards; my whole assumption is that most people getting the
message are not using a deprecated feature, but accidentally mis-typing a
constant or keyword. I'd be happy with raising it toE_WARNING
to make it
more noticeable, though, if we don't think removing the functionality is
acceptable.
turn it into E_WARNING
if we are planning to keep the feature, turn it into
E_DEPRECATED
if we want to remove the feature later on.
On the other hand, if the concern is having to support it, then it could
just be E_ERROR. The reason I choseE_RECOVERABLE_ERROR
is because that had
recently been adopted for other things, such as invalid method calls. As I
mentioned in my original e-mail, the recovery behaviour needn't be the
existing string interpretation, but could just be to treat the value as
NULL, which would probably be simpler to implement. Personally, I've never
actually recovered an E_RECOVERABLE_ERROR, so am not sure what use cases we
should be supporting.
ok, I was assuming that by turning it into E_RECOVERABLE_ERROR
you would
still keep the assuming string behavior if there is a custom error handler
which returns true for this error.
I think if we want to remove the assuming string feature, we should handle
it the same way as we handle referencing undefined function call or class
reference(and for consistency the same should be considered for "Undefined
variable", which is also an E_NOTICE
atm).
at this point I think I would -1 on removing the assuming this, +0.5 on
turning it into E_DEPRECATED
and +1 on turning the "Use of undefined
constant"/"Undefined variable" errors into E_WARNING.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote on 28/01/2015 12:13:
`E_DEPRECATED` is likely to be even more ignored than E_NOTICE, so would be a step backwards; my whole assumption is that most people getting the message are not using a deprecated feature, but accidentally mis-typing a constant or keyword. I'd be happy with raising it to `E_WARNING` to make it more noticeable, though, if we don't think removing the functionality is acceptable.
turn it into
E_WARNING
if we are planning to keep the feature, turn it
intoE_DEPRECATED
if we want to remove the feature later on.
I see absolutely no advantage in changing this into E_DEPRECATED, as
it's even more likely to be hidden or ignored than E_NOTICE.
There are actually two parts to the message: that you used an undefined
constant, and that PHP interpreted this as you wanting a string. If you
really intended a string, then "deprecated" is relevant; but if you
intended a keyword or constant, then there is nothing to deprecate, it's
just an error.
If we want to keep the string fallback for now, how about we raise the
severity to E_WARNING, and change the message to mention deprecation?
e.g.
Warning: Use of undefined constant foo - assumed 'foo', but this
behaviour will change in a future version
Regards,
Rowan Collins
[IMSoP]
On Wed, Jan 28, 2015 at 4:42 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Ferenc Kovacs wrote on 28/01/2015 12:13:
`E_DEPRECATED` is likely to be even more ignored than E_NOTICE, so would be a step backwards; my whole assumption is that most people getting the message are not using a deprecated feature, but accidentally mis-typing a constant or keyword. I'd be happy with raising it to `E_WARNING` to make it more noticeable, though, if we don't think removing the functionality is acceptable.
turn it into
E_WARNING
if we are planning to keep the feature, turn it
intoE_DEPRECATED
if we want to remove the feature later on.I see absolutely no advantage in changing this into E_DEPRECATED, as it's
even more likely to be hidden or ignored than E_NOTICE.
removing a feature (which is present in the language since 4.0, probably
even previously) without marking it deprecated first would be a rude move
imo.
so if we want to remove it, I can't see any other way.
There are actually two parts to the message: that you used an undefined
constant, and that PHP interpreted this as you wanting a string. If you
really intended a string, then "deprecated" is relevant; but if you
intended a keyword or constant, then there is nothing to deprecate, it's
just an error.
yeah, the problem is that we can't tell, what was the intention from the
author, so while I can see how this behavior can cause huge WTF moments for
our users (http://3v4l.org/QW8qV), but there are still a bunch of code out
there relying on it.
having a php version where it is E_DEPRECATED
allows people to upgrade to
that version, log the deprecated messages and fix their code.
(ofc. you can also try to detect this specific usage with static analysis,
but that would still miss dynamic usages like eval).
If we want to keep the string fallback for now, how about we raise the
severity to E_WARNING, and change the message to mention deprecation?
e.g.
Warning: Use of undefined constant foo - assumed 'foo', but this behaviour
will change in a future version
I don't like mixing the deprecation notice in the string but maybe it's
just me.
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Ferenc Kovacs wrote on 28/01/2015 16:42:
On Wed, Jan 28, 2015 at 4:42 PM, Rowan Collins
<rowan.collins@gmail.com mailto:rowan.collins@gmail.com> wrote:Ferenc Kovacs wrote on 28/01/2015 12:13: `E_DEPRECATED` is likely to be even more ignored than E_NOTICE, so would be a step backwards; my whole assumption is that most people getting the message are not using a deprecated feature, but accidentally mis-typing a constant or keyword. I'd be happy with raising it to `E_WARNING` to make it more noticeable, though, if we don't think removing the functionality is acceptable. turn it into `E_WARNING` if we are planning to keep the feature, turn it into `E_DEPRECATED` if we want to remove the feature later on. I see absolutely no advantage in changing this into E_DEPRECATED, as it's even more likely to be hidden or ignored than E_NOTICE.
removing a feature (which is present in the language since 4.0,
probably even previously) without marking it deprecated first would be
a rude move imo.
so if we want to remove it, I can't see any other way.
The only documentation I can find mentioning it explicitly discourages
is it, so I don't think it's a huge stretch to say that it's already
deprecated, but if we feel that's not enough, we may not want to jump
straight to fatal. However, I don't think changing to E_DEPRECATED
is
the only, or best, way of warning people of future removal.
(ofc. you can also try to detect this specific usage with static
analysis, but that would still miss dynamic usages like eval).
Actually, I was going to mention that it's quite hard to spot with
static analysis, because like classes and functions, you have to chase
includes to know what's defined. Unless you look for bare words which
aren't all caps, I suppose.
If we want to keep the string fallback for now, how about we raise the severity to E_WARNING, and change the message to mention deprecation? e.g. Warning: Use of undefined constant foo - assumed 'foo', but this behaviour will change in a future version
I don't like mixing the deprecation notice in the string but maybe
it's just me.
I know it's unusual, but I think really there are two messages we're
expressing: "Warning: Undefined constant" and "Deprecated: Using
unquoted string". Unless we issue both at once (which seems
unnecessarily confusing), I think the higher of the two severities
should be used. But maybe that's just me, I'll see what others think.
Regards,
Rowan Collins
[IMSoP]
Ferenc Kovacs wrote on 28/01/2015 12:13:
ok, I was assuming that by turning it into
E_RECOVERABLE_ERROR
you
would still keep the assuming string behavior if there is a custom
error handler which returns true for this error.
I'm undecided on whether the string value or null would make more sense,
and listed both possibilities in my original post:
Proposed behaviour: an
E_RECOVERABLE_ERROR
is issued; if the error is
caught, either the existing string behaviour could be retained, or the
value could be treated as NULL. (I'm open to other alternatives.)
Returning null would probably allow the engine code to be simplified,
but returning the string would allow people to write compatibility
layers using a custom error handler.
Regards,
Rowan Collins
[IMSoP]
Hi Rowan,
On Wed, Jan 28, 2015 at 7:21 AM, Rowan Collins rowan.collins@gmail.com
wrote:
What do people think?
I would like to vote +1 for this indeed.
My concern is that there are too many users ignore E_NOTICE. Use of
undefined
constant is fatal error IMHO, though. I will not vote and let people decide.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
On Tue, Jan 27, 2015 at 11:21 PM, Rowan Collins rowan.collins@gmail.com
wrote:
Hi All,
I would like to propose that the error given for an undefined constant
should be raised from E_NOTICE, probably to E_RECOVERABLE_ERROR, in PHP 7.I'm happy to expand this into a proper RFC, and work on patches for the
engine, tests, and spec, but want to see if initial reception to the idea
is favourable first.Current behaviour: any bare word which is not a defined constant is
treated as though it were a quoted string, with anE_NOTICE
issued.Proposed behaviour: an
E_RECOVERABLE_ERROR
is issued; if the error is
caught, either the existing string behaviour could be retained, or the
value could be treated as NULL. (I'm open to other alternatives.)PROS
- Consistency.
An undefined class constant (e.g. Foo::BAR) gives a fatal error, but a
normal global constant gives only a notice. Class constants being "more
modern" made this seem reasonable, but with namespaces, it's even worse - a
qualified namespace constant (e.g. Foo\BAR) gives a fatal error, as does a
global constant in namespace notation (e.g. \BAR); but a namespace constant
referenced relative to the current namespace is a bare word, and gives only
a notice (e.g. namespace Foo; const BAR=42; echo BAAR;).Functions and classes also give fatal errors if used without declaring;
variables do not, but this is more useful, because they can meaningfully be
created as null, and there isn't a syntax to declare a local variable, only
to initialise it.
- Inherent severity.
It's probably relatively common to turn off
E_NOTICE
in error_reporting,
or to miss one notice among many others. But an undefined constant
magically turning into a string could cause relatively major problems.Firstly, a constant is likely to have some value which needs to be used or
matched somewhere else. A READ_ONLY flag mis-typed as REED_ONLY could be
interpreted as int(0) (i.e. intval('REED_ONLY')) and cause a catastrophic
error.Secondly, keywords are also bare words, and any expression can be a
statement. Thus the following is an infinite loop, no matter what
blahblah() returns:
while(true) {
if ( blahblah() ) {
brek;
}
}Obviously, there are other mistakes which could cause such errors, but
this is one that the engine could easily pick up on and protect the user.CONS
Like most changes, the downside is a break in compatibility.
As far as I can make out, the current behaviour is actually for
compatibility back as far as PHP 3 (correct me if I'm wrong, I didn't dig
far).I'm not aware of any notice officially deprecating barewords-as-strings,
but nor can I find many references to it in the manual at all. Its use for
array keys has been explicitly discouraged in the manual since some time in
- [1]
Outside code golf, I'd be very surprised if any code written or updated in
the last 10 years deliberately uses this "feature", and I think changing it
in PHP 7 is justifiable given the benefits to maintainers of even the
simplest non-OO, non-namespaced code.What do people think?
I think this is too big a BC break.
The usage of $array[key] = 0 instead of "key" is widespread.
[1] http://web.archive.org/web/20010614144157/http://www.php.
net/manual/en/language.types.array.php#language.types.array.dontsRegards,
--
Rowan Collins
[IMSoP]
Benjamin Eberlei wrote on 28/01/2015 08:43:
I think this is too big a BC break.
The usage of $array[key] = 0 instead of "key" is widespread.
As I mentioned, the advice not to write that has been in the manual
since 2001, so I can't think why any tutorial or example code would
teach it as the right way, but it is possible, I suppose.
I realise the majority of code is not available publicly, but it would
be good to make some attempt to judge whether it really is widespread,
so if you have any examples or way of measuring it, that would be useful.
Regards,
Rowan Collins
[IMSoP]
HI,
Benjamin Eberlei wrote on 28/01/2015 08:43:
I think this is too big a BC break.
The usage of $array[key] = 0 instead of "key" is widespread.
As I mentioned, the advice not to write that has been in the manual since
2001, so I can't think why any tutorial or example code would teach it as
the right way, but it is possible, I suppose.I realise the majority of code is not available publicly, but it would be
good to make some attempt to judge whether it really is widespread, so if
you have any examples or way of measuring it, that would be useful.
From what I've seen, most developers seem to put everything in quotes
- strings (obviously), integers, floats, even numeric array keys ...
boolean and null being the only exceptions (but I've seen that too).
No opinion on the subject, just my observations.
Cheers,
Andrey.