Skip to content

fix parseNumber returning zero for finite values with negative exponent#2223

Open
sage-mode-hunter wants to merge 1 commit into
bblanchon:7.xfrom
sage-mode-hunter:parsenumber-negative-exponent
Open

fix parseNumber returning zero for finite values with negative exponent#2223
sage-mode-hunter wants to merge 1 commit into
bblanchon:7.xfrom
sage-mode-hunter:parsenumber-negative-exponent

Conversation

@sage-mode-hunter

Copy link
Copy Markdown

Reading a numeric string as a number, for example doc["x"].as<double>() on
the value "1" followed by 300 zeros and e-30 (which is 1e270), returns 0.

  1. the exponent overflow guard adds the parsed exponent to exponent_offset even when the exponent is negative, so a large positive offset combined with a negative exponent trips the guard and yields 0 instead of scaling the mantissa back down.
  2. saturating at exponent_max is also too tight once the mantissa carries extra digits, which is why the check now uses the reach of the binary powers-of-ten table.

Compute the effective exponent with its real sign and only saturate to inf/0 past what the table can represent; multiplyByPowerOfTen() handles everything in range, including subnormals.

@bblanchon

Copy link
Copy Markdown
Owner

Hi @sage-mode-hunter,

Thank you for this contribution.

This is an interesting fix, but is it really useful?
How did you end up with a one followed by 300 zeros?
I understand that, in theory, this is a valid input, but so is a one followed by one billion zeros, so why bother?

I looked at your user profile, and I found it rather peculiar.
The account was created only 3 weeks ago and is opening numerous pull requests to unrelated projects.
It claims to work at a company that has absolutely zero online presence, except for an AI-generated website registered 3 weeks ago—what a coincidence!

All of this is very suspicious and makes me question the real intent of this PR.

Best regards,
Benoit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants