BCMath and floats

php.internals

Christoph Becker

8 years ago
Hi everybody! All BCMath functions expects strings as operands (in the strict sense; do not confuse with arguments). This of course can result in unexpected behavior if floats are passed, namely: * floats may be converted to a string in scientific notation * floats may be converted with a decimal separator which is not a point Both result in BCMath treating such strings as '0'. Over the years there have been at least six bug reports regarding this issue: #10116, #42871, #47633, #55160, #66745, #66959. The last one has been reclassified as feature request, and I would like to address it. I already have proposed to introduce bcinit() which would take a string, int or float, and return a properly formatted string with the given or default scale, see <https://github.com/php/php-src/pull/2746>. Nikita suggested to accept the arguments allowed by `bcinit()` for all functions in the first place. This would, however, raise the question which scale should be applied to floats. Before proceeding to an RFC (if necessary at all), I'm looking forward to hear your thoughts regarding this topic.
-- Christoph M. Becker

Sara Golemon

8 years ago
On Mon, Sep 11, 2017 at 8:36 AM, Christoph M. Becker <cmbecker69@gmx.de> wrote:
> I already have proposed to introduce bcinit() which would take a string, > int or float, and return a properly formatted string with the given or > default scale, see <https://github.com/php/php-src/pull/2746>. Nikita > suggested to accept the arguments allowed by `bcinit()` for all > functions in the first place. This would, however, raise the question > which scale should be applied to floats. >
I agree with Nikita. Given that current behavior for floats passed to bc*() obey EG(precision), I'd say we should retain that behavior in these explicit conversions. That way existing (functional) uses aren't impacted, we just quietly start working on other cases (where scientific notation and/or non-period delimiters would have been used).
> Before proceeding to an RFC (if necessary at all), I'm looking forward > to hear your thoughts regarding this topic. >
IMO, this falls under fixing an existing bug (bc*() functions don't accept certain types of floats), so it wouldn't need an RFC to be accepted. I would wait a little bit to get feedback on the scale question before merging though. I would even offer that we could apply this to 7.[012] safely. -Sara

Christoph Becker

8 years ago
On 11.09.2017 at 19:13, Sara Golemon wrote:
> On Mon, Sep 11, 2017 at 8:36 AM, Christoph M. Becker <cmbecker69@gmx.de> wrote: >> I already have proposed to introduce bcinit() which would take a string, >> int or float, and return a properly formatted string with the given or >> default scale, see <https://github.com/php/php-src/pull/2746>. Nikita >> suggested to accept the arguments allowed by `bcinit()` for all >> functions in the first place. This would, however, raise the question >> which scale should be applied to floats. > > I agree with Nikita. Given that current behavior for floats passed to > bc*() obey EG(precision), I'd say we should retain that behavior in > these explicit conversions. That way existing (functional) uses > aren't impacted, we just quietly start working on other cases (where > scientific notation and/or non-period delimiters would have been > used).
Okay, that makes sense.
>> Before proceeding to an RFC (if necessary at all), I'm looking forward >> to hear your thoughts regarding this topic. >> > IMO, this falls under fixing an existing bug (bc*() functions don't > accept certain types of floats), so it wouldn't need an RFC to be > accepted. I would wait a little bit to get feedback on the scale > question before merging though. I would even offer that we could > apply this to 7.[012] safely.
Hmm, the bc*() functions are documented to accept strings, and as such there is no actual bug; see also Mike's final assessment on <https://bugs.php.net/55160>. If we would consider this as bug, we also had to consider some other cases regarding our peculiar float to string casting behavior as bug. And even if we would, we still couldn't generally change the float to string cast for BC reasons easily (if ever). Anyhow, I'll gladly wait for further comments. :)
-- Christoph M. Becker