I discussed this with Rasmus and Derick, yesterday, but I don't think we
came to a conclusion..
Is this a bug, or intended behaviour?
sean@iconoclast:~$ /opt/src/php-5.0.4/sapi/cli/php -r 'echo date("r",
"1132003418 ") ."\n";'
Mon, 14 Nov 2005 16:23:38 -0500
sean@iconoclast:~$ /opt/src/php5-200511131530/sapi/cli/php -r 'echo
date("r", "1132003418 ") ."\n";'
Warning: date() expects parameter 2 to be long, string given in Command
line code on line 1
sean@iconoclast:~$
The conclusion SEEMED to be that it's a bug in the parameter parsing API
(as date() now uses the new API).
I think that the 2nd param should automatically be cast to an int. Yes,
I realize that the documentation says it should be an int, and I'm ok
with that (the extraneous whitespace is my fault). I think it should be
mentioned in the release docs if it won't be fixed, though.
Also, a warning is pretty harsh. A E_NOTICE is more representative of
what's happening. Also, in the E_WARNING scenario above, date() no
longer returns the expected result (based on <=5.0 code).
So: is this a bug? If so, I can send a report -- I just didn't want it
to get bogussed without discussion.
If not -- I really think this should be fixed before 5.1.. I realize
it's very late in the game, though.
Opinions?
S
If you pass bad data to a function, it should not warn you?
I'd rather have it as a FATAL error. :)
Nothing to fix here, move along. (and fix your code..)
--Jani
I discussed this with Rasmus and Derick, yesterday, but I don't think we
came to a conclusion..Is this a bug, or intended behaviour?
sean@iconoclast:~$ /opt/src/php-5.0.4/sapi/cli/php -r 'echo date("r",
"1132003418 ") ."\n";'
Mon, 14 Nov 2005 16:23:38 -0500
sean@iconoclast:~$ /opt/src/php5-200511131530/sapi/cli/php -r 'echo
date("r", "1132003418 ") ."\n";'Warning:
date()expects parameter 2 to be long, string given in Command
line code on line 1sean@iconoclast:~$
The conclusion SEEMED to be that it's a bug in the parameter parsing API
(asdate()now uses the new API).I think that the 2nd param should automatically be cast to an int. Yes,
I realize that the documentation says it should be an int, and I'm ok
with that (the extraneous whitespace is my fault). I think it should be
mentioned in the release docs if it won't be fixed, though.Also, a warning is pretty harsh. A E_NOTICE is more representative of
what's happening. Also, in theE_WARNINGscenario above,date()no
longer returns the expected result (based on <=5.0 code).So: is this a bug? If so, I can send a report -- I just didn't want it
to get bogussed without discussion.If not -- I really think this should be fixed before 5.1.. I realize
it's very late in the game, though.Opinions?
S
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:
If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)
PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).
--Pierre
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).
The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old
zend_get_parameters() following by a convert_to_long() says Yes. The
newer zend_parse_parameters() says no.
-Rasmus
On Tue, 15 Nov 2005 09:17:20 -0800
rasmus@lerdorf.com (Rasmus Lerdorf) wrote:
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as
string there (for example, imagecreate("100", "100"); works).The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old
zend_get_parameters() following by a convert_to_long() says Yes. The
newer zend_parse_parameters() says no.
My answer was to Jani's.
--Pierre
Pierre wrote:
On Tue, 15 Nov 2005 09:17:20 -0800
rasmus@lerdorf.com (Rasmus Lerdorf) wrote:Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as
string there (for example, imagecreate("100", "100"); works).
The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old
zend_get_parameters() following by a convert_to_long() says Yes. The
newer zend_parse_parameters() says no.My answer was to Jani's.
I realize that, but Jani said "bad data" not data of the wrong type.
Your example didn't have any bad data. You just had "100" which is a
perfectly valid numeric string and will work in all cases.
-Rasmus
On Tue, 15 Nov 2005 09:29:29 -0800
rasmus@lerdorf.com (Rasmus Lerdorf) wrote:
my answer was to Jani's.
I realize that, but Jani said "bad data" not data of the wrong type.
Your example didn't have any bad data. You just had "100" which is a
perfectly valid numeric string and will work in all cases.
The point is still valid. I believe there is will be enough changes in
5.1 in the date area without this one.
I do not consider trailing white chars as invalid (especially not
spaces). It is common to pass those values directly from a database
results, results can sometimes use fixed length strings.
--Pierre
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old zend_get_parameters()
following by a convert_to_long() says Yes. The newer zend_parse_parameters()
says no.
With new version it's possible to catch such typos, with old one they'd
just be silently ignored and perhaps could cause very hard to find bugs in your code..
--Jani
Jani Taskinen wrote:
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old
zend_get_parameters() following by a convert_to_long() says Yes. The
newer zend_parse_parameters() says no.With new version it's possible to catch such typos, with old one they'd just be silently ignored and perhaps could cause very hard to findbugs in your code..
I suppose, but I still find it weird that:
date("h", (int)$foo);
and
date("h", $foo);
will behave differently when $foo isn't a clean numeric string. And
when we move 100% to zend_parse_parameters() in PHP 6 there will be many
more functions changing their behaviour due to this.
-Rasmus
Jani Taskinen wrote:
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old zend_get_parameters()
following by a convert_to_long() says Yes. The newer
zend_parse_parameters() says no.With new version it's possible to catch such typos, with old one they'd just be silently ignored and perhaps could cause very hard to find bugsin your code..
I suppose, but I still find it weird that:
date("h", (int)$foo);
and
date("h", $foo);
will behave differently when $foo isn't a clean numeric string. And when we
move 100% to zend_parse_parameters() in PHP 6 there will be many more
functions changing their behaviour due to this.
I bet this isn't only thing that's changing. But you really can't get an
omelette without breaking some eggs, can you? :)
--Jani
I can't think of any case where you'd want to error out when given
'100 ' if it would accept '100' quite happily.
I'd lean towards a single cast semantic for this, and remove that
strict checking flag from zend_parse_parameters(); lazy dynamic type
handling for primitive data types is one of the cornerstones of PHP
IMO.
--Wez.
Jani Taskinen wrote:
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).The question isn't what to do with "100","100" but what to do with
"100abc","100abc". Should that still work? The old
zend_get_parameters() following by a convert_to_long() says Yes. The
newer zend_parse_parameters() says no.With new version it's possible to catch such typos, with old one they'd just be silently ignored and perhaps could cause very hard to findbugs in your code..
I suppose, but I still find it weird that:
date("h", (int)$foo);
and
date("h", $foo);
will behave differently when $foo isn't a clean numeric string. And
when we move 100% to zend_parse_parameters() in PHP 6 there will be many
more functions changing their behaviour due to this.-Rasmus
Wez Furlong wrote:
I can't think of any case where you'd want to error out when given
'100 ' if it would accept '100' quite happily.I'd lean towards a single cast semantic for this, and remove that
strict checking flag from zend_parse_parameters(); lazy dynamic type
handling for primitive data types is one of the cornerstones of PHP
IMO.
Yeah, I am leaning towards that as well. I don't like the inconsistency
between direct casting via (int) vs. zend_parse_parameter's strict
casting. It is much simpler to explain that functions will cast to the
required parameter type and have one consistent way to cast things.
-Rasmus
Rasmus Lerdorf wrote:
Wez Furlong wrote:
I can't think of any case where you'd want to error out when given
'100 ' if it would accept '100' quite happily.I'd lean towards a single cast semantic for this, and remove that
strict checking flag from zend_parse_parameters(); lazy dynamic type
handling for primitive data types is one of the cornerstones of PHP
IMO.Yeah, I am leaning towards that as well. I don't like the inconsistency
between direct casting via (int) vs. zend_parse_parameter's strict
casting. It is much simpler to explain that functions will cast to the
required parameter type and have one consistent way to cast things.
+1
this is really what i have come to expect from PHP
(which is why I never understood why we changed array_merge to not cast
null to an empty array back in PHP 5.0.0)
regards,
Lukas
Perhaps, but I would maintain that passing "123abc" and having it
interpreted as 123 is still wrong.
-Andrei
I can't think of any case where you'd want to error out when given
'100 ' if it would accept '100' quite happily.I'd lean towards a single cast semantic for this, and remove that
strict checking flag from zend_parse_parameters(); lazy dynamic type
handling for primitive data types is one of the cornerstones of PHP
IMO.--Wez.
Perhaps, but I would maintain that passing "123abc" and having it interpreted
as 123 is still wrong.
Yeah, I lean that way too, although trailing whitespace should be
supported IMO.
regards,
Derick
--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it interpreted
as 123 is still wrong.Yeah, I lean that way too, although trailing whitespace should be
supported IMO.
I don't like having two different ways to cast things and I think we
would break a lot of stuff if (int)"123abc" no longer resulted in 123.
-Rasmus
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it
interpreted
as 123 is still wrong.Yeah, I lean that way too, although trailing whitespace should be supported
IMO.I don't like having two different ways to cast things and I think we would
break a lot of stuff if (int)"123abc" no longer resulted in 123.
Sure, I'm not suggesting to change the cast. But it's IMO good to treat
anything except trailing zeroes as "broken" for these kinds of things in
the parameter parsing API.
Derick
--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org
Derick Rethans wrote:
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it
interpreted
as 123 is still wrong.
Yeah, I lean that way too, although trailing whitespace should be supported
IMO.
I don't like having two different ways to cast things and I think we would
break a lot of stuff if (int)"123abc" no longer resulted in 123.Sure, I'm not suggesting to change the cast. But it's IMO good to treat
anything except trailing zeroes as "broken" for these kinds of things in
the parameter parsing API.
That seems rather arbitrary to me. So trailing spaces or trailing
whitespace? Tabs ok? Carriage returns too? If not, why not? Why not
just have a single consistent way to cast things?
-Rasmus
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it
interpreted
as 123 is still wrong.Yeah, I lean that way too, although trailing whitespace should be supported
IMO.I don't like having two different ways to cast things and I think we would
break a lot of stuff if (int)"123abc" no longer resulted in 123.
What has casting and passing a parameter have to do with each other anyway?
IMO, they're totally different things.
--Jani
Jani Taskinen wrote:
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it
interpreted
as 123 is still wrong.Yeah, I lean that way too, although trailing whitespace should be
supported IMO.I don't like having two different ways to cast things and I think we
would break a lot of stuff if (int)"123abc" no longer resulted in 123.What has casting and passing a parameter have to do with each otheranyway?
IMO, they're totally different things.
How is it not related? A function defined to take an int is passed
"123" and it magically works. Or a function defined to take a string is
passed 123 and that also magically works. How do you describe that
other than through casting? Do you really want to document a different
set of "function parameter type conversion rules" that have nothing to
do with, and are completely different from casting?
-Rasmus
Rasmus Lerdorf wrote:
Derick Rethans wrote:
Perhaps, but I would maintain that passing "123abc" and having it
interpreted
as 123 is still wrong.Yeah, I lean that way too, although trailing whitespace should be
supported IMO.I don't like having two different ways to cast things and I think we
would break a lot of stuff if (int)"123abc" no longer resulted in 123.
neither do I - and I garantee it will make PHP less attractive as a tool especially
to the legions of 'noobs' that account for the popularity of php. (I throw the
same argument at the changes made to array_merge() with regard to no longer accepting
non-array values)
I would suggest that psychologically any BC breakage and inconsitency introduced
in the short to medium term hits twice as hard because of the reference related
fixes/breaks/changes (which in itself is already a big hurdle for lots of developers
and ISPs) - my point being that for the sake of the future wellbeing of the community
someone should be stepping on the "Purism Brake" a little harder.
the pragmatic ideal embodied in php since its conception is being somewhat eroded imho,
and I don't see the point in creating a 'perfect' tool if only 10 people are capable of
and want to use it.
rgds,
Jochem
-Rasmus
Perhaps, but I would maintain that passing "123abc" and having it
interpreted as 123 is still wrong.
-Andrei
I can't think of any case where you'd want to error out when given
'100 ' if it would accept '100' quite happily.I'd lean towards a single cast semantic for this, and remove that
strict checking flag from zend_parse_parameters(); lazy dynamic type
handling for primitive data types is one of the cornerstones of PHP
IMO.--Wez.
Pierre wrote:
On Tue, 15 Nov 2005 18:59:32 +0200 (EET)
sniper@iki.fi (Jani Taskinen) wrote:If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)PHP is losely typed, I see nothing wrong to pass an integer as string
there (for example, imagecreate("100", "100"); works).
FWIW, I don't mind forcing an INT here (or an all-numeric string) -- I
know my code was wrong; I admitted this in the first mail.
We need to warn users about it, if we do, though.
I'm only bringing this up because the behaviour changed from 5.0
S
Jani Taskinen wrote:
If you pass bad data to a function, it should not warn you? I'd rather have it as a FATAL error. :) Nothing to fix here, move along. (and fix your code..)
The issue here is we effectively have two different casting mechanisms.
One of the things we need to do when moving to PHP 6 is to make sure
everything is using zend_parse_parameters() to handle function
parameters. Most people would assume that a function that wasn't using
zend_parse_parameters() before and was defined to take a long and thus
passed the parameter through convert_to_long() wouldn't change its
behaviour when moved to zend_parse_parameters() with an "l".
That is:
zval **foo;
long lfoo;
zend_get_parameters_ex(1, &foo)
convert_to_long_ex(foo);
lfoo = Z_LVAL_PP(foo);
is not equivalent to:
long foo;
zend_parse_parameters(1, "l", &foo);
In order to not change the behaviour of the function moving to
zend_parse_parameters() from zend_get_parameters() you have to use a "z"
and do the convert_to_long yourself.
The reason for this is that is_numeric_string() has a flag called
allow_errors which specifies whether or not it should be strict. When
you cast from PHP or use one of the conversion functions internally this
flag is off. When calling zend_parse_parameters it is on.
I am not sure what the right answer here is. I can see the argument for
being strict on parameter types, but at the same time, we will
potentially be breaking a lot of existing code. And at a higher level I
don't like the concept of having two different casting modes.
-Rasmus