Skip to content

strconv: fix ParseFloat exponent and overflow bookkeeping #129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gaoflow wants to merge 2 commits into
base: master
Choose a base branch
from
Open

strconv: fix ParseFloat exponent and overflow bookkeeping #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2b06cf
strconv: fix ParseFloat exponent and overflow bookkeeping
gaoflow Jul 27, 2026
d6b77c8
strconv: restore the negative-exponent fast path in ParseDecimal
gaoflow Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions strconv/decimal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func ParseDecimal(b []byte) (float64, int) {
f := sign * float64(n)
if 0 <= exp && exp < 23 {
return f * float64pow10[exp], i
} else if 23 < exp && exp < 0 {
return f / float64pow10[exp], i

@tdewolff tdewolff Jul 27, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix this clause and do this like float, besides it moves more results closer according to your measurements.

@gaoflow gaoflow Jul 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — the dead 23 < exp && exp < 0 branch is now the real negative fast path, mirroring ParseFloat: -22 <= exp && exp < 0 divides by the exact table power instead of multiplying by math.Pow10's inexact reciprocal. Measured over 300k structured inputs: 12,664 results closer to the correctly-rounded value vs 448 farther (e.g. 0.3 now parses to exactly 3/10). Regression test added; d6b77c8.

} else if -22 <= exp && exp < 0 { // int / 10^k
return f / float64pow10[-exp], i
}
return f * math.Pow10(exp), i
}
Expand Down
17 changes: 17 additions & 0 deletions strconv/decimal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,23 @@ func TestParseDecimalError(t *testing.T) {
}
}

func TestParseDecimalNegativeFastPath(t *testing.T) {
// Dividing by an exact power of ten rounds differently from multiplying
// by math.Pow10's inexact reciprocal for some values (e.g. 3 * 0.1 != 3 / 10).
for _, tt := range []struct {
in string
want float64
}{
{"0.3", 0.3},
{"0.7", 0.7},
} {
f, n := ParseDecimal([]byte(tt.in))
if n != len(tt.in) || f != tt.want {
t.Errorf("%s: got %v (n=%d), want %v", tt.in, f, n, tt.want)
}
}
}

func TestAppendDecimal(t *testing.T) {
tests := []struct {
f float64
Expand Down
41 changes: 32 additions & 9 deletions strconv/float.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func ParseFloat(b []byte) (float64, int) {
c := b[i]
if '0' <= c && c <= '9' {
if trunk == -1 {
if math.MaxUint64/10 < n {
if math.MaxUint64/10 < n || math.MaxUint64-uint64(c-'0') < n*10 {
trunk = i
} else {
n *= 10
Expand All @@ -54,7 +54,12 @@ func ParseFloat(b []byte) (float64, int) {
if trunk == -1 {
trunk = i
}
mantExp = int64(trunk - dot - 1)
if trunk < dot {
// truncated before the dot, so no dot lies between trunk and dot
mantExp = int64(trunk - dot)
} else {
mantExp = int64(trunk - dot - 1)
}
} else if trunk != -1 {
mantExp = int64(trunk - i)
}
Expand All @@ -77,18 +82,36 @@ func ParseFloat(b []byte) (float64, int) {
} else if 0 < exp && exp <= 15+22 { // int * 10^k
// If exponent is big but number of digits is not,
// can move a few zeros into the integer part.
if 22 < exp {
f *= float64pow10[exp-22]
exp = 22
// Scale a copy, the fall-through below rescales f from scratch.
g, gexp := f, exp
if 22 < gexp {
g *= float64pow10[gexp-22]
gexp = 22
}
if -1e15 <= f && f <= 1e15 {
return f * float64pow10[exp], i
if -1e15 <= g && g <= 1e15 {
return g * float64pow10[gexp], i
}
} else if -22 <= exp && exp < 0 { // int / 10^k
return f / float64pow10[-exp], i
}
f *= math.Pow10(int(-mantExp))
return f * math.Pow10(int(expExp)), i
if f == 0.0 {
// 0 * math.Pow10(out of range) is NaN
return f, i
}
h := f * math.Pow10(int(-mantExp))
h *= math.Pow10(int(expExp))

@tdewolff tdewolff Jul 27, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can both Pow10's be merged?

@gaoflow gaoflow Jul 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They could be merged mathematically, but the two-step is deliberate: for inputs like 1e-289 one of the factors stays inside math.Pow10's [-323,308] domain while the combined exponent does not. I measured the one-step variant — it turns 11,118 currently bit-exact results 1 ulp wrong — so the fix keeps the two-step and only re-scales when the result degenerates to 0/Inf.

if h == 0.0 || math.IsInf(h, 0) {

@tdewolff tdewolff Jul 27, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we check for 0.0 above with f==0.0?

@gaoflow gaoflow Jul 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both checks are needed: f == 0.0 catches a zero mantissa, but h == 0.0 also fires when f != 0 and the 10^-mantExp factor underflows (e.g. 5e-324 → 5 * 10^0 * 10^-324 = 0), which is exactly the case the re-scale rescues.

// either factor alone can leave math.Pow10's [-323,308] domain
if exp < -308 {
f *= math.Pow10(-308)
exp += 308
} else if 308 < exp {
f *= math.Pow10(308)
exp -= 308
}
h = f * math.Pow10(int(exp))
}
return h, i
}

const log2 = 0.3010299956639812
Expand Down
91 changes: 91 additions & 0 deletions strconv/float_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"math"
"math/rand"
"strconv"
"strings"
"testing"

"github.com/tdewolff/test"
Expand All @@ -23,6 +24,26 @@ func TestParseFloat(t *testing.T) {
{"0.0e1", 0.0e1},
{"18446744073709551620", 18446744073709551620.0},
{"1e23", 1e23},
// mantissa truncated in the integer part, with a dot
{"123456789123456789123.5", 1.234567891234568e+20},
{"123456789123456789123.", 1.234567891234568e+20},
{"-123456789123456789123.5", -1.234567891234568e+20},
{"123456789123456789123.5e3", 1.234567891234568e+23},
{"99999999999999999999.0", 1e20},
{"1234567891234567891234.25", 1.234567891234568e+21},
// uint64 accumulator at its limit
{"18446744073709551615", 18446744073709551615.0},
{"18446744073709551616", 1.8446744073709552e+19},
{"18446744073709551616.5", 1.8446744073709552e+19},
{"-18446744073709551616", -1.8446744073709552e+19},
{"184467440737095516165", 1.844674407370955e+20},
// 15 digits with 22 < exp, so the int*10^k path bails out
{"993349238352373e23", 9.93349238352373e+37},
// math.Pow10 is 0 below 1e-323 and infinite above 1e308
{"0.0000000000000000000001610832207361e330", 1.610832207361e+308},
{"5e-324", 5e-324},
{"9e-324", 1e-323},
{"0e309", 0},
// TODO: hard to test due to float imprecision
// {"1.7976931348623e+308", 1.7976931348623e+308)
// {"4.9406564584124e-308", 4.9406564584124e-308)
Expand All @@ -36,6 +57,30 @@ func TestParseFloat(t *testing.T) {
}
}

// Literals that need scaling in more than one step, where a single math.Pow10
// leaves its domain. Compared against the stdlib within a few ULP.
func TestParseFloatScaling(t *testing.T) {
floatTests := []string{
"1" + strings.Repeat("0", 41) + "e-330",
"1" + strings.Repeat("0", 60) + "e-360",
"0." + strings.Repeat("0", 100) + "1e400",
"0." + strings.Repeat("0", 200) + "5e450",
"-1" + strings.Repeat("0", 41) + "e-330",
"-0e309",
}
for _, tt := range floatTests {
t.Run(fmt.Sprint(len(tt)), func(t *testing.T) {
expected, err := strconv.ParseFloat(tt, 64)
test.Error(t, err)
f, n := ParseFloat([]byte(tt))
test.T(t, n, len(tt))
if math.Abs(f-expected) > 1e-14*math.Abs(expected) {
test.Fail(t, fmt.Sprintf("%v != %v for %v", f, expected, tt))
}
})
}
}

func TestParseFloatError(t *testing.T) {
floatTests := []struct {
f string
Expand Down Expand Up @@ -177,6 +222,52 @@ func FuzzAppendFloat(f *testing.F) {

////////////////////////////////////////////////////////////////

// ParseFloat keeps at most 19-20 significant digits, so it is not bit-exact
// against the stdlib, but it must stay within a few ULP of it.
func TestParseFloatRandom(t *testing.T) {
N := int(2e5)
if testing.Short() {
N = int(1e4)
}
r := rand.New(rand.NewSource(6))
bad := 0
for i := 0; i < N; i++ {
var sb strings.Builder
if r.Intn(4) == 0 {
sb.WriteByte('-')
}
// vary the integer length across the uint64 accumulator limit
sb.WriteByte(byte('1' + r.Intn(9)))
for j := 1 + r.Intn(30); 0 < j; j-- {
sb.WriteByte(byte('0' + r.Intn(10)))
}
if frac := r.Intn(14); 0 < frac || r.Intn(4) == 0 {
sb.WriteByte('.')
for ; 0 < frac; frac-- {
sb.WriteByte(byte('0' + r.Intn(10)))
}
}
if r.Intn(2) == 0 {
sb.WriteString("e")
sb.WriteString(strconv.Itoa(r.Intn(700) - 350))
}
s := sb.String()
expected, err := strconv.ParseFloat(s, 64)
if err != nil || math.IsInf(expected, 0) || math.Abs(expected) < 1e-280 {
continue // below 1e-280 scaling runs into subnormals
}
f, n := ParseFloat([]byte(s))
if n != len(s) || math.Abs(f-expected) > 1e-14*math.Abs(expected) {
if bad++; bad < 10 {
t.Errorf("ParseFloat(%v): %v (%v bytes) != %v", s, f, n, expected)
}
}
}
if 0 < bad {
t.Errorf("%v of %v literals differ from strconv.ParseFloat", bad, N)
}
}

func TestAppendFloatRandom(t *testing.T) {
N := int(1e6)
if testing.Short() {
Expand Down