Hi Andrea,
After carefully reading the RFC again, and despite the fact that I globally
agree, here are some suggestions and comments that were not discussed yet
(sorry for this huge post) :
- The 'resource' hint does not exist 'as this would prevent moving from
resources to objects for existing extensions, which some have already done
(e.g. GMP)'. This is strange as moving an extension from resource to OO and
keeping the same API is not common. Actually, such a move is quite useless if
the API remains the same (it is just a little slower and, look, it's 'OO'). GMP
is a very special case and, even in this case, PHP 7 could be the occasion to
replace GMP functions with methods. Anyway, this special case does not justify,
IMO, not to define a type hinting for resources (this would be the only
undefined one).
- I suggest you add a catch-all type ('mixed', for example), as this is very
different from not setting the type, especially for static analysis. It would
explicitly define that the element can be any type.
- A 'null' type would be useful for return type. Once again, the reason is that
a function returning 'null' is not the same as a function not defining its
return type. This is DbC but, anyway, as soon as it checks the return type,
your RFC has a foot in DbC.
- More generally, beyond the proposed nullable hint (whose syntax is limited,
ugly and not intuitive), what do you think of a multi-type syntax, something
like a set of types, separated by '|' (like 'string|array'). Interesting for
'|null' as this is the main use case, but '|bool' in return type is also a
common use case to allow returning false, and many string functions would use
(string|array). May I say that phpdoc already uses such a syntax without being
shot down :)
- Actually, I must admit that I have a problem with a syntax that does not
fully support every usual cases. If a user wants to set explicit types in his
code, in many case, it is just impossible. A simple example : even if I want
to, I cannot write anything more precise than 'str_replace($search, $replace,
$subject, int $count=null)', when I would like to write something like
'str_replace(string|array $search, string|array $replace...)'. And people
reading my code, future compilers, and static analyzers would like me to write
that too ! If we add a 'mixed' type, it is already better because explicit, but
not precise enough for the tools that could use this information. And the case
is very common.
- How do you forbid arrays and resources weak conversion to scalar ? Do you
modify the convert_to_xxx() functions or is it just specific to type hinting ?
In this case, it means that internal and userspace functions won't follow the
same weak rules (an int arg in an internal function will accept an
array/resource/null zval while it will be refused for a userspace argument).
There's an ambiguity here because, when you talk about 'the traditional weak
type checking rules' followed by zend_parse_parameters(), that's not the 'weak
type check' rules you define 10 lines above.
- Another point is that modifying zend_parse_parameters() won't be sufficient
to enforce strict checking (because too many internal functions get bare zvals
and handle type checking and conversion by themselves). For all these reasons,
I think we must extend the ARG_INFO mechanism to allow internal function
developers to set type hinting in a similar way to userspace type hinting (just
optimizing values for performance). The return type hint should be set there
too. Anyway, if we want reflection to provide this information for internal
functions, the only way is to add it to the ARG_INFO information (we could then
provide a way to generate the function prototype from this info). That's a big
work but I don't see another way to be consistent.
- The following weak conversions : (bool -> float), (bool -> string), and
(string -> bool) are not much more natural than (array -> scalar), or (resource
-> int), which are forbidden. While it is natural in a C-like world to consider
any non-null number as true, I don't see why we should allow sending a bool to
a function expecting a float, even in weak mode. I would consider this as a
bug, but there may be use cases I'm not aware of. Note that BC is not an
argument here, as weak mode already modifies the way arguments are handled (by
refusing arrays, resources, and null for scalars), so we are free to restrict
it a little more if it makes sense.
- I didn't see anything saying if you check the return type in weak mode.
- Do you plan a way to define the return type of arguments passed by reference,
or do you assume that input and output types must be the same ? Even if you
assume they are the same, do you also check these types along with the
function's return type ? What about a syntax like
'<input-type>[:<output-type>]' (with an error if arg is not passed by ref) ?
- In 'unaffected PHP functionality', you might change this : ' When the strict
type-checking mode isn't in use (which is the default), function calls behave
identically to previous PHP versions' as it is not exactly the case : setting a
scalar type in front of an argument restricts the possible zval types he will
accept. In one case, IS_ARRAY, IS_RESOURCE and IS_NULL inputs are converted, in
the other, they are rejected when entering the function. So, you cannot say
that weak type checking behaves exactly the same as PHP native type juggling.
- Maybe a way to let the user decide whether he wants errors to be raised as
exceptions or PHP errors (a declare addition/variation?). If we define such an
option, it must go with the declare directive, not as an INI setting. Exception
could contain the required and provided types.
- Please, please, don't move declare() to '<?php strict' ! That's the most
ambiguous syntax I can imagine.
- About the 'numeric' type you would introduce in a future RFC, would you (in
strict mode) allow everything accepted by is_numeric() or is it just a shortcut
for 'int|float' ?
Well, that's all for today. I spent 3 hours writing this from previous notes,
but the subject deserves it.
Of course, most of this is for a future RFC but we can start thinking about it.
I send a copy of this post to Anthony because he probably has an interesting
opinion on the subjects I'm listing here.
Cheers,
François
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
