Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120715
Content-Type: multipart/alternative;
 boundary="------------BQiOJk6Np5SZG5IL6S3vp0p7"
Message-ID: <3bf5d1a5-f0c2-9db7-acbf-d25dceb65da1@mabe.berlin>
Date: Wed, 28 Jun 2023 22:04:43 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Content-Language: en-US
To: internals@lists.php.net
References: <05b7e019-27d4-e6a9-2088-f712c22317a9@mabe.berlin>
 <CA+kxMuSP1TTB5141-5ePJWZT5_DZ8rCHvkh0ikKBcnXYSMV5zg@mail.gmail.com>
 <4D8DE606-5E99-4D6F-962E-CBE1392C4B94@gmail.com>
 <8b23cbe6-1c4d-40b6-a68b-780d5e8f9dbf@app.fastmail.com>
 <27098d15-c257-b667-5509-0129b7efd695@mabe.berlin>
 <CAPzBOBOUsP=sJqD1UA_8cLukvoVni6tmA6qpfmH88atSF8yTbA@mail.gmail.com>
In-Reply-To: <CAPzBOBOUsP=sJqD1UA_8cLukvoVni6tmA6qpfmH88atSF8yTbA@mail.gmail.com>
Subject: Re: [PHP-DEV] rounding integers
From: marc@mabe.berlin (Marc)

--------------BQiOJk6Np5SZG5IL6S3vp0p7
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi again,

On 22.05.23 13:55, Robert Landers wrote:
>> New function(s) on the other hand sounds like an ugly solution as well
>> only helping people explicitly searching for it after they got bitten at
>> least once.
> What about deprecating round() in favor of round_float() and
> round_int()? or something.
>
> Robert Landers
> Software Engineer
> Utrecht NL


I have done some research and worked on an implementation [1] even 
thought I'm not a C engineer.

There are some things I have noticed:

1. From PHP's type system perspective an int is completely compatible to 
an int also in strict mode - https://3v4l.org/sspOM

That means making round, ceil and floor returning an int should not be 
much less of an issue

2. The functions round, ceil and floor are documented to take an 
int|float as argument rather then a float only instead of a float only

For me that means there is already some kind of difference expected as 
if it would be documented as float only

3. Documenting it as input type matches the output type does not work in 
corner cases as rounding up/down but integer numbers could lead to 
integer over/under flows which needs to be handled and casted into a 
floating point number falling back current behavior.

4. I figured out one more function "number_format()" which casts 
everything into a floating point number even though it can unnecessary 
ending up in rounding errors.

Fixing this is not even a BC break at all and could go separately as a 
bug fix in my opinion.

5. It least in case of ceil and floor this would be a no-op instead of a 
cast + memory allocation for a new zval

In case of round as well but there are more things to do - haven't done 
any performance tests so far


I have the felling that this change is not that much of a BC break and 
serves the right purpose by reducing rounding errors as much as possible.

Still as these are very highly used functions I would target the next 
major just as there BC breaks are somehow expected even though I now 
think the only BC break is on explicitly checking the types after rounding.


Thanks,

Marc


[1] 
https://github.com/php/php-src/compare/master...marc-mabe:php-src:integer_rounding

--------------BQiOJk6Np5SZG5IL6S3vp0p7--