Skip to content

Fix GH-12143: Remove pre round#12260

Closed
SakiTakamachi wants to merge 2 commits intophp:masterfrom
SakiTakamachi:fix/gh-12143-remove-pre-round
Closed

Fix GH-12143: Remove pre round#12260
SakiTakamachi wants to merge 2 commits intophp:masterfrom
SakiTakamachi:fix/gh-12143-remove-pre-round

Conversation

@SakiTakamachi
Copy link
Member

@SakiTakamachi SakiTakamachi commented Sep 21, 2023 •
edited
Loading

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

@SakiTakamachi SakiTakamachi mentioned this pull request Sep 21, 2023
Closed
SakiTakamachi
SakiTakamachi commented Sep 21, 2023
View reviewed changes
ext/standard/math.c
* 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

@SakiTakamachi SakiTakamachi Sep 21, 2023

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

SakiTakamachi commented Sep 21, 2023

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

Reviewers

@bukka bukka Awaiting requested review from bukka bukka is a code owner

Assignees

No one assigned

Labels

Extension: standard

Projects

None yet

Milestone

No milestone

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

@SakiTakamachi