Everything

php

[RFC] [Discussion] Allow int type argument to BCMath function

php.internals

Saki Takamachi

2 years ago

Hi all,

I created an RFC and put it in discussion status.
https://wiki.php.net/rfc/allow_int_args_to_bcmath_function

It's a very simple RFC, so unless there's any major discussion, I plan to start voting on this on 7/23, after two weeks have passed.

Regards,

Saki

Unnamed Person

2 years ago

On 8-7-2024 3:23, Saki Takamachi wrote:

> Hi all,
>
> I created an RFC and put it in discussion status.
> https://wiki.php.net/rfc/allow_int_args_to_bcmath_function
>
> It's a very simple RFC, so unless there's any major discussion, I plan to start voting on this on 7/23, after two weeks have passed.
>
> Regards,
>
> Saki

Hi Saki,

Just wondering: why `int|string` instead of `float|string` ?
In my experience, floats are the more pertinent type for which to use
the BCMath extension.

Smile,
Juliette

Saki Takamachi

2 years ago

Hi Juliette,

> Hi Saki,
>
> Just wondering: why `int|string` instead of `float|string` ?
> In my experience, floats are the more pertinent type for which to use the BCMath extension.
>
> Smile,
> Juliette

Since floating point numbers always contain a certain amount of error, I believe that they are not suitable as arguments for BCMath.

For example, 0.285 becomes 0.284999... in floating point. From the perspective of arbitrary precision math, there is no universally agreeable way to properly handle values ​​like this: there will always be people who want to treat it as 0.285, and people who want to treat it as 0.284999....

Therefore, I believe BCMath should not accept floats as arguments.

Regards,

Saki

Unnamed Person

2 years ago

On 8-7-2024 5:18, Saki Takamachi wrote:

> Hi Juliette,
>
>> Hi Saki,
>>
>> Just wondering: why `int|string` instead of `float|string` ?
>> In my experience, floats are the more pertinent type for which to use the BCMath extension.
>>
>> Smile,
>> Juliette
> Since floating point numbers always contain a certain amount of error, I believe that they are not suitable as arguments for BCMath.

Okay, so in that case, I must be missing something.

> For example, 0.285 becomes 0.284999... in floating point. From the perspective of arbitrary precision math, there is no universally agreeable way to properly handle values ​​like this: there will always be people who want to treat it as 0.285, and people who want to treat it as 0.284999....

I honestly don't understand what you are saying here.

If 0.285 is a string, then no type conversion would be needed to pass it
to BCMath anyhow.
Otherwise, 0.285 would be a floating point number already.

If the type for BCMath would become `int|string`, in non-strict mode,
there would be quite a significant change in behaviour for the BCMath
functions as passing the _float_ 0.285 would previously result in the
string "0.285" being passed to the BCMath function, but would now result
in the integer 0 being passed.

I think this BC-break will need to be called out as it is not mentioned
in the RFC.
I also think this BC-break should be enough reason not to accept the RFC.

Smile,
Juliette

Saki Takamachi

2 years ago

Hi Juliette,

> I honestly don't understand what you are saying here.
>
> If 0.285 is a string, then no type conversion would be needed to pass it to BCMath anyhow.
> Otherwise, 0.285 would be a floating point number already.
>
> If the type for BCMath would become `int|string`, in non-strict mode, there would be quite a significant change in behaviour for the BCMath functions as passing the _float_ 0.285 would previously result in the string "0.285" being passed to the BCMath function, but would now result in the integer 0 being passed.
>
> I think this BC-break will need to be called out as it is not mentioned in the RFC.
> I also think this BC-break should be enough reason not to accept the RFC.

I had overlooked the priority order of Union type casts, thank you.

I don't think a satisfactory specification will be decided on this before the feature freeze, so I will not rush into discussing this RFC for now. Of course, no voting will take place until the specifications are decided. If we were to discuss this change again, it would be in versions beyond 8.4.

Regards,

Saki

Tim Düsterhus

2 years ago

Hi

On 7/8/24 06:29, Saki Takamachi wrote:

> I don't think a satisfactory specification will be decided on this before the feature freeze, so I will not rush into discussing this RFC for now. Of course, no voting will take place until the specifications are decided. If we were to discuss this change again, it would be in versions beyond 8.4.

I agree with Juliette that as long as passing a non-integer float to an
int parameter is only a deprecation warning, widening the signature to
`string|int` is too much of a breaking change.

Given that once you called a BCMath function the output is a string, the
int->string conversion would only happen at the inputs. The performance
cost should be fully acceptable and this looks like a premature
micro-optimization.

Best regards
Tim Düsterhus

Derick Rethans

2 years ago

On Tue, 9 Jul 2024, Tim Düsterhus wrote:

> On 7/8/24 06:29, Saki Takamachi wrote:
>
> > I don't think a satisfactory specification will be decided on this
> > before the feature freeze, so I will not rush into discussing this
> > RFC for now. Of course, no voting will take place until the
> > specifications are decided. If we were to discuss this change again,
> > it would be in versions beyond 8.4.
>
> I agree with Juliette that as long as passing a non-integer float to
> an int parameter is only a deprecation warning, widening the signature
> to `string|int` is too much of a breaking change.

I agree too, and don't think we should change this in PHP 8.x.

and probably would also question:

"bcfloor() and bcceil() are excluded because there is no point in
allowing int type arguments due to the characteristics of their
functions. "

Although that is right, having these *also* accept ints would at least
be consistent.

cheers,
Derick

Jordan LeDoux

2 years ago

On Sun, Jul 7, 2024 at 8:19 PM Saki Takamachi <saki@sakiot.com> wrote:

> Hi Juliette,
>
> > Hi Saki,
> >
> > Just wondering: why `int|string` instead of `float|string` ?
> > In my experience, floats are the more pertinent type for which to use
> the BCMath extension.
> >
> > Smile,
> > Juliette
>
> Since floating point numbers always contain a certain amount of error, I
> believe that they are not suitable as arguments for BCMath.
>
> For example, 0.285 becomes 0.284999... in floating point. From the
> perspective of arbitrary precision math, there is no universally agreeable
> way to properly handle values like this: there will always be people who
> want to treat it as 0.285, and people who want to treat it as 0.284999....
>
> Therefore, I believe BCMath should not accept floats as arguments.
>
> Regards,
>
> Saki

I suspected the same thing when I was doing my arbitrary precision library,
but I actually have yet to find a test case in all my unit tests where this
happens when converting to a string. You can see this here:

https://3v4l.org/Ud8Cn

Jordan

Tim Düsterhus

2 years ago

Hi

On 7/8/24 11:36, Jordan LeDoux wrote:

> I suspected the same thing when I was doing my arbitrary precision library,
> but I actually have yet to find a test case in all my unit tests where this
> happens when converting to a string. You can see this here:
>
> https://3v4l.org/Ud8Cn

PHP emits the shortest possible representation that roundtrips, so the
precision loss indeed does not happen when converting from float, but
rather when converting to float / when the implicit rounding happens
during the calculation.

Nevertheless the inherent rounding error of floats and an arbitrary
precision maths library do not mix: Users should not be encouraged to
mindlessly pass a float, but rather work with strings all the time or
explicitly perform the necessary conversion as appropriate for their use
case - which of course is easier with strict_types enabled because then
the conversion would need to be made explicit with a cast.

Best regards
Tim Düsterhus

Jordan LeDoux

2 years ago

On Tue, Jul 9, 2024 at 10:47 AM Tim Düsterhus <tim@bastelstu.be> wrote:

> Hi
>
> On 7/8/24 11:36, Jordan LeDoux wrote:
> > I suspected the same thing when I was doing my arbitrary precision
> library,
> > but I actually have yet to find a test case in all my unit tests where
> this
> > happens when converting to a string. You can see this here:
> >
> > https://3v4l.org/Ud8Cn
>
> PHP emits the shortest possible representation that roundtrips, so the
> precision loss indeed does not happen when converting from float, but
> rather when converting to float / when the implicit rounding happens
> during the calculation.
>
> Nevertheless the inherent rounding error of floats and an arbitrary
> precision maths library do not mix: Users should not be encouraged to
> mindlessly pass a float, but rather work with strings all the time or
> explicitly perform the necessary conversion as appropriate for their use
> case - which of course is easier with strict_types enabled because then
> the conversion would need to be made explicit with a cast.
>
> Best regards
> Tim Düsterhus
>

Yes, absolutely agree that arbitrary precision math should not be done with
floats, and developers should not be encouraged to use floats. However for
the specific case of type-coercion into the argument of a `bcadd` or
similar call, the detailed string conversion issues do not happen, which is
all I was saying.

Integers ALSO have a maximum precision, detailed by the `PHP_INT_MAX`
constant, which varies by system and install (mainly on whether 64-bit
integers are available).

My point was not that floats or integers are sufficient, but that the entry
point into an arbitrary precision calculation often comes from numbers that
are stored as ints and floats. If we accept ints, and don't ask the
developer to explicitly cast to string, I think we should do the same for
floats. Both or neither. I can see the argument against accepting either, I
can see the argument for accepting either.

I do not understand the argument for accepting one but not the other.

Jordan

Tim Düsterhus

2 years ago

Hi

On 7/9/24 22:27, Jordan LeDoux wrote:

> I do not understand the argument for accepting one but not the other.
>

That's fair enough.

I do still consider accepting 'int' to be less unsafe than accepting
'float', because the behavior of ints can be reasoned about more easily
and because they to not experience implicit rounding.

I do not particularly care whether or not 'int' is accepted or not for
that reason, but I care about 'float' not being accepted. So perhaps
it's best to accept neither as you suggested.

Best regards
Tim Düsterhus

Saki Takamachi

2 years ago

Hi Tim,

> That's fair enough.
>
> I do still consider accepting 'int' to be less unsafe than accepting 'float', because the behavior of ints can be reasoned about more easily and because they to not experience implicit rounding.
>
> I do not particularly care whether or not 'int' is accepted or not for that reason, but I care about 'float' not being accepted. So perhaps it's best to accept neither as you suggested.

Yes, I had completely forgotten about casting priorities.

So, at least I don't plan on including it in 8.4, and may even retract the change altogether.

Regards,

Saki