Re: Re: com php-src: Fix bug #67064 => Countable interface inconsistency

Am 30.7.2014 um 17:08 schrieb Ferenc Kovacs <tyra3l@gmail.com>: > On Wed, Jul 30, 2014 at 4:48 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: >> On Wed, Jul 30, 2014 at 3:54 PM, Matteo Beccati <php@beccati.com> wrote: >>> Hi Bob, >>> >>> On 30/07/2014 14:33, Bob Weinand wrote: >>>> Is this a problem if the interface internally doesn't expect a >>> parameter? >>>> >>>> You're free to expect the parameter or not, where's the issue? >>>> We allow implementations to accept more optional parameters than the >>>> interface specifies, but not less. >>> >>> Yes, precisely. If something is using an Interface it should *not* add >>> unspecified parameters to the method calls, as the implementation might >>> have added some of their own with a different meaning. >>> >>>> So, it seems optimal to me, to make the interface specify no parameters, >>>> then the class implementing this interface is free to accept either no >>>> parameter (= don't care about recursive) or one optional one. >>> >>> So, PHP specifies a Countable interface, and its count() method has no >>> parameters. However, if you check the documentation then you'll find out >>> that in fact it might be sent one. On some occasions. >>> >>>> I'll though happily apply your patch now. >>> >>> I wrote it, but to be honest now I think it's just hiding the problem >>> under the carpet. >>> >>> >>> Cheers >>> -- >>> Matteo Beccati >>> >>> Development & Consulting - http://www.beccati.com/ >>> >> >> for the record here is a code snippet showing the BC break: >> http://3v4l.org/4O9nA >> and as far as I Can understand, even with Matteo's patch, the output would >> be still different(1,1,0,0) >> >> > if we want to introduce it properly, I think it would be better to do it in > a major version, where we can add the $mode to the Countable::count() > signature as a mandatory argument, so if by chance anybody already used an > optional argument to their count() implementation, they will be notified > via the "Declaration of MyCountable::count() must be compatible with that > of Countable::count()" fatal error. > Given that this is a BC break(and a hard-to-catch one) which is not allowed > in a minor release and that > https://bugs.php.net/bug.php?id=67064 is more > of a feature request than an actual bugfix, I would like to ask you to > revert this from the PHP-5.6 branch. > Sorry for not spotting this sooner. :( > > -- > Ferenc Kovács > @Tyr43l - http://tyrael.hu Okay, after lengthy discussions it's now reverted. Bob p.s.: And I didn't make attention and a part of the revert went into the next commit (which is basically totally unrelated...), sorry.