Fix #46564 precision bug in bcmod #1582
Conversation
|
It would be nice to add a test for this. |
|
Test added. |
There was a problem hiding this comment.
I'm not sure I get this - how comes bcmod returns non-integer? I always thought module operation always returns integer. Also, 3.5 % 4 returns 3, so I don't see why BC version should be different.
There was a problem hiding this comment.
Operator % probably calculate only with integer. But for example function fmod calculate with floats numbers. Example: fmod(3.5, 4) returns 3.5. Why is diference between fmod and bcmod...?
Example on wolfram aplha: http://www.wolframalpha.com/input/?i=3.5+mod+4
|
Who needs correct calculation modulo without bugfix in PHP, can use: function modulo($number, $modulus) {
return bcsub($number, bcmul(bcdiv($number, $modulus, 0), $modulus));
}
bcscale(1);
var_dump(modulo("3.5", "4")); // 3.5
var_dump(modulo("1071", "357.5")); // 356.0 |
|
@krakjoe Done. |
|
This change looks fine to me, but I'm slightly concerned about the BC impact this might have. Code may rely on the current behavior :/ |
|
@nikic there's plenty of time for an RFC for 7.2, think that's best ? |
|
I don't think an RFC is worthwhile for such a minor change. This is a "bug fix with potential BC impact", not a new feature or an intentional BC break. I'm inclined to just merge this with an UPGRADING note and see if anyone complains. |
|
Oh, yeah, UPGRADING is a must. I was quite comfortable with it in 7.2 anyway, so long as no major objections. |
|
Merged via 90dcbbe with a note in UPGRADING. Thanks! Interestingly, bcmod() already behaved correctly in HHVM :) |
PR fixed bug #46564 in bcmod. Now is supported decimal number.
This bug is not in 3rd party library (as write in ticket), but is in PHP.