Re: Re: cvs: php-src /ext/standard array.c

On Thu, Feb 19, 2009 at 3:14 PM, Ian Eure <ian@digg.com> wrote: > On Feb 18, 2009, at 6:52 PM, Moriyoshi Koizumi wrote: > >> On Thu, Feb 19, 2009 at 4:51 AM, Andrei Zmievski <andrei@gravitonic.com> >> wrote: >>> >>> Moriyoshi Koizumi wrote: >>>> >>>> As I said earlier, the function is never supposed to be used with >>>> objects. Therefore, we cannot declare it to be broken, and any change >>>> to the behavior anyway leads to a huge BC break. I got a report that >>>> claims the reporter's real-world application behaves strangely with >>>> the latest release candidate. >>> >>> Should we have array_unique_for_non_strings() then or something? >> >> That'd be stupid. What is reasonable is to make it default to >> SORT_STRING... Or is there any strong reason to make the default >> SORT_REGULAR now that you can specify SORT_REGULAR to array_unique() >> throught the second argument? >> > Because it's bad to require an argument to make a function do the right > thing; it should do the right thing by default. As I said, whether it is right or not is pretty much a context-depndent matter. >>>> That said, I'm not really against making SORT_REGULAR default for >>>> later versions than 5.2.x as long as *appropriate notices* are >>>> provided, while I strongly disagree for 5.2.x. >>> >>> What sort of notices do you propose? At runtime or in the docs? >> >> You even didn't mention that the behavior would be changed starting >> with 5.2.9 in the document, instead you simply added the description >> for the second optional argument that defaults to SORT_REGULAR, as if >> it was the default long before. That's absolutely the thing we should >> not do. >> > Why would it be necessary? SORT_REGULAR is a superset of SORT_STRING, yes? > If the function should "only be used with strings" (prior to this patch), as > you claim, that's not a BC break. Unless you're arguing that we should > preserve BC for code calling functions in unsupported ways. If SORT_REGULAR was a superset of SORT_STRING, there would have been no problem in the first place. See the original bug report to see what is to be broken. >> Eitherway, if we were to make such change, I think we should at least >> make the second argument mandatory. >> > Wouldn't this be an even bigger BC break than fixing the existing (broken) > behavior? A little sarcasm, the point here is that it'd be better off making the *incompatible* application not work at all than still allowing them to silently malfunction, but such a decision is just a bit less absurd than changing the default, as you pointed out. Anyway that's basically not a fix IMO. Moriyoshi > > - Ian > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >