Bug #61467 [Nab]: New "callable" typehint does not work (autoloading)

From: Date: Fri, 23 Mar 2012 23:47:03 +0000
Subject: Bug #61467 [Nab]: New "callable" typehint does not work (autoloading)
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-168784@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=61467&edit=1

 ID:                 61467
 Updated by:         rasmus@php.net
 Reported by:        david at grudl dot com
 Summary:            New "callable" typehint does not work (autoloading)
 Status:             Not a bug
 Type:               Bug
 Package:            Class/Object related
 PHP Version:        5.4.0
 Block user comment: N
 Private report:     N

 New Comment:

1) Yes, something other than a string was expected. When a string points to 
something that isn't a callable function, then it is just a string, so I 
consider the message correct.

2) Yes, I consider it correct. A callable isn't a basic type like a string. It 
has to have a couple of characteristics, one of which is being actually callable 
which you can't know unless you check. This is what most users expect when they 
check if something is callable. If something just looks like a callable function 
and it isn't, then you end up with an uncatchable fatal when you try to call it, 
so knowing that something looks like it might be a callable function isn't very 
useful to most. They want to write robust code and catch any errors and that can 
only be done if we check for the existence of the callable function which is why 
it is the default.

And your example with array just reinforces this. An array typehint fully checks 
if an array is an array because it is a simple type that has no other 
characteristics. An array can't pass the array typehint and then fatal out when 
you try to use it as an array.


Previous Comments:
------------------------------------------------------------------------
[2012-03-23 19:34:44] david at grudl dot com

Rasmus we're talking about two different things:

1) Error message "Must be callable, string given" means, that something other than string
was expected. Do you agree? I think current message is confusing.

2) The behavior of typehint is subjective, of course. But it would be nice if you could consider, if
it is really correct. We are talking about "type hint" - and checking of validity of a
*type*. 

Type hint "array" means that the argument must be array. It's just about the type.
Validity of array must be checked by function itself. The type hint "callable" I expect
checks if argument is valid callable type (valid syntax), nothing more.

------------------------------------------------------------------------
[2012-03-23 13:35:48] rasmus@php.net

Well, that's a very subjective thing. callable matches the default behaviour of 
is_callable(). In most cases when you pass a callable it is going to get called. 
If you want to implement lazy loading it is simple enough to do an is_callable 
syntax-check only in your method itself. The default had to be set to one or the 
other and we tend to set things to the common use case.

------------------------------------------------------------------------
[2012-03-23 12:55:09] david at grudl dot com

I understand its behavior, but I think it is wrong. Callable shouldn't check the existence of
class, but only check argument is a syntactically correct. Otherwise it makes lazy-loading
impossible. Not every callback is called. 

In addition, the non-existence of class results in confusing error message: "must be callable,
string given". So "A::b" is not callable?

------------------------------------------------------------------------
[2012-03-23 05:18:22] rasmus@php.net

But it only triggers the autoloader when you pass it something that looks exactly 
like A::b(). In this case it will go try to load 'A' in order to see if there is 
a b() method. If you pass it array(1,2,3) it will obviously not trigger the 
autoloader so I think your assertion that this will cause "major performance 
issues" is a bit drastic.

------------------------------------------------------------------------
[2012-03-21 23:57:25] david at grudl dot com

Possible fix is to change in file zend_execute.c on line 645 flag IS_CALLABLE_CHECK_SILENT to
IS_CALLABLE_CHECK_SYNTAX_ONLY.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=61467


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=61467&edit=1


Thread (18 messages)

« previous php.bugs (#168784) next »