On 23.08.2016 at 00:25, Levi Morrison wrote:

> On Mon, Aug 22, 2016 at 3:40 PM, Rowan Collins <rowan.coll...@gmail.com> 
> wrote:
>>
>> Christoph already linked to this comment in the source 
>> [https://github.com/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/7cb0480d04933e3d27b75edf29822815a6108894/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 Schlossnagle in 
>> July 2003: 
>> https://github.com/php/php-src/commit/84f5e4870e13f76a6223a0a937809092ae70d543#diff-cf9733a6fe0eeed1f5a44b59667967baR984
>>
>> The inconsistency then comes in when invokeArgs is added by Marcus Boerger 
>> over a year later: 
>> https://github.com/php/php-src/commit/41b87ab486c26f9b2d1bc315988b8e8271b6e06b
>>
>> The new methods made use of the (presumably new?) zend_parse_parameters 
>> system, and specified the first argument as a mandatory object, which was 
>> fixed a few days later to have an optional object, and a separate check when 
>> it is required: 
>> https://github.com/php/php-src/commit/63b288c4646d405d0edfb7657505b2acf5643514
>>
>> 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 to be 
>> different, just different contributors at different times. It's also pretty 
>> clear that the only thought put into the first argument with static methods 
>> is "ignore it".

ACK

>> 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 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 just 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.  Thanks
Levi!  I suggest to wait for Julian, though, who wrote: "I'll prepare a
patch exposing my ideas soon."

-- 
Christoph M. Becker


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to