Skip to content

Commit afa4dba

Browse files
committed
Reimplement php_round_helper() using modf()
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 phpGH-12143 (this fixes one of the reported cases) Closes phpGH-12159 which was an alternative attempt to fix the rounding issue for `0.49999999999999994`
1 parent 7c4db15 commit afa4dba

File tree

5 files changed

+140
-17
lines changed

5 files changed

+140
-17
lines changed

ext/standard/math.c

+32-17
Original file line numberDiff line numberDiff line change
@@ -93,27 +93,42 @@ 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;
9797

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;
98+
fractional = fabs(modf(value, &integral));
99+
100+
switch (mode) {
101+
case PHP_ROUND_HALF_UP:
102+
if (fractional >= 0.5) {
103+
return integral + copysign(1.0, integral);
104+
}
105+
106+
return integral;
107+
108+
case PHP_ROUND_HALF_DOWN:
109+
if (fractional > 0.5) {
110+
return integral + copysign(1.0, integral);
111+
}
112+
113+
return integral;
114+
115+
case PHP_ROUND_HALF_EVEN:
116+
case PHP_ROUND_HALF_ODD: {
117+
if (fractional == 0.5) {
118+
bool even = !fmod(integral, 2.0);
119+
120+
if ((mode == PHP_ROUND_HALF_EVEN && !even) || (mode == PHP_ROUND_HALF_ODD && even)) {
121+
return integral + copysign(1.0, integral);
122+
}
123+
} else if (fractional > 0.5) {
124+
return integral + copysign(1.0, integral);
125+
}
126+
127+
return integral;
113128
}
114129
}
115130

116-
return tmp_value;
131+
ZEND_ASSERT(0 && "Unknown rounding mode");
117132
}
118133
/* }}} */
119134

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
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
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
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)