Hi!

On Wed, Jul 23, 2014 at 4:26 PM, Remi Collet <r...@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

Reply via email to