# Math functions (new parameter parsing) - Thread: https://externals.io/message/35426 - Original archive: https://news-web.php.net/php.internals/35426 - Started by Felipe Pena on 2008-02-13T00:04:05+00:00 - Messages: 11 - 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 --- ## #35426 — Felipe Pena — 2008-02-13T00:04:05+00:00 URL: https://externals.io/message/35426#35426 Hi all, I made a patch for 5_3 where all functions in ext/standard/math.c use the new parameter parsing. (As was done in HEAD) If anyone think it good, let me know, and then i'll commit it. Several tests break with the patch. Hence, i'll fix them also, if agreed. :) -- Regards, Felipe Pena. --- ## #35427 — Pierre Joye — 2008-02-13T00:31:58+00:00 URL: https://externals.io/message/35426#35427 · In reply to: #35426 Hi! > Hi all, > > I made a patch for 5_3 where all functions in ext/standard/math.c use > the new parameter parsing. (As was done in HEAD) I think it got lost between you and the list :) > If anyone think it good, let me know, and then i'll commit it. > > Several tests break with the patch. Hence, i'll fix them also, if > agreed. :) How do they fail? I worry a bit about BC here. Thanks for your work! -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #35430 — Felipe Pena — 2008-02-13T02:08:11+00:00 URL: https://externals.io/message/35426#35430 · In reply to: #35427 Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu: > Hi! > > > > […] > > I think it got lost between you and the list :) > > > […] > > How do they fail? I worry a bit about BC here. There are two cases. In most part of it: 003+ Warning: `acos()` expects exactly 1 parameter, 2 given ... 003- Warning: Wrong parameter count for `acos()` ... And breaking BC: 010+ 011+ Warning: `acos()` expects parameter 1 to be double, string given ... 012+ `NULL` 010- float(1.570796327) 014+ 015+ Notice: A non well formed numeric value encountered ... PS: The same behavior in HEAD. http://felipe.ath.cx/diff/math.diff > Thanks for your work! Thanks for your attention! :) -- Regards, Felipe Pena. --- ## #35431 — David Coallier — 2008-02-13T02:31:03+00:00 URL: https://externals.io/message/35426#35431 · In reply to: #35430 > Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu: > > […] > > There are two cases. > > In most part of it: > 003+ Warning: `acos()` expects exactly 1 parameter, 2 given ... > 003- Warning: Wrong parameter count for `acos()` ... > > And breaking BC: > 010+ > 011+ Warning: `acos()` expects parameter 1 to be double, string given ... > 012+ `NULL` > 010- float(1.570796327) > 014+ > 015+ Notice: A non well formed numeric value encountered ... > > > PS: The same behavior in HEAD. > > http://felipe.ath.cx/diff/math.diff > > > […] > > Thanks for your attention! :) > -- > Regards, > Felipe Pena. > > > -- > > > > As long as it's in 5_3 I believe there'll be no problems. I did the same for a few string.c functions. As long as the behavior itself doesn't change, I'd say good work :) -- David Coallier, Founder & Software Architect, Agora Production (http://agoraproduction.com) 51.42.06.70.18 --- ## #35435 — Antony Dovgal — 2008-02-13T08:33:35+00:00 URL: https://externals.io/message/35426#35435 · In reply to: #35431 > As long as it's in 5_3 I believe there'll be no problems. I did the > same for a few string.c functions. > > As long as the behavior itself doesn't change, I'd say good work :) Agree, it's ok in 5_3. We need to convert all the functions to use new param parsing API anyway. Some more tests would not hurt though =) -- Wbr, Antony Dovgal --- ## #35436 — Pierre Joye — 2008-02-13T08:47:43+00:00 URL: https://externals.io/message/35426#35436 · In reply to: #35430 > Em Qua, 2008-02-13 às 01:31 +0100, Pierre Joye escreveu: > > […] > > There are two cases. > > In most part of it: > 003+ Warning: `acos()` expects exactly 1 parameter, 2 given ... > 003- Warning: Wrong parameter count for `acos()` ... Ok, any branch. > And breaking BC: > 010+ > 011+ Warning: `acos()` expects parameter 1 to be double, string given ... > 012+ `NULL` > 010- float(1.570796327) > 014+ > 015+ Notice: A non well formed numeric value encountered ... I tend to think to do not break such things in 5.x. But 5.3 sounds like a good opportunity to add a notice about the bad inputs. I can't test the patch right now but does it change the result or only raises a notice? Cheers, -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #35438 — Felipe Pena — 2008-02-13T10:05:18+00:00 URL: https://externals.io/message/35426#35438 · In reply to: #35436 Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu: > I can't test the patch right now but does it change the result or only raises > a notice? It changes the result when a string is given, and issue notice when the string starts with numeric. var_dump(acos("nonsense")); Warning: `acos()` expects parameter 1 to be double, string given ... `NULL` var_dump(acos("1000ABC")); Notice: A non well formed numeric value encountered ... float(NAN) --- ## #35439 — zoe.slattery@googlemail.com — 2008-02-13T10:15:33+00:00 URL: https://externals.io/message/35426#35439 · In reply to: #35426 Felipe Pena wrote: > Hi all, > > I made a patch for 5_3 where all functions in ext/standard/math.c use > the new parameter parsing. (As was done in HEAD) > If anyone think it good, let me know, and then i'll commit it. > > Several tests break with the patch. Hence, i'll fix them also, if > agreed. :) > > Felipe - Great work!! I'll wait for the patch to go in and then re-run the tests that I'm still trying to debug to see if they pass. Zoe --- ## #35440 — Pierre Joye — 2008-02-13T10:16:49+00:00 URL: https://externals.io/message/35426#35440 · In reply to: #35438 > Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu: > > […] > > It changes the result when a string is given, and issue notice when the > string starts with numeric. > > var_dump(acos("nonsense")); > Warning: `acos()` expects parameter 1 to be double, string given ... > `NULL` That's a BC but it seems to be sane to do it. It is amazing that the current version returns acos(0) (obvious but still amazing :). > var_dump(acos("1000ABC")); > Notice: A non well formed numeric value encountered ... > float(NAN) No BC here, as far as I can tell. It returns the same result as before. Thanks for your great work! :) -- Pierre http://blog.thepimp.net | http://www.libgd.org --- ## #35441 — Jani Taskinen — 2008-02-13T10:17:41+00:00 URL: https://externals.io/message/35426#35441 · In reply to: #35438 > Em Qua, 2008-02-13 às 09:47 +0100, Pierre Joye escreveu: > > […] > > It changes the result when a string is given, and issue notice when the > string starts with numeric. > > var_dump(acos("nonsense")); > Warning: `acos()` expects parameter 1 to be double, string given ... > `NULL` > > var_dump(acos("1000ABC")); > Notice: A non well formed numeric value encountered ... > float(NAN) # php -n -derror_reporting=E_ALL -r 'var_dump(acos("ABC"));' float(1.5707963267949) # php -n -derror_reporting=E_ALL -r 'var_dump(acos("100ABC"));' float(NAN) That's with PHP 5.2.5. I rather like the idea of these functions not doing magical conversion like this. -- Patches/Donations: http://pecl.php.net/~jani/ --- ## #35443 — Marcus Boerger — 2008-02-13T10:35:12+00:00 URL: https://externals.io/message/35426#35443 · In reply to: #35426 Hello Felipe, nice work. The others already talked about the implications. And given what I can only chime in. Since so far people think it is a ok for 5.3 I wonder if we should not continue with moving towards the new parameter parsing API. In the end the best would be to get rid of the old one. Anyway thanks for the work. marcus Wednesday, February 13, 2008, 1:04:05 AM, you wrote: > Hi all, > I made a patch for 5_3 where all functions in ext/standard/math.c use > the new parameter parsing. (As was done in HEAD) > If anyone think it good, let me know, and then i'll commit it. > Several tests break with the patch. Hence, i'll fix them also, if > agreed. :) > -- > Regards, > Felipe Pena. Best regards, Marcus