Skip to content

Fix GH-12143: Remove pre round #12260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

SakiTakamachi
Copy link
Member

@SakiTakamachi SakiTakamachi commented Sep 21, 2023

This is an improved version of #12162.
This PR fixes #12143

Simply removing the pre round caused the following problems.

var_dump (round (0.285, 2, PHP_ROUND_HALF_UP));
// float(0.28)

Therefore, first, adjust the digits so that the 15th digit, which is guaranteed to be accurate, is an integer.
For example, 0.285(0.28499999999999998) to 285000000000000.

Then, adjust again to the digit that should be calculated to use the helper.
For example, 285000000000000 to 28.5

Floating point numbers have some degree of error, so it try to approximate the value as close to an integer as possible.
Therefore, by setting the range that guarantees accuracy to an integer, we can approximate it in a way that is closer to our intuition.

this change
Breakpoint 1, _php_math_round (value=0.28499999999999998, places=2, mode=1) at /php-src/ext/standard/math.c:171
warning: Source file is more recent than executable.
171	PHPAPI double _php_math_round(double value, int places, int mode) {
(gdb) n
176		if (!zend_finite(value) || value == 0.0) {
(gdb) 
180		places = places < INT_MIN+1 ? INT_MIN+1 : places;
(gdb) 
181		value_places = -php_intlog10abs(value);
(gdb) 
183		f1 = php_intpow10(abs(places));
(gdb) 
184		if (value_places + 16 > places && value_places <= places) {
(gdb) 
185			int precision_places = value_places + 14;
(gdb) 
186			double f2 = php_intpow10(abs(precision_places));
(gdb) 
188			if (precision_places >= 0) {
(gdb) 
189				tmp_value = value * f2;
(gdb) 
194			int use_precision = places - precision_places;
(gdb) p tmp_value
$1 = 285000000000000
(gdb) n
195			use_precision = use_precision < INT_MIN+1 ? INT_MIN+1 : use_precision;
(gdb) 
196			f2 = php_intpow10(abs(use_precision));
(gdb) 
198			if (use_precision >= 0) {
(gdb) 
201				tmp_value = tmp_value / f2;
(gdb) 
184		if (value_places + 16 > places && value_places <= places) {
(gdb) p tmp_value
$2 = 28.5
simply removed pre-round
Breakpoint 1, _php_math_round (value=0.28499999999999998, places=2, mode=1) at /php-src/ext/standard/math.c:127
127	PHPAPI double _php_math_round(double value, int places, int mode) {
(gdb) n
131		if (!zend_finite(value) || value == 0.0) {
(gdb) 
135		places = places < INT_MIN+1 ? INT_MIN+1 : places;
(gdb) 
137		f1 = php_intpow10(abs(places));
(gdb) 
140		if (places >= 0) {
(gdb) 
141			tmp_value = value * f1;
(gdb) 
146		if (fabs(tmp_value) >= 1e15) {
(gdb) p tmp_value
$1 = 28.499999999999996

* digits are adjusted to ensure accuracy. Digits adjustment is performed only if
* the location specifies a range that can be represented by floating point numbers.
*/
if (value_places + 16 > places && value_places <= places) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a mistake here, I'll fix it later

@SakiTakamachi
Copy link
Member Author

This method didn't work. Too many edge cases.

@SakiTakamachi SakiTakamachi deleted the fix/gh-12143-remove-pre-round branch September 24, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect round($num, 0, PHP_ROUND_HALF_UP) result for $num = 1.4999999999999998 / 4503599627370495.5
1 participant