Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:95394
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.194 as permitted sender)
MIME-Version: 1.0
Sender: morrison.levi@gmail.com
In-Reply-To: <031a2bf1-1996-1144-2b85-e10a40be9514@gmail.com>
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>
Date: Mon, 22 Aug 2016 16:25:57 -0600
Message-ID: <CAFMT4Nrc7nXx4Q0qZzwUSw2LDsFwe9i9aVNTYOgjeFdhWjEtgg@mail.gmail.com>
To: Rowan Collins <rowan.collins@gmail.com>
Cc: "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 parameter1
 to be object, string given
From: levim@php.net (Levi Morrison)

On Mon, Aug 22, 2016 at 3:40 PM, Rowan Collins <rowan.collins@gmail.com> wr=
ote:
>
> On 22/08/2016 20:57, Levi Morrison wrote:
>>
>> I think the ideal but difficult path forward is to find where it was int=
roduced and pray the commit log or something else will indicate the behavio=
r.
>
>
> OK, I'll bite.
>
> Christoph already linked to this comment in the source [https://github.co=
m/php/php-src/blob/PHP-7.0.10/ext/reflection/php_reflection.c#L3197-L3202]:
>
>> /* 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 ignore =
the
>>      * first parameter.
>>      *
>>      * Else, we verify that the given object is an instance of the class=
.
>>      */
>
>
> A simple blame takes that comment back effectively unchanged to Nov 2005,=
 when reflection was first moved to "ext/reflection":
> https://github.com/php/php-src/blob/7cb0480d04933e3d27b75edf29822815a6108=
894/ext/reflection/php_reflection.c#L2163
>
> Before that, it was in zend_reflection_api.c, and blames back to the rath=
er general "more of Timm's implementation" committed by George Schlossnagle=
 in July 2003: https://github.com/php/php-src/commit/84f5e4870e13f76a6223a0=
a937809092ae70d543#diff-cf9733a6fe0eeed1f5a44b59667967baR984
>
> The inconsistency then comes in when invokeArgs is added by Marcus Boerge=
r over a year later: https://github.com/php/php-src/commit/41b87ab486c26f9b=
2d1bc315988b8e8271b6e06b
>
> The new methods made use of the (presumably new?) zend_parse_parameters s=
ystem, and specified the first argument as a mandatory object, which was fi=
xed a few days later to have an optional object, and a separate check when =
it is required: https://github.com/php/php-src/commit/63b288c4646d405d0edfb=
7657505b2acf5643514
>
> Notably, the same comment completely ignoring the first parameter was pre=
sent in that first implementation of invokeArgs.
>
> So, it's pretty clear to me that there was no intention for the two to be=
 different, just different contributors at different times. It's also prett=
y clear that the only thought put into the first argument with static metho=
ds is "ignore it".
>
> None of which really answers what the behaviour should be, in my opinion.=
 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 argument gi=
ve?
> - 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 just b=
e an oversight.
>
> Regards,
>
> --
> Rowan Collins
> [IMSoP]
>
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php


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.