Re: com php-src: Faster sorting algo: UPGRADING Zend/Makefile.am Zend/tests/methods-on-non-objects-usort.phpt Zend/zend_API.c Zend/zend_hash.c
Zend/zend_hash.h Zend/zend_ini.c Zend/zend_llist.c Zend/zend_qsort.c Zend/zend_qsort.h Zend/zend_sort.c Zend/zend_sort.h Zend/zend_ts_hash.c
Zend/zend_types.h configure.in ext/ereg/ereg.c ext/intl/collator/collator_sort.c ext/phar/dirstream.c ext/phar/phar_internal.h ext/standard/array.c
ext/standard/info.c ex
| From: | Alexey Zakhlestin | Date: | Mon, 19 Jan 2015 09:13:45 +0000 |
| Subject: | Re: com php-src: Faster sorting algo: UPGRADING Zend/Makefile.am Zend/tests/methods-on-non-objects-usort.phpt Zend/zend_API.c Zend/zend_hash.c Zend/zend_hash.h Zend/zend_ini.c Zend/zend_llist.c Zend/zend_qsort.c Zend/zend_qsort.h Zend/zend_sort.c Zend/zend_sort.h Zend/zend_ts_hash.c Zend/zend_types.h configure.in ext/ereg/ereg.c ext/intl/collator/collator_sort.c ext/phar/dirstream.c ext/phar/phar_internal.h ext/standard/array.c ext/standard/info.c ex |
||
| References: | 1 2 3 4 5 6 7 8 9 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-80779@lists.php.net to get a copy of this message | ||
> On 19 Jan 2015, at 07:52, Rasmus Lerdorf <rasmus@lerdorf.com> wrote:
>
> On 01/18/2015 02:08 PM, Rasmus Lerdorf wrote:
>> We have to be really really careful with this "oh, that code is wrong,
>> so we can break it argument". This will break hundreds if not thousands
>> of sites in a hard-to-debug way. It took me less than a minute of
>> looking on Github to find a case that will break:
>>
>>
>> https://github.com/chenboking/service.downloadmanager.amule/blob/cda510415f9a58660e096a7de42b3ea6f39ee851/webserver/php-default/amuleweb-main-search..php#L121
>>
>> It is extremely common to just do a less-than or a greater-than check in
>> a user comparison function. Of course it isn't textbook-correct, but
>> since it has always worked, people do it.
>
> And just to further illustrate this, I just downloaded the current
> Wordpress release and sure enough, Wordpress would also be broken by
> this change.
>
> In http://core.svn.wordpress.org/trunk/wp-includes/class-simplepie.phpNÖÂ3´„s
> ð ï ÖM
> look at the sort_items() method:
>
> /**
> * Sorting callback for items
> *
> * @access private
> * @param SimplePie $a
> * @param SimplePie $b
> * @return boolean
> */
> public static function sort_items($a, $b)
> {
> return $a->get_date('U') <= $b->get_date('U');
> }
>
> I am not saying we should revert the change, but we need to be very
> aware of the effect this change will have on PHP7 adoption. The really
> nasty part is that even if a big codebase has unit tests covering the
> code, unless the unit test actually tests an array with more than 16
> elements, all tests will pass and the application will only fail in
> production with production data. And there are no errors or warnings of
> any sort either that will help people track this down.
>
> This will need to be front and center in the UPGRADING doc with an
> explanation about how to write a proper ordering function.
Should we add E_STRICT for cases when comparison-function returns non-integer value?
Having it in stable version should help to fix such anti-patterns in a long-run
--
Alexey Zakhlestin
CTO at Grids.by/you
https://github.com/indeyets
PGP key: http://indeyets.ru/alexey.zakhlestin.pgp.asc
Attachment: [application/pgp-signature] Message signed with OpenPGP using GPGMail signature.asc
Attachment: [application/pgp-signature] Message signed with OpenPGP using GPGMail signature.asc