Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:57996
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.83.42 as permitted sender)
Received-SPF: pass (google.com: domain of julienpauli@gmail.com designates 10.213.6.209 as permitted sender) client-ip=10.213.6.209;
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <4F469BA8.1080105@sugarcrm.com>
References: <F7.45.18992.D05964F4@pb1.pair.com> <4F469BA8.1080105@sugarcrm.com>
Date: Fri, 24 Feb 2012 10:48:39 +0100
Message-ID: <CAMUwpuTqvomSc-J2WtFj1YSoXBgzdt8FXEAH262C_0kmDMJAgA@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: "Matthew Weier O'Phinney" <weierophinney@php.net>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=0015174bde98af3ed404b9b2ac1c
Subject: Re: [PHP-DEV] 5.4 and is_callable()
From: jpauli@php.net (jpauli)

--0015174bde98af3ed404b9b2ac1c
Content-Type: text/plain; charset=ISO-8859-1

On Thu, Feb 23, 2012 at 9:03 PM, Stas Malyshev <smalyshev@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

--0015174bde98af3ed404b9b2ac1c--