php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #61467 New "callable" typehint does not work (autoloading)
Submitted: 2012-03-21 20:22 UTC Modified: 2012-03-27 16:58 UTC
Votes:6
Avg. Score:5.0 ± 0.0
Reproduced:2 of 2 (100.0%)
Same Version:2 (100.0%)
Same OS:1 (50.0%)
From: david at grudl dot com Assigned:
Status: Not a bug Package: Class/Object related
PHP Version: 5.4.0 OS:
Private report: No CVE-ID: None
 [2012-03-21 20:22 UTC] david at grudl dot com
Description:
------------
Is really new type hint callable implemented? I see no difference between PHP 5.3 and PHP 5.4, both versions only throw catchable fatal errors.

(I think this unexpected behaviour is due to the fact that class "A" do not exists. In this case the error message is confusing. But the callable should not trigger autoload, it should behave like is_callable($arg, TRUE) and just check the syntax. Otherwise typehint callable will cause major performance issues.)


Test script:
---------------
function test(callable $a)
{
}

test(array('A', 'b')); 
// Catchable fatal error: Argument 1 passed to test() must be an instance of callable, array given

test('A::b'); 
// Catchable fatal error: Argument 1 passed to test() must be an instance of callable, string given


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-03-21 20:25 UTC] david at grudl dot com
-Summary: New "callable" typehint do not work (autoloading) +Summary: New "callable" typehint does not work (autoloading)
 [2012-03-21 20:25 UTC] david at grudl dot com
do -> does
 [2012-03-21 20:49 UTC] rasmus@php.net
-Status: Open +Status: Feedback
 [2012-03-21 20:49 UTC] rasmus@php.net
Are you sure you are running 5.4? Your your test script:

% php53 test.php

Catchable fatal error: Argument 1 passed to test() must be an instance of 
callable, array given, called in /home/rasmus/r on line 6 and defined in 
/home/rasmus/r on line 2

% php54 test.php

Catchable fatal error: Argument 1 passed to test() must be callable, array 
given, called in /home/rasmus/r on line 6 and defined in /home/rasmus/r on line 
2
 [2012-03-21 21:58 UTC] david at grudl dot com
Sorry, in PHP 5.4 there is not "an instance of" in error message. But that's not the point.
 [2012-03-21 23:57 UTC] 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.
 [2012-03-22 06:47 UTC] vrana@php.net
-Status: Feedback +Status: Open
 [2012-03-23 05:18 UTC] 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-23 05:18 UTC] rasmus@php.net
-Status: Open +Status: Feedback
 [2012-03-23 12:55 UTC] david at grudl dot com
-Status: Feedback +Status: Open
 [2012-03-23 12:55 UTC] 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 13:35 UTC] 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 13:35 UTC] rasmus@php.net
-Status: Open +Status: Not a bug
 [2012-03-23 19:34 UTC] 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 23:47 UTC] rasmus@php.net
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.
 [2012-03-24 23:41 UTC] jan at skrasek dot com
There is no logic to check is_callable by typehint - these typehint is useless, brings no new functionality. The only one possibile benefit is just check validity of the callback structure, not the callabality self.

> A callable isn't a basic type like a string.
So, this typehint behaves differently than others. Yeah, so you prefer to mix things together and make php much more chaotic.
> This is what most users expect when they check if something is callable.
Some research available? Or just empty words?

From my point of view php is heading to hell by making these type of shortcuts. Yes, we are lazy programmers, but there is no need to rewrite if (is_callable(..)) as a typehint.
 [2012-03-27 16:58 UTC] david at grudl dot com
1) "Yes, something other than a string was expected." Really? What exactly is expected? When a string points to something that _is_ callable, then it is not a string? 

call_user_func('xxx') triggers error 'expects parameter 1 to be a valid callback, function 'xxx' not found or invalid function name' which is perfectly understandable. Why the same function with typehint callable will not trigger the same error message? It will trigger 'Argument 1 passed to call_user_funct() must be callable, string given', which says that I shouldn't use the string. And that's not simply true.

2) I would totally agree with you, if PHP was static language. But PHP is dynamic language and it makes sense to have stored in the variable name of class/function that has not yet been loaded. And most importantly: the life cycle of PHP is a single HTTP request, lazy loading is very important for good performance.
 [2012-07-16 22:33 UTC] paladin at jstation dot cz
Hmm... I agree with David Grudl that current behavior is not ideal... so what 
about two typehints? callback and callable?

function testA(callback $a)
{
//callback checks only syntax
}

function testB(callable $a)
{
//callable checks if it is really callable
}
 [2012-12-07 11:19 UTC] honza at mujserver dot net
I agree with paladin, but I don't think that *callable* is useful.
 [2012-12-07 19:18 UTC] nikic@php.net
@paladin / @honza: We certainly won't introduce another type hint just to cover some minority use case. Type hints are there to simplify the most common type checks and callability-without-actual-callability is certainly not one of the common cases. If you have special needs you can always just do the manual is_callable call.
 [2013-03-09 20:20 UTC] sorin dot badea91 at gmail dot com
@nikic I don't think that the use case presented here a minority one. Callbacks 
are often send as arguments. The current typehint functionality is completely 
useless.
 [2015-08-03 11:01 UTC] hijarian at gmail dot com
Here's the use case.

I have read the http://php.net/manual/en/language.types.callable.php and know several syntax variants of specifying callable instances. Among them the array and string ones.

I want to make higher-order function definition accepting a callable instance.
To both enforce the argument type and provide a type hint for other developers I make the following definition:

private function wrapper(callable $procedure) {
  // pre-actions
  $procedure();
  // post-actions
}

Next developer creates new method in the same class, and inside it wraps some other method of the same class:

$this->wrapper([$this, 'someOtherMethod']);

PHP responds with parse error described by OP, and that developer is forced to use anonymous function instead.

$this->wrapper(function () { $this->someOtherMethod(); });

This can be considered a minor annoyance, but it's explicitly stated in the documentation that we can specify callable instances by arrays, and [$this, 'someOtherMethod'] *is* callable, because, let's say, 'someOtherMethod' actually exists in this class.

All your previous discussion is meaningless, because the core of the problem is this: docs say that you can specify callables as array (and you actually can) but `callable` type hint DOES NOT ACCEPT ARRAYS.

Honestly, I don't care whether `callable` type hint checks actual callability or not, but I as a programer assume that `callable` must accept anything `call_user_func` accepts, with same behavior. Fatal error is completely unjustified here.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Apr 18 16:01:29 2024 UTC