On 14/12/2014 18:35, Andrea Faulds wrote:
Good evening,Unfortunately, zend_parse_parameters and userland type hint error messages use outdated type names, and don’t even do so consistently. For example: $ php -r 'fread(0, 0);' PHP Warning: fread() expects parameter 1 to be resource, integer given in Command line code on line 1 $ php -r 'fread(fopen("data:text/plain,test", "r"), fopen("data:text/plain,test", "r"));' PHP Warning: fread() expects parameter 2 to be long, resource given in Command line code on line 1 $ php -r 'function foo(foobar $x) {} foo(1.0);' PHP Catchable fatal error: Argument 1 passed to foo() must be an instance of foobar, double given, called in Command line code on line 1 and defined in Command line code on line 1 Specifically, zend_parse_parameters will always “expect” a “long”, yet when the expected type isn’t an integer and an integer is passed, it says “integer given”. Alongside this, both userland type hint errors and zend_parse_parameters errors refer to “double” and not “float”. I want to change the type names to be consistent, because I think our current inconsistency is confusing. Integers are sometimes ints or integers, but other times longs. Floats are sometimes floats, but other times doubles. If scalar type hints are ever added, then jettisoning the old aliases means we don’t have to add extra reserved words. I’ve made a patch which makes zend_parse_parameters and userland type hints consistently show “integer” and “float” rather than “long” and “double”: https://github.com/php/php-src/pull/955 I also wrote a GNU sed script which I used to update the tests: https://gist.github.com/TazeTSchnitzel/c0d780466def9f226318 Alongside error messages, there are some other places that could be changed. Obviously the return value of gettype() shouldn’t be changed, but I think that we should, for example, make is_long() an alias of is_int() and not the other way around. Thoughts?
I had a go at this a few months ago, but haven't updated my patch based on what's changed in the engine since, so it probably wouldn't merge cleanly: https://github.com/php/php-src/pull/769
An additional case which it looks like you haven't covered is "Object of class %s could not be converted to double" in zend_operators.c and zend_object_handlers.c: https://github.com/IMSoP/php-src/commit/8dd601ac39a4241c9f119fd2d3e0e46474980a53
Other than the awkward BC implications in gettype(), I see no reason that the user should ever see the types labelled as "long" or "double" in their output.
However, note that zend_get_type_by_const does get used in a handful of non-error contexts, such as the ReflectionClass output shown in ext/reflection/tests/bug29986.phpt [https://github.com/TazeTSchnitzel/php-src/commit/c0ee87297298b4ff9b8d68a01f8bbb99f94d9a10#diff-241] I had a grep around and couldn't see any that were likely to cause any real concern, though.
Regards, -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
