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 05:00:04 +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-80764@lists.php.net to get a copy of this message
Hey Rasmus, > On 19 Jan 2015, at 04: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.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. Related: since we have no Perl-like spaceship operator ($a <=> $b), writing comparison functions is unnecessarily complex in the common case, as you must produce -1, 0, 1 yourself. Could we expose a cmp() or compare() function that calls our internal comparison operator? This would make writing custom sort functions a lot nicer, and quite possibly improve some other kinds of code. That would also mean a future sorting API could unify user sorts and non-user sorts: just make the default callback be cmp(). Usage would be like this: cmp(1, 2); // 1 cmp(1, 1); // 0 cmp(2, 1); // 1 Essentially, exactly like the spaceship in Perl, but a function. Thoughts? -- Andrea Faulds http://ajf.me/

« previous php.internals (#80764) next »