Hi,

> On 14 Feb 2015, at 00:44, F & N Laupretre <nf.laupre...@yahoo.fr> wrote:
> 
> - 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. 

It’s happened more than once before, e.g. GMP.

> 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.

The imperative API may not change, but who’s to say that the objects won’t have 
methods added? This allows you to add a new API with an OOP style without 
breaking backwards-compatibility, and which can be used with the imperative API 
if necessary

> 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.

This could be proposed in a follow-up RFC. It’s deliberately omitted as I think 
it’s a distraction from the core proposal.

> - 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. 

I have a draft RFC in the works for a ‘void’ return type which I’ll probably be 
putting under discussion sometime soon.

> - 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 :)

I’m not entirely convinced about this (it’d probably cause problems with weak 
type hinting), but if a good implementation could be made, I might vote for it.

> - 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.\

Yeah, even with this RFC, I agree that PHP’s type hints have some ways to go.

> - 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).

I’m not sure what you’re talking about there. zend_parse_parameters() (and thus 
internal functions) doesn’t currently accept arrays or resources for scalar 
parameters. In fact, this RFC’s type hints use the same underlying 
implementation as zpp does.

The only difference is the handling of NULL.

> 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.

It is the weak type check rules defined above it . They’re the ones zpp uses, 
bar NULL handling.

> - 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).

This is true, actually. However, I suppose those parameters are technically 
“mixed” or lack type hints, in a sense. It’s no different to what userland 
functions can do.

> 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.

There’s already a mechanism for this, I believe. I don’t think it is widely 
used, however.

> - 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.

No, BC is an argument: the RFC tries to keep weak userland hints as close to 
zpp as possible. The only difference is in NULL handling, and I’d like to keep 
it that way. If the behaviour is to be changed, zpp should be too.

> - I didn't see anything saying if you check the return type in weak mode.

It does check it in weak mode, but the wording might have been a little 
unclear. Sorry about that.

> - 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) ?

Types of pass-by-reference arguments are only checked on the initial function 
call, like for all other type hints in PHP. This is an interesting point that 
you raise, though.

> - 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.

Ah, that’s a mistake on my part. It should say function calls to 
built-in/extension PHP functions (i.e. internal functions). I’ll correct that.

> - 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.

Nikita’s Exceptions in the Engine RFC should replace this E_RECOVERABLE_ERROR 
with exceptions. I don’t think we need to allow choice, though… and PHP already 
lets you convert errors to exceptions if you want.

> - Please, please, don't move declare() to '<?php strict' ! That's the most 
> ambiguous syntax I can imagine.

That’ll be a separate, follow-up RFC. You’ll be free to vote against it.

> - 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’ ?

The idea is just int or float (except in weak mode of course, where it needs to 
accept strings etc.). But that makes the naming possibly misleading. I’d prefer 
“number”, but that’d be a BC issue I expect. But that’s for a different RFC.

> 
> 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.


Thanks for your comments. :)

--
Andrea Faulds
http://ajf.me/





--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to