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

Hi! On Wed, Jul 23, 2014 at 4:26 PM, Remi Collet <Remi@famillecollet.com> wrote: > Notice, > > All Internal classes, implementing Countable still doesn't accept this > optional parameter. > > Ex: > > Method [ <internal:SPL, prototype Countable> public method count ] > { > > - Parameters [0] { > } > > Indeed, all Spl classes still use zend_parse_parameters_none(). > This work, because count($a) doesn't use the method, but the > count_elements handler. > Actually, the count_elements handler doesn't actually use the mode, so that's another inconsistency; fixing that will cause a binary incompatibility, though. Are we "allowed" to fix that so close to a release? Unless the current changes are reverted, at the very least all SPL classes that implement Countable must support the optional argument for ::count(). > If a internal class doesn't implement the handler, it will be hit by > this change. > > Both solution "should" fix this: > - adding the count_elements handler > - accepting the optional mode > Not sure whether you meant to say that any of those solutions work, because either only the latter is implemented or both; i.e. you can't implement count_elements handler and then not accept the optional mode :) > > But, Reflection still not correct, and count($a) still raise the > > Warning: Imagick::count() expects exactly 0 parameters, 1 given in > /work/GIT/imagick/tests/021-countable.phpt on line 19 > > I think this is not consistent (internal vs userland) > Agreed! > > > Remi. > > > P.S. discovered on imagick > See: https://github.com/mkoppanen/imagick/pull/35 > > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > -- -- Tjerk