Skip to content

Commit 9652889

Browse files
TimWollaTimWolla
authored
Reimplement php_round_helper() using modf() (#12220)
This change makes the implementation much easier to understand, by explicitly handling the various cases. It fixes rounding for `0.49999999999999994`, because no loss of precision happens by adding / subtracing `0.5` before turning the result into an integral float. Instead the fractional parts are explicitly compared. see GH-12143 (this fixes one of the reported cases) Closes GH-12159 which was an alternative attempt to fix the rounding issue for `0.49999999999999994`
1 parent 83738fc commit 9652889

File tree

7 files changed

+183
-18
lines changed

7 files changed

+183
-18
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,7 @@ DOM:
1010

1111
Standard:
1212
. Implement GH-12188 (Indication for the int size in phpinfo()). (timwolla)
13+
. Partly fix GH-12143 (Incorrect round() result for 0.49999999999999994).
14+
(timwolla)
1315

1416
<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>

‎UPGRADING

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ PHP 8.4 UPGRADE NOTES
5050
5. Changed Functions
5151
========================================
5252

53+
- Standard:
54+
. The internal implementation for rounding to integers has been rewritten
55+
to be easier to verify for correctness and to be easier to maintain.
56+
Some rounding bugs have been fixed as a result of the rewrite. For
57+
example previously rounding 0.49999999999999994 to the nearest integer
58+
would have resulted in 1.0 instead of the correct result 0.0. Additional
59+
inputs might also be affected and result in different outputs compared to
60+
earlier PHP versions.
61+
5362
========================================
5463
6. New Functions
5564
========================================

‎ext/standard/math.c

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,73 @@ static inline double php_intpow10(int power) {
9393
/* {{{ php_round_helper
9494
Actually performs the rounding of a value to integer in a certain mode */
9595
static inline double php_round_helper(double value, int mode) {
96-
double tmp_value;
96+
double integral, fractional;
97+
98+
/* Split the input value into the integral and fractional part.
99+
*
100+
* Both parts will have the same sign as the input value. We take
101+
* the absolute value of the fractional part (which will not result
102+
* in branches in the assembly) to make the following cases simpler.
103+
*/
104+
fractional = fabs(modf(value, &integral));
105+
106+
switch (mode) {
107+
case PHP_ROUND_HALF_UP:
108+
if (fractional >= 0.5) {
109+
/* We must increase the magnitude of the integral part
110+
* (rounding up / towards infinity). copysign(1.0, integral)
111+
* will either result in 1.0 or -1.0 depending on the sign
112+
* of the input, thus increasing the magnitude, but without
113+
* generating branches in the assembly.
114+
*
115+
* This pattern is equally used for all the other modes.
116+
*/
117+
return integral + copysign(1.0, integral);
118+
}
97119

98-
if (value >= 0.0) {
99-
tmp_value = floor(value + 0.5);
100-
if ((mode == PHP_ROUND_HALF_DOWN && value == (-0.5 + tmp_value)) ||
101-
(mode == PHP_ROUND_HALF_EVEN && value == (0.5 + 2 * floor(tmp_value/2.0))) ||
102-
(mode == PHP_ROUND_HALF_ODD && value == (0.5 + 2 * floor(tmp_value/2.0) - 1.0)))
103-
{
104-
tmp_value = tmp_value - 1.0;
105-
}
106-
} else {
107-
tmp_value = ceil(value - 0.5);
108-
if ((mode == PHP_ROUND_HALF_DOWN && value == (0.5 + tmp_value)) ||
109-
(mode == PHP_ROUND_HALF_EVEN && value == (-0.5 + 2 * ceil(tmp_value/2.0))) ||
110-
(mode == PHP_ROUND_HALF_ODD && value == (-0.5 + 2 * ceil(tmp_value/2.0) + 1.0)))
111-
{
112-
tmp_value = tmp_value + 1.0;
113-
}
120+
return integral;
121+
122+
case PHP_ROUND_HALF_DOWN:
123+
if (fractional > 0.5) {
124+
return integral + copysign(1.0, integral);
125+
}
126+
127+
return integral;
128+
129+
case PHP_ROUND_HALF_EVEN:
130+
if (fractional > 0.5) {
131+
return integral + copysign(1.0, integral);
132+
}
133+
134+
if (fractional == 0.5) {
135+
bool even = !fmod(integral, 2.0);
136+
137+
/* If the integral part is not even we can make it even
138+
* by adding one in the direction of the existing sign.
139+
*/
140+
if (!even) {
141+
return integral + copysign(1.0, integral);
142+
}
143+
}
144+
145+
return integral;
146+
case PHP_ROUND_HALF_ODD:
147+
if (fractional > 0.5) {
148+
return integral + copysign(1.0, integral);
149+
}
150+
151+
if (fractional == 0.5) {
152+
bool even = !fmod(integral, 2.0);
153+
154+
if (even) {
155+
return integral + copysign(1.0, integral);
156+
}
157+
}
158+
159+
return integral;
114160
}
115161

116-
return tmp_value;
162+
ZEND_UNREACHABLE();
117163
}
118164
/* }}} */
119165

‎ext/standard/tests/math/round_gh12143_1.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-12143: Test rounding of 0.49999999999999994.
3+
--FILE--
4+
<?php
5+
foreach ([
6+
0.49999999999999994,
7+
-0.49999999999999994,
8+
] as $number) {
9+
foreach ([
10+
'PHP_ROUND_HALF_UP',
11+
'PHP_ROUND_HALF_DOWN',
12+
'PHP_ROUND_HALF_EVEN',
13+
'PHP_ROUND_HALF_ODD',
14+
] as $mode) {
15+
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
16+
}
17+
}
18+
?>
19+
--EXPECT--
20+
PHP_ROUND_HALF_UP : +0.49999999999999994 -> +0
21+
PHP_ROUND_HALF_DOWN : +0.49999999999999994 -> +0
22+
PHP_ROUND_HALF_EVEN : +0.49999999999999994 -> +0
23+
PHP_ROUND_HALF_ODD : +0.49999999999999994 -> +0
24+
PHP_ROUND_HALF_UP : -0.49999999999999994 -> -0
25+
PHP_ROUND_HALF_DOWN : -0.49999999999999994 -> -0
26+
PHP_ROUND_HALF_EVEN : -0.49999999999999994 -> -0
27+
PHP_ROUND_HALF_ODD : -0.49999999999999994 -> -0

‎ext/standard/tests/math/round_gh12143_2.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-12143: Test rounding of 0.50000000000000011.
3+
--FILE--
4+
<?php
5+
foreach ([
6+
0.50000000000000011,
7+
-0.50000000000000011,
8+
] as $number) {
9+
foreach ([
10+
'PHP_ROUND_HALF_UP',
11+
'PHP_ROUND_HALF_DOWN',
12+
'PHP_ROUND_HALF_EVEN',
13+
'PHP_ROUND_HALF_ODD',
14+
] as $mode) {
15+
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
16+
}
17+
}
18+
?>
19+
--EXPECT--
20+
PHP_ROUND_HALF_UP : +0.50000000000000011 -> +1
21+
PHP_ROUND_HALF_DOWN : +0.50000000000000011 -> +1
22+
PHP_ROUND_HALF_EVEN : +0.50000000000000011 -> +1
23+
PHP_ROUND_HALF_ODD : +0.50000000000000011 -> +1
24+
PHP_ROUND_HALF_UP : -0.50000000000000011 -> -1
25+
PHP_ROUND_HALF_DOWN : -0.50000000000000011 -> -1
26+
PHP_ROUND_HALF_EVEN : -0.50000000000000011 -> -1
27+
PHP_ROUND_HALF_ODD : -0.50000000000000011 -> -1

‎ext/standard/tests/math/round_gh12143_3.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-12143: Test rounding of 0.5.
3+
--FILE--
4+
<?php
5+
foreach ([
6+
0.5,
7+
-0.5,
8+
] as $number) {
9+
foreach ([
10+
'PHP_ROUND_HALF_UP',
11+
'PHP_ROUND_HALF_DOWN',
12+
'PHP_ROUND_HALF_EVEN',
13+
'PHP_ROUND_HALF_ODD',
14+
] as $mode) {
15+
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
16+
}
17+
}
18+
?>
19+
--EXPECT--
20+
PHP_ROUND_HALF_UP : +0.5 -> +1
21+
PHP_ROUND_HALF_DOWN : +0.5 -> +0
22+
PHP_ROUND_HALF_EVEN : +0.5 -> +0
23+
PHP_ROUND_HALF_ODD : +0.5 -> +1
24+
PHP_ROUND_HALF_UP : -0.5 -> -1
25+
PHP_ROUND_HALF_DOWN : -0.5 -> -0
26+
PHP_ROUND_HALF_EVEN : -0.5 -> -0
27+
PHP_ROUND_HALF_ODD : -0.5 -> -1

‎ext/standard/tests/math/round_gh12143_4.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-12143: Test rounding of 1.5.
3+
--FILE--
4+
<?php
5+
foreach ([
6+
1.5,
7+
-1.5,
8+
] as $number) {
9+
foreach ([
10+
'PHP_ROUND_HALF_UP',
11+
'PHP_ROUND_HALF_DOWN',
12+
'PHP_ROUND_HALF_EVEN',
13+
'PHP_ROUND_HALF_ODD',
14+
] as $mode) {
15+
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
16+
}
17+
}
18+
?>
19+
--EXPECT--
20+
PHP_ROUND_HALF_UP : +1.5 -> +2
21+
PHP_ROUND_HALF_DOWN : +1.5 -> +1
22+
PHP_ROUND_HALF_EVEN : +1.5 -> +2
23+
PHP_ROUND_HALF_ODD : +1.5 -> +1
24+
PHP_ROUND_HALF_UP : -1.5 -> -2
25+
PHP_ROUND_HALF_DOWN : -1.5 -> -1
26+
PHP_ROUND_HALF_EVEN : -1.5 -> -2
27+
PHP_ROUND_HALF_ODD : -1.5 -> -1

0 commit comments

Comments
 (0)