ext/bcmath - Fixed GH-17049: Correctly compare 0 and -0
#17051
Conversation
SakiTakamachi
force-pushed
the
fix/gh17049
branch
from
bab1053 to
5f3a31b
Compare
NEWS
Outdated
| (Saki Takamachi) | ||
| . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) |
There was a problem hiding this comment.
8.4.2 is tagged already, I guess this branch is outdated.
ext/bcmath/libbcmath/src/compare.c
Outdated
| @@ -45,6 +45,14 @@ bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool us | |||
|
|
|||
| /* First, compare signs. */ | |||
| if (use_sign && n1->n_sign != n2->n_sign) { | |||
| if (n1->n_len == 1 && n2->n_len == 1 && | |||
There was a problem hiding this comment.
I understand the change in bc_str2num.
However, I don't fully understand this change: why is this necessary on 8.4 but not on lower branches?
There was a problem hiding this comment.
This is for Number objects, so it is not necessary for lower branches.
(scale and n->n_scale differ only in Number objects. This happens when explicitly specify the scale in a Number method.)
Perhaps it would be more efficient to compare the scale first in the order of the if conditions?
There was a problem hiding this comment.
I don't think this maters a lot for performance.
I missed that the scales are not the same for Number objects of course. Please add a small comment to the check to explain this.
SakiTakamachi
force-pushed
the
fix/gh17049
branch
from
5f3a31b to
28fe1c9
Compare
|
Fixed! |
SakiTakamachi
changed the title
0 and -00 and -0
SakiTakamachi
force-pushed
the
fix/gh17049
branch
from
977c863 to
9f05c40
Compare
|
The last force push just resolved the NEWS conflict. I'll merge, thanks for the review! |
* PHP-8.4: Correctly compare 0 and -0 (#17051)
fixes #17049