On Thu, Feb 23, 2012 at 9:03 PM, Stas Malyshev <smalys...@sugarcrm.com>wrote:
> Hi!
>
>
> If I create a callback with either of these values:
>>
>> * $callback = 'Foo::bar';
>> * $callback = array('Foo', 'Bar');
>>
>> is_callable() now returns true. In PHP 5.2 and 5.3, it returned false,
>> which is what I'd expect.
>>
>
> I tried the code from the bug and it returned true for me in 5.3:
>
> class Foo
> {
> public function bar()
> {
> return __METHOD__;
> }
> }
> $callback = array('Foo', 'Bar');
> var_dump(is_callable($**callback));
>
> prints true in 5.3.9. Same happens for me with 5.2.
>
> I think if it returned false, it was a bug - this method is certainly
> callable, which one can check by calling it and succeeding.
>
>
> you can call it; it just raises an E_STRICT." That's true. Unless the
>> method actually utilizes $this, in which case you get a fatal: "Using
>> $this when not in object context." And I can think of precisely zero
>> situations where I'd want to call a non-static method statically and
>> expect it to work.
>>
>
> This is a different thing - yes, the method may fail, however is_callable
> is not meant to answer the question "will method work as expected" - it is
> not possible - but only the question "if I called this method as a callback
> - would the engine be able to proceed with the call?". We could of course
> make the engine to disallow static calls to non-static functions - it would
> be quite complex as Foo::Bar not always means static call - but that is not
> within the responsibilities of is_callable. is_callable is supposed to
> dry-run the call code and see if it would fail to resolve, nothing more.
>
> As for situations where static and non-static method would be expected to
> work - unfortunately, I've seen code that calls functions both statically
> and non-statically, and expects it to work. I'm not saying it's a good
> thing but that's what people use. Changing it means changing the engine
> rules and probably will break some code.
>
>
> The point is: if I call is_callable() and it returns true, I should have
>> a reasonable expectation that calling the callback will now work. In
>>
>
> We have different definitions of what "work" means here. The call will
> work. The method itself may not work, but is_callable is certainly never
> would be able to guarantee that certain method will never fail. I
> understand I'm being a bit formalistic here, but that's because I'm trying
> to explain what is_callable is actually does. The problem is not in
> is_callable but in the fact that the engine allows you to call Foo::Bar.
> This call may mean a number of things - static call, parent method call,
> etc.
>
>
> I propose that when a string callback referencing a static method call
>> OR an array callback referencing a static method call refers to a method
>> that is not marked static, is_callable() should return false.
>>
>
> I don't see this happening in 5.4, but in more general way, I don't see
> is_callable departing from what the engine does. To make it worse, there
> are more cases where is_callable returns true but you can not actually call
> it - try making bar abstract method. The problem is that even the success
> of the call is actually runtime-dependent, so what is_callable is
> *actually* saying is "I can see how it could work, given the right
> circumstances", but nobody can guarantee there won't be wrong circumstances
> when it is actually called. I don't think it can be really fixed without
> doing pretty serious changes to the engine and breaking some code.
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Well, I'm +1 with Stas (and his explanation) ; but I'm also +1 to break the
compatibility about that behavior in our next major (PHP6? PHP7?)
We need to patch the engine, as patching is_callable() to handle just some
cases wouldn't be a good solution. As Stas said, is_callable() has to
reflect what the engine thinks (zend_is_callable_ex()).
But to patch it, we need approval from the community, and to wait for the
next major release as our release process prevents us from breaking BC in
minors :)
Cheers,
Julien.P
