On 23.08.2016 at 13:36, Julien Pauli wrote: > On Tue, Aug 23, 2016 at 10:30 AM, Christoph M. Becker <cmbecke...@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.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." > > All right. > > I was thinking something like this : > https://github.com/jpauli/php-src/commit/8735d6b72de0edc3b370245737c0f4f204fcbea0 > > 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. -- Christoph M. Becker -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
