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: Date: Mon, 19 Jan 2015 06:55:23 +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 10  Groups: php.internals 
Request: Send a blank email to internals+get-80772@lists.php.net to get a copy of this message
Hey: Rasmus, Thanks for the reporting... I didn't think it is a real reasonable question at the very beginning Anyway, this should be fixed in : https://github.com/php/php-src/commit/020b51b46eceb1a8c2876aac263772ed55ba9a1a thanks On Mon, Jan 19, 2015 at 2:06 PM, Xinchen Hui <laruence@php.net> wrote: > Hey: > > On Mon, Jan 19, 2015 at 12:52 PM, 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 èÐv6¬¬4„ >> ߧNC·ø8http://core.svn.wordpress.org/trunk/wp-includes/class-simplepie.php >> 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. > hmm, maybe there is some thing I can do.. > > that is, we don't use >= in sorting algo, but use > and <= as condition... > > which should avoid break such use cases.. I'will have a try > > thanks >> >> -Rasmus >> > > > > -- > Xinchen Hui > @Laruence > http://www.laruence.com/ -- Xinchen Hui @Laruence http://www.laruence.com/

« previous php.internals (#80772) next »