# bug 54547 - Thread: https://externals.io/message/60538 - Original archive: https://news-web.php.net/php.internals/60538 - Started by Stas Malyshev on 2012-05-13T02:39:33+00:00 - Messages: 12 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #60538 — Stas Malyshev — 2012-05-13T02:39:33+00:00 URL: https://externals.io/message/60538#60538 Hi! 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. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #60539 — Gustavo Lopes — 2012-05-13T10:57:44+00:00 URL: https://externals.io/message/60538#60539 · In reply to: #60538 On Sun, 13 May 2012 04:39:33 +0200, Stas Malyshev 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 --- ## #60549 — Stas Malyshev — 2012-05-13T21:39:31+00:00 URL: https://externals.io/message/60538#60549 · In reply to: #60539 Hi! > 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) I understand this may not be ideal but I really see this as very narrow use case - if you really want to compare strings in lexicographical order, you just should use strcmp. So I think this fixes common cases where people have high WTF factor, and if we later have better idea on how to fix all the cases we could amend it further. But I'm not sure adding more magic to the mix (i.e. doing special comparisons, etc.) is going to be better - it will only make it harder for people to understand. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #60840 — Hartmut Holzgraefe — 2012-06-15T00:56:19+00:00 URL: https://externals.io/message/60538#60840 · In reply to: #60538 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 --- ## #60843 — Tjerk Anne Meesters — 2012-06-15T02:17:55+00:00 URL: https://externals.io/message/60538#60843 · In reply to: #60840 On Fri, Jun 15, 2012 at 8:56 AM, Hartmut Holzgraefe wrote: > Too late to raise this now anyway, but ... > > > > […] > > "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 ... I don't think this is a case of "never compare floats for equality", as: 12345678901234567890.0 == 12345678901234567890.1 The number is simply too big, even for a float, to handle reliably. In fact number_format(12345678901234567890.0, 1, '', '') == '123456789012345671680' > > -- > hartmut > > > -- > > > -- -- Tjerk --- ## #60844 — OISHI Kazuo — 2012-06-15T05:42:35+00:00 URL: https://externals.io/message/60538#60844 · In reply to: #60840 > > […] > > "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 ... In addition to == operator, >, <, >=, and <= operators are influenced. And, hexdecimal format for big number is now case-sensitive. http://www.mail-archive.com/internals@lists.php.net/msg58789.html -- OISHI Kazuo --- ## #60928 — Christopher Jones — 2012-06-21T18:15:52+00:00 URL: https://externals.io/message/60538#60928 · In reply to: #60844 > > […] > > In addition to == operator, >, <, >=, and <= operators are influenced. > > And, hexdecimal format for big number is now case-sensitive. > > http://www.mail-archive.com/internals@lists.php.net/msg58789.html > Can you add some phpt tests for all the cases you've raised? Chris -- christopher.jones@oracle.com http://twitter.com/#!/ghrd --- ## #60934 — OISHI Kazuo — 2012-06-22T07:08:58+00:00 URL: https://externals.io/message/60538#60934 · In reply to: #60928 Hi, > > […] > > Can you add some phpt tests for all the cases you've raised? This is the phpt. All tests passed in PHP 5.4.3 but failed in 5.4.4. (I don't know how I can add this phpt into tests.) ========================================================== --TEST-- Bug #62097: New behavior of string == has a compatibility problem (2) --FILE-- "0xFFFFFFFFFFFFFFFF"); var_dump("0xffffffffffffffff" <= "0xFFFFFFFFFFFFFFFF"); --EXPECT-- bool(true) bool(true) bool(true) bool(true) bool(true) bool(true) bool(false) bool(true) ========================================================== -- OISHI Kazuo --- ## #60972 — Christopher Jones — 2012-06-27T00:14:17+00:00 URL: https://externals.io/message/60538#60972 · In reply to: #60934 > Hi, > > > […] > > This is the phpt. All tests passed in PHP 5.4.3 but failed in 5.4.4. > (I don't know how I can add this phpt into tests.) > > ========================================================== > --TEST-- > Bug #62097: New behavior of string == has a compatibility problem (2) > --FILE-- > var_dump("09223372036854775808" == "9223372036854775808"); > var_dump(" 9223372036854775808" == "9223372036854775808"); > var_dump("12345678901234567890.0" == "12345678901234567890"); > var_dump("12345678901234567890e1" == "123456789012345678900"); > var_dump("12345678901234567890e1" == "12345678901234567890E1"); > var_dump("0xffffffffffffffff" == "0xFFFFFFFFFFFFFFFF"); > var_dump("0xffffffffffffffff" > "0xFFFFFFFFFFFFFFFF"); > var_dump("0xffffffffffffffff" <= "0xFFFFFFFFFFFFFFFF"); > > --EXPECT-- > bool(true) > bool(true) > bool(true) > bool(true) > bool(true) > bool(true) > bool(false) > bool(true) > ========================================================== > Does this need an architecture specific SKIPIF? See the mention of `PHP_INT_SIZE` on http://qa.php.net/write-test.php I don't know whether to suggest adding an XFAIL for 5.4, or whether to suggest changing the expected output. This would depend on whether there is likely to be any code change to 5.4 or not. Chris -- christopher.jones@oracle.com http://twitter.com/#!/ghrd --- ## #60973 — OISHI Kazuo — 2012-06-27T04:06:55+00:00 URL: https://externals.io/message/60538#60973 · In reply to: #60972 > Does this need an architecture specific SKIPIF? See the mention of > `PHP_INT_SIZE` on http://qa.php.net/write-test.php Like this? =============================================================== --SKIPIF-- =============================================================== I'm afraid I may miss your point. > > […] Could you tell me your intention of this question? > I don't know whether to suggest adding an XFAIL for 5.4, or whether to > suggest changing the expected output. This would depend on whether > there is likely to be any code change to 5.4 or not. Sorry, I cannot understand what you say, but PHP code changes from 5.4.3 to 5.4.4 for #54547 and #62097 are: https://github.com/php/php-src/commit/9344bf193c6e35c8706923953f3e63bb01cc05ed https://github.com/php/php-src/commit/acd711685a592c52be200e248154283c6c49c9f8 zendi_smart_strcmp() and is_numeric_string[_ex]() are changed. -- OISHI Kazuo --- ## #61006 — Christopher Jones — 2012-06-27T18:26:40+00:00 URL: https://externals.io/message/60538#61006 · In reply to: #60973 > > […] > > Like this? > =============================================================== > --SKIPIF-- > if (PHP_INT_SIZE != 8) die("skip this test is for 64bit platforms only"); > ?> > =============================================================== > > Yes - if your test is for 64 bit platforms. If other conditions affect the output (endianess etc?) then you should add other tests to the SKIPIF section. Can you also create another PHPT file for 32 bit platforms, if there isn't one already? > I'm afraid I may miss your point. > > > […] > > Could you tell me your intention of this question? So that next time this code changes any breakage is obvious. > > > > […] > > Sorry, I cannot understand what you say, but PHP code changes > from 5.4.3 to 5.4.4 for #54547 and #62097 are: > > https://github.com/php/php-src/commit/9344bf193c6e35c8706923953f3e63bb01cc05ed > https://github.com/php/php-src/commit/acd711685a592c52be200e248154283c6c49c9f8 > > zendi_smart_strcmp() and is_numeric_string[_ex]() are changed. > Any test you create for PHP 5.4 need to either: 1. Pass, or 2. have an XFAIL section I don't know whether or not PHP 5.4 is going to be changed. If it is not going to be changed, then you should do #1. Chris -- christopher.jones@oracle.com http://twitter.com/#!/ghrd --- ## #61012 — Oishi Kazuo — 2012-06-28T02:06:13+00:00 URL: https://externals.io/message/60538#61012 · In reply to: #61006 > So that next time this code changes any breakage is obvious. Next time? I had reported breakage for PHP 5.4.4 RC, but it "wont fix" and PHP 5.4.4 was released. https://bugs.php.net/bug.php?id=62097 I think the breakage exists in current version (PHP 5.4.4). > Any test you create for PHP 5.4 need to either: > > 1. Pass, or > 2. have an XFAIL section > > I don't know whether or not PHP 5.4 is going to be changed. If it is not > going to be changed, then you should do #1. All my tests are passed at previous version (PHP 5.4.3), and are failed at current version (PHP 5.4.4). Behavior of PHP 5.4 WAS changed. I intend to show evidence of breakage (backward incompatibility) in PHP 5.4.4, not intend to write test code to be passed on PHP 5.4.4. -- Oishi Kazuo