php / php-src

The PHP Interpreter
https://www.php.net
Other
38.29k stars 7.76k forks source link

PHP 8.4.1 round() behaves differently than earlier versions #16930

Open splitbrain opened 1 day ago

splitbrain commented 1 day ago

Description

The following code:

<?php                                                                           
echo PHP_VERSION;                                                               
echo "\n";                                                                      

$float = 213.49999999999997;                                                    
$round = round($float, 0, PHP_ROUND_HALF_UP);                                   

var_dump($float);                                                               
var_dump($round);

Resulted in this output:

8.4.1
float(213.49999999999997)
float(213)

But I expected this output instead:

8.3.14
float(213.49999999999997)
float(214)

All tested previous releases did produce the expected output.

PHP Version

PHP 8.4.1

Operating System

official docker php:8.4-cli image

splitbrain commented 1 day ago

This might be an intended change, possibly related to 2e50371a7a238552f6e74a22d4f9587df7f3dd2e or 703ead5a26a57cd4de03e4a9d978646b01107ee5? If so it should be documented at https://www.php.net/manual/en/migration84.incompatible.php because it may subtly break application's tests relying on the old behavior and is a pain to identify.

splitbrain commented 1 day ago

Note: for people relying on the old behavior, pre-rounding to a lesser precision first seems to work:

$int = round(round($float, 13));
Girgias commented 1 day ago

It is documented, but on the Other Changes section: https://www.php.net/manual/en/migration84.other-changes.php

cmb69 commented 18 hours ago

@splitbrain, I haven't checked, but I think this is caused by 703ead5a26a57cd4de03e4a9d978646b01107ee5.

@Girgias, it may be sensible to elevate this change:

The maximum precision that can be handled by round() has been extended by one digit. For example, round(4503599627370495.5) returned in 4503599627370495.5, but now returns 4503599627370496.

to the incompatible section (and maybe to explain more thoroughly). There has been quite some discussion and complaining about it in #14332.

Girgias commented 17 hours ago

I am happy for this to be moved, and I still think this whole round() change was a mistake in the end, even if it did fix some weird edge cases... considering that in the end we are still not using IEEE 754 semantics...

cmb69 commented 17 hours ago

Well, the rounding changes might have been a bit rushed, but considering https://3v4l.org/YbUMA#v8.4.1, I think it doesn't only fix weird edge cases. I mean, I'm telling round() to give me an integral number, but it fails. WTF! Still, we should get rid of the pre-rounding; looks bonkers to me.

Girgias commented 17 hours ago

Agreed, but considering the discussion for the related RFC had this discussion, and it went nowhere, I don't have high hopes...