FIX GH-12143: Remove pre-rounding

[SakiTakamachi]

take2 of #12268
This PR fixes #12143.

cc:
@TimWolla
@Girgias

remarks:
There is a proposed change that would allow rounding of floating point numbers to the 16th significant digit, but this PR does not include it.
I don't like having too many review perspectives, so I'll deal with it in a separate PR.

---

Main points

The discussion is scattered all over the place and difficult to understand, so I will summarize the main points.

There were two problems with round

1. Values ​​such as round(0.49999999999999994, 0), where "adding 0.5 will carry up due to error'' are incorrectly rounded.
2. "pre-round" incorrectly rounds values ​​like round(1.70000000000145, 13)

1 was already fixed and I was trying to fix 2.

Learn more about problem 2

"pre-round" performs the following calculations.

round(1.70000000000145, 13)

1.70000000000145
17000000000014.5 // digit adjustment
17000000000015 // pre-round
1700000000001.5 // digit adjustment
1700000000002 // round
1.700000000002 // return

Wrongly rounded due to "pre-round".

Why is "pre-round" necessary?

The need for "pre-round" is summarized in the following article, "Analysis of the problems of the previous round() implementation" and "Pre-rounding to the value's precision if possible".

https://wiki.php.net/rfc/rounding

Simply put, "pre-round" was needed to solve the kind of problem where the result of round(0.285, 2) is 0.28.

But the premise is wrong

This PR change is based on the idea that treating FP as if it were a decimal rational number is logically incorrect.
See #12268 for more information.

About my changes

"pre-round" processing has been completely removed. And I removed the test for "pre-round".

[TimWolla]

Will need to think about the other stuff a little more.

[mvorisek]

0.28499999999999998 is theoretical meaningless value, the stored accuracy is lower.

0.285 is simply 0.285, the rounding must match double to string representation /w precision set to -1.

https://3v4l.org/WiS71

[SakiTakamachi]

please see #12268

To be honest, I'm starting to feel like it's impossible for me to reconcile the divided opinions, and I'm starting to think about posting about this on internals.

[mvorisek]

see https://github.com/php/php-src/blob/php-8.2.10/Zend/zend_strtod.c#L3704 for double -> string conversion code /w mode=0, not sure if rounding thru string conversion will be fast enough, but in the end, all roundings must equal when does this way

[SakiTakamachi]

@bukka

I do not plan to make any further changes. Could you please look it when you have time?

[bukka]

@SakiTakamachi I have been thinking about this and #12268 and there is a good point about what was voted. The currently accepted RFC is https://wiki.php.net/rfc/rounding which explicitly states that it should be treated as decimal. If we want to change this (which is what this PR does), we need an RFC to apply such change.

But until there is an RFC, what's in #12268 should be considered as the right approach as it fixes the mentioned pre-rounding issue and handles the edge cases. I would even consider this as a bug fix in some way but probably still just for master because it depends on some master only code and it is quite invasive.

So I think it's up to you what you solution you want to pursue. I'm willing to accept #12268 as a bug fix for master but if you prefer to do it this way, then it needs an RFC and deeper discussion to override the current RFC. Of course if you don't want to do such RFC but @TimWolla or @Girgias are happy to do that, then we should wait for decision on their RFC.

[SakiTakamachi]

@bukka
I see. That makes sense. Thank you for both confirmations.

No matter which PR we choose, I think we should target the master branch because as you say, there are dependencies on the master branch's code. That means we have enough time to create the RFC and wait for the results, without rushing to merge now.

Both PRs were prepared by me, so I will first email internals regarding this matter. If the responses are not clearly biased towards downvotes, create an RFC.

I will keep both PRs in draft status until the results are known.

[SakiTakamachi]

This pull request will be closed as the policy has been finalized in the following RFC.
https://wiki.php.net/rfc/change_the_edge_case_of_round