internal/number: fix more rounding issues
Removed AppendFloat-based rounding to
not hit bug golang/go#21714.
Fixed several edge cases and implemented some
missing features that were exposed as a result
of having to fall back on our own rounding.
This also fixed some previously unnoticed bugs.
Change-Id: Id675bdc9a9099eda0b9c31e5166b510b714887b5
Reviewed-on: https://go-review.googlesource.com/60730
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Nigel Tao <nigeltao@golang.org>
diff --git a/internal/number/decimal.go b/internal/number/decimal.go
index ac2098d..62074e7 100644
--- a/internal/number/decimal.go
+++ b/internal/number/decimal.go
@@ -411,14 +411,18 @@
// ToNearestEven.
// Something like this would work:
// AppendDigits(dst []byte, x float64, base, size, prec int) (digits []byte, exp, accuracy int)
- if r.Mode == ToNearestEven {
- if n := r.RoundSignificantDigits(); n >= 0 {
- prec = n
- } else if n = r.RoundFractionDigits(); n >= 0 {
- prec = n
- verb = 'f'
- }
- }
+ //
+ // TODO: At this point strconv's rounding is imprecise to the point that it
+ // is not useable for this purpose.
+ // See https://github.com/golang/go/issues/21714
+ // if r.Mode == ToNearestEven {
+ // if n := r.RoundSignificantDigits(); n >= 0 {
+ // prec = n
+ // } else if n = r.RoundFractionDigits(); n >= 0 {
+ // prec = n
+ // verb = 'f'
+ // }
+ // }
b := strconv.AppendFloat(d.Digits[:0], abs, verb, prec, size)
i := 0
diff --git a/internal/number/decimal_test.go b/internal/number/decimal_test.go
index f13b7c1..04aa8b2 100644
--- a/internal/number/decimal_test.go
+++ b/internal/number/decimal_test.go
@@ -256,6 +256,13 @@
rc RoundingContext
out string
}{
+ // TODO: uncommented tests can be restored when convert does its own
+ // rounding.
+ // {-0.001, scale2, "-0.00"}, // not normalized
+ // {0.1234, prec3, "0.123"},
+ // {1234.0, prec3, "1230"},
+ // {1.2345e10, prec3, "12300000000"},
+
{int8(-34), scale2, "-34"},
{int16(-234), scale2, "-234"},
{int32(-234), scale2, "-234"},
@@ -266,22 +273,18 @@
{uint32(234), scale2, "234"},
{uint64(234), scale2, "234"},
{uint(234), scale2, "234"},
- {-0.001, scale2, "-0.00"}, // not normalized
- {-1e9, scale2, "-1000000000.00"},
+ {-1e9, scale2, "-1000000000"},
{0.234, scale2away, "0.234"}, // rounding postponed as not ToNearestEven
- {0.1234, prec3, "0.123"},
- {1234.0, prec3, "1230"},
- {1.2345e10, prec3, "12300000000"},
{0.03, inc0_05, "0.05"},
- {0.025, inc0_05, "0.00"}, // not normalized
- {0.075, inc0_05, "0.10"},
+ {0.025, inc0_05, "0"},
+ {0.075, inc0_05, "0.1"},
{325, inc50, "300"},
{375, inc50, "400"},
// Here the scale is 2, but the digits get shifted left. As we use
// AppendFloat to do the rounding an exta 0 gets added.
- {0.123, roundShift, "0.1230"},
+ {0.123, roundShift, "0.123"},
{converter(3), scale2, "100"},
diff --git a/internal/number/format.go b/internal/number/format.go
index 4104789..0a5ffb5 100755
--- a/internal/number/format.go
+++ b/internal/number/format.go
@@ -154,48 +154,47 @@
}
n := Digits{digits: d.normalize().digits}
- if maxSig := int(r.MaxSignificantDigits); maxSig > 0 {
- // TODO: really round to zero?
- n.round(ToZero, maxSig)
- }
- digits := n.Digits
exp := n.Exp
exp += int32(r.DigitShift)
// Cap integer digits. Remove *most-significant* digits.
if r.MaxIntegerDigits > 0 {
if p := int(exp) - int(r.MaxIntegerDigits); p > 0 {
- if p > len(digits) {
- p = len(digits)
+ if p > len(n.Digits) {
+ p = len(n.Digits)
}
- if digits = digits[p:]; len(digits) == 0 {
+ if n.Digits = n.Digits[p:]; len(n.Digits) == 0 {
exp = 0
} else {
exp -= int32(p)
}
// Strip leading zeros.
- for len(digits) > 0 && digits[0] == 0 {
- digits = digits[1:]
+ for len(n.Digits) > 0 && n.Digits[0] == 0 {
+ n.Digits = n.Digits[1:]
exp--
}
}
}
- // Rounding usually is done by convert, but we don't rely on it.
- numFrac := len(digits) - int(exp)
- if r.MaxSignificantDigits == 0 && int(r.MaxFractionDigits) < numFrac {
- p := int(exp) + int(r.MaxFractionDigits)
- if p <= 0 {
- p = 0
- } else if p >= len(digits) {
- p = len(digits)
- }
- digits = digits[:p] // TODO: round
+ // Rounding if not already done by Convert.
+ p := len(n.Digits)
+ if maxSig := int(r.MaxSignificantDigits); maxSig > 0 {
+ p = maxSig
}
+ if maxFrac := int(r.MaxFractionDigits); maxFrac >= 0 {
+ if cap := int(exp) + maxFrac; cap < p {
+ p = int(exp) + maxFrac
+ }
+ if p < 0 {
+ p = 0
+ }
+ }
+ n.round(r.Mode, p)
// set End (trailing zeros)
- n.End = int32(len(digits))
- if len(digits) == 0 {
+ n.End = int32(len(n.Digits))
+ if n.End == 0 {
+ exp = 0
if r.MinFractionDigits > 0 {
n.End = int32(r.MinFractionDigits)
}
@@ -210,7 +209,6 @@
n.End = int32(r.MinSignificantDigits)
}
}
- n.Digits = digits
n.Exp = exp
return n
}
@@ -323,9 +321,14 @@
numInt += d
}
- if maxSig := int(r.MaxFractionDigits); maxSig >= 0 {
- n.round(r.Mode, maxSig+numInt)
+ p := len(n.Digits)
+ if maxSig := int(r.MaxSignificantDigits); maxSig > 0 {
+ p = maxSig
}
+ if maxFrac := int(r.MaxFractionDigits); maxFrac >= 0 && numInt+maxFrac < p {
+ p = numInt + maxFrac
+ }
+ n.round(r.Mode, p)
n.Comma = uint8(numInt)
n.End = int32(len(n.Digits))
diff --git a/internal/number/format_test.go b/internal/number/format_test.go
index 1273818..01a0894 100755
--- a/internal/number/format_test.go
+++ b/internal/number/format_test.go
@@ -112,7 +112,7 @@
test: pairs{
"0": "0",
"1234.5678": "1234.5678",
- "0.123456789": "0.123456",
+ "0.123456789": "0.123457",
"NaN": "NaN",
"Inf": "∞",
},
@@ -142,7 +142,7 @@
pattern: "#,##0.###",
test: pairs{
"0": "0",
- "1234.5678": "1,234.567",
+ "1234.5678": "1,234.568",
"0.123456789": "0.123",
},
}, {
@@ -157,7 +157,7 @@
test: pairs{
"0": "0,00,000",
"123456789012": "1,23,45,67,89,012",
- "12.3456789": "0,00,012.345",
+ "12.3456789": "0,00,012.346",
"0.123456789": "0,00,000.123",
},
@@ -245,7 +245,7 @@
pattern: "@@##",
test: pairs{
"1": "1.0",
- "0.1": "0.10",
+ "0.1": "0.10", // leading zero does not count as significant digit
"123": "123",
"1234": "1234",
"12345": "12340",
diff --git a/internal/number/pattern.go b/internal/number/pattern.go
index eebaafd..b95ca40 100644
--- a/internal/number/pattern.go
+++ b/internal/number/pattern.go
@@ -358,6 +358,7 @@
case '@':
p.groupingCount++
p.leadingSharps = 0
+ p.MaxFractionDigits = -1
return p.sigDigits(r)
case ',':
if p.leadingSharps == 0 { // no leading commas
diff --git a/internal/number/pattern_test.go b/internal/number/pattern_test.go
index 3a0b2be..a7517d0 100644
--- a/internal/number/pattern_test.go
+++ b/internal/number/pattern_test.go
@@ -203,6 +203,7 @@
RoundingContext: RoundingContext{
MinSignificantDigits: 1,
MaxSignificantDigits: 1,
+ MaxFractionDigits: -1,
},
},
}, {
@@ -213,6 +214,7 @@
RoundingContext: RoundingContext{
MinSignificantDigits: 4,
MaxSignificantDigits: 4,
+ MaxFractionDigits: -1,
},
},
}, {
@@ -222,6 +224,7 @@
RoundingContext: RoundingContext{
MinSignificantDigits: 1,
MaxSignificantDigits: 4,
+ MaxFractionDigits: -1,
},
},
}, {
