Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57984
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain sugarcrm.com designates 207.97.245.183 as permitted sender)
Message-ID: <4F469BA8.1080105@sugarcrm.com>
Date: Thu, 23 Feb 2012 12:03:52 -0800
Organization: SugarCRM
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0.1) Gecko/20120208 Thunderbird/10.0.1
MIME-Version: 1.0
To: Matthew Weier O'Phinney <weierophinney@php.net>
CC: "internals@lists.php.net" <internals@lists.php.net>
References: <F7.45.18992.D05964F4@pb1.pair.com>
In-Reply-To: <F7.45.18992.D05964F4@pb1.pair.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] 5.4 and is_callable()
From: smalyshev@sugarcrm.com (Stas Malyshev)

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