On Sun, 13 May 2012 04:39:33 +0200, Stas Malyshev smalyshev@sugarcrm.com
wrote:

I know this was discussed a number of times here, but just to bring it
to a conclusion - I intend to apply patch in the bug report - which
removes conversion for strings that do not convert to integers - to 5.4.
If anybody sees anything that breaks because of this please tell. Not
sure what about 5.3 - Johannes, could you please comment?
I've run the tests and the patch does not seem to cause any breakage.

I should point out that "remove[ing] conversion for strings that do not
convert to integers" (let's call it proposition A) is not exactly what the
patch does. In addition to that condition, one other is required:

• The floats a and b the strings convert to are such that a - b == 0.0 (B)

It's implemented as

 if (oflow1 != 0 && oflow1 == oflow2 && dval1 - dval2 == 0.) { ... }

This is irrelevant for == or != comparisons. Let's call C "strings are
equal in the memcmp() sense". If only A is evaluated, then if ~A, the
result of the comparison is the value C, i.e., if strings do not convert
to integers, the result of the comparison is the result of memcmp(). Also
under ~A, if we add B into the mix, we have the following results:

 B  ~B

C 1 can't occur
~C 0 0

But for the comparisons >, <, etc. the result may be surprising. Under ~A
&& ~B && ~C, using only the first condition results in a memcmp()
comparison, while under the two comparisons results in the floats being
compared. See:

$ php -r 'var_dump(strcmp(" 9223372036854775809",
"-9223372036854775808"));'
int(-1) (str1 is less than str2)
$ php -r 'var_dump((float)" 9223372036854775809" <
(float)"-9223372036854775808");'
bool(false)

So the float comparison behavior under ~B (what's in the patch) may seem
more desirable because it preserves the numerical comparison when possible
(and we don't have to add leading whitespace and zeros to the mix,
strcmp("9, "11") returns 1). Until you realize it's alternating between
two behaviors depending on whether B or ~B. So:

"9223372036854775809" < " 09443372036854775809" (true, -- floats differ,
compare as float)
"9223372036854775809" < " 09223372036854775810" (false -- floats are the
same, memcmp)

In both cases (incorporating the test for B or not), there's no escaping a
discontinuity in behavior, unless we revert not to memcmp() but to a
custom string comparison function that strips whitespace and leading
zeros, compares the size of the input and finally calls memcpy().

--
Gustavo Lopes

Too late to raise this now anyway, but ...

I know this was discussed a number of times here, but just to bring it
to a conclusion - I intend to apply patch in the bug report - which
removes conversion for strings that do not convert to integers - to 5.4.
If anybody sees anything that breaks because of this please tell.

"12345678901234567890" == "12345678901234567890.0"

used to be true, is now false ... i'd still say that's ok though as
it is a case of "never compare floats for equality" here, now that
the decimal clearly says that at least the right side is supposed
to be float, not integer ...

--
hartmut