Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95403
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.68 as permitted sender)
MIME-Version: 1.0
Sender: julienpauli@gmail.com
In-Reply-To: <eb2a9f37-37ea-54fb-e094-fdb8629d159b@gmx.de>
References: <CAEbg-4ZAMB=Em-nwCp++K3n59btaFSgW-yexhg3cbM3bQ1WjTg@mail.gmail.com>
 <CAMUwpuQe1bS1D9KVRKkqd7-jXw8sx_mWUDrxDjX=i9J5LQSnwg@mail.gmail.com>
 <dd455ef8-d02f-9e7f-94ba-8e276bce2688@gmx.de> <CAFMT4Noh_2BHDUETFtUppJi36m4vuYtmU1A2h_t-Swha1sxKdQ@mail.gmail.com>
 <031a2bf1-1996-1144-2b85-e10a40be9514@gmail.com> <CAFMT4Nrc7nXx4Q0qZzwUSw2LDsFwe9i9aVNTYOgjeFdhWjEtgg@mail.gmail.com>
 <7b81c830-1d05-a4ac-4713-1bb67cbec12c@gmx.de> <CAMUwpuSiLEToYCXzxQnhmtPXi=aO45vAhLWVi-3+U8+TOLYrVg@mail.gmail.com>
 <eb2a9f37-37ea-54fb-e094-fdb8629d159b@gmx.de>
Date: Tue, 23 Aug 2016 14:51:17 +0200
Message-ID: <CAMUwpuSoXwzf_sy7E5KfT2_Zg0oSoWC+fMf4p_xR=CbAmUAF_A@mail.gmail.com>
To: "Christoph M. Becker" <cmbecker69@gmx.de>
Cc: Levi Morrison <levim@php.net>, Rowan Collins <rowan.collins@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] BC break: ReflectionMethod::invoke() expects
 parameter1to be object, string given
From: jpauli@php.net (Julien Pauli)

On Tue, Aug 23, 2016 at 1:51 PM, Christoph M. Becker <cmbecker69@gmx.de> wr=
ote:
> On 23.08.2016 at 13:36, Julien Pauli wrote:
>
>> On Tue, Aug 23, 2016 at 10:30 AM, Christoph M. Becker <cmbecker69@gmx.de=
> wrote:
>>
>>> On 23.08.2016 at 00:25, Levi Morrison wrote:
>>>
>>>> On Mon, Aug 22, 2016 at 3:40 PM, Rowan Collins <rowan.collins@gmail.co=
m> wrote:
>>>>>
>>>>> Christoph already linked to this comment in the source [https://githu=
b.com/php/php-src/blob/PHP-7.0.10/ext/reflection/php_reflection.c#L3197-L32=
02]:
>>>>>
>>>>>> /* In case this is a static method, we should'nt pass an object_ptr
>>>>>>      * (which is used as calling context aka $this). We can thus ign=
ore the
>>>>>>      * first parameter.
>>>>>>      *
>>>>>>      * Else, we verify that the given object is an instance of the c=
lass..
>>>>>>      */
>>>>>
>>>>> A simple blame takes that comment back effectively unchanged to Nov 2=
005, when reflection was first moved to "ext/reflection":
>>>>> https://github.com/php/php-src/blob/7cb0480d04933e3d27b75edf29822815a=
6108894/ext/reflection/php_reflection.c#L2163
>>>>>
>>>>> Before that, it was in zend_reflection_api.c, and blames back to the =
rather general "more of Timm's implementation" committed by George Schlossn=
agle in July 2003: https://github.com/php/php-src/commit/84f5e4870e13f76a62=
23a0a937809092ae70d543#diff-cf9733a6fe0eeed1f5a44b59667967baR984
>>>>>
>>>>> The inconsistency then comes in when invokeArgs is added by Marcus Bo=
erger over a year later: https://github.com/php/php-src/commit/41b87ab486c2=
6f9b2d1bc315988b8e8271b6e06b
>>>>>
>>>>> The new methods made use of the (presumably new?) zend_parse_paramete=
rs system, and specified the first argument as a mandatory object, which wa=
s fixed a few days later to have an optional object, and a separate check w=
hen it is required: https://github.com/php/php-src/commit/63b288c4646d405d0=
edfb7657505b2acf5643514
>>>>>
>>>>> Notably, the same comment completely ignoring the first parameter was=
 present in that first implementation of invokeArgs.
>>>
>>> Thanks for investigating on the history, Rowan!
>>>
>>>>> So, it's pretty clear to me that there was no intention for the two t=
o be different, just different contributors at different times. It's also p=
retty clear that the only thought put into the first argument with static m=
ethods is "ignore it".
>>>
>>> ACK
>>>
>>>>> None of which really answers what the behaviour should be, in my opin=
ion. We still have to decide 3 things:
>>>>>
>>>>> - Is there a compelling reason to change the current behaviour?
>>>>> - What error or behaviour should a string or other non-object argumen=
t give?
>>>>> - What error or behaviour should an object argument give?
>>>>>
>>>>> In my opinion, the best "fix", if something needs to change, would be=
 to reject anything other than null; that anything else works appears to ju=
st be an oversight.
>>>>
>>>> Anyone oppose to emitting E_DEPRECATED for a parameter other than an
>>>> object or null? This opens up the possibility to use it for something
>>>> no earlier than 8.0.
>>>
>>> That appears to be the most reasonable compromise presented yet.  Thank=
s
>>> Levi!  I suggest to wait for Julian, though, who wrote: "I'll prepare a
>>> patch exposing my ideas soon."
>>
>> All right.
>>
>> I was thinking something like this :
>> https://github.com/jpauli/php-src/commit/8735d6b72de0edc3b370245737c0f4f=
204fcbea0
>>
>> But it makes no sense.
>>
>> It makes no sense to pass a class name as first param of invoke() , to
>> target a specific function.
>> Why not create the ReflectionClass instance using that latter so ?
>>
>> I'm OK to deprecate the first arg as a string, then remove such an
>> usage in next major.
>>
>> I can make such a patch if consensus.
>
> I suggest to deprecate all other types than NULL as first arg for static
> methods, because passing an int, for instance, makes even less sense as
> Rowan has already pointed out elsewhere in this thread.


What about passing nothing ?

$reflectionMeth->invoke();


Actually, this is not possible and a Warning:
ReflectionMethod::invoke() expects at least 1 parameter, 0 given in %s
on line %d is thrown.
Should we keep that behavior as well ? Knowing that passing nothing,
the function will get an IS_NULL zval.



Julien