5.4 and is_callable()

On Thu, Feb 23, 2012 at 9:03 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:

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

--

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