internal/number: fix rounding issues - rounding with large scales - correct interpretation for engineering - default rounding is exact representation - simplify increments Change-Id: I6a4959974069f5e1706e396a836bd6989b93d60d Reviewed-on: https://go-review.googlesource.com/60350 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 0491c97..8d80a92 100644 --- a/internal/number/decimal.go +++ b/internal/number/decimal.go
@@ -375,6 +375,24 @@ d.NaN = true return } + // Simple case: decimal notation + if r.Increment > 0 { + scale := int(r.MaxFractionDigits) + mult := 1.0 + if scale > len(scales) { + mult = math.Pow(10, float64(scale)) + } else { + mult = scales[scale] + } + // We multiply x instead of dividing inc as it gives less rounding + // issues. + x *= mult + x /= float64(r.Increment) + x = r.Mode.roundFloat(x) + x *= float64(r.Increment) + x /= mult + } + abs := x if x < 0 { d.Neg = true @@ -384,64 +402,64 @@ d.Inf = true return } - // Simple case: decimal notation - scale := r.scale() - prec := r.precision() - if scale > 0 || r.Increment > 0 || prec == 0 { - if scale > len(scales) { - x *= math.Pow(10, float64(scale)) - } else { - x *= scales[scale] - } - if r.Increment > 0 { - inc := float64(r.Increment) - x /= float64(inc) - x = r.Mode.roundFloat(x) - x *= inc - } else { - x = r.Mode.roundFloat(x) - } - d.fillIntDigits(uint64(math.Abs(x))) - d.Exp = int32(len(d.Digits) - scale) - return - } - // Nasty case (for non-decimal notation). - // Asides from being inefficient, this result is also wrong as it will - // apply ToNearestEven rounding regardless of the user setting. - // TODO: expose functionality in strconv so we can avoid this hack. + // By default we get the exact decimal representation. + verb := byte('g') + prec := -1 + // Determine rounding, if possible. As the strconv API does not return the + // rounding accuracy (exact/rounded up|down), we can only round using + // ToNearestEven. // Something like this would work: // AppendDigits(dst []byte, x float64, base, size, prec int) (digits []byte, exp, accuracy int) - // TODO: This only supports the nearest even rounding mode. - - if prec > 0 { - prec-- + if r.Mode == ToNearestEven { + // We can't round if limitations are placed on both the fraction and + // significant digits. + if r.MaxFractionDigits == 0 && r.MaxSignificantDigits > 0 { + prec = int(r.MaxSignificantDigits) + } else if r.isScientific() { + if r.MaxIntegerDigits == 1 && (r.MaxSignificantDigits == 0 || + int(r.MaxFractionDigits+1) == int(r.MaxSignificantDigits)) { + verb = 'e' + prec = int(r.MaxFractionDigits) + prec += int(r.DigitShift) + } + } else if r.MaxFractionDigits > 0 && r.MaxSignificantDigits == 0 { + verb = 'f' + prec = int(r.MaxFractionDigits) + } } - b := strconv.AppendFloat(d.Digits, abs, 'e', prec, size) + + b := strconv.AppendFloat(d.Digits[:0], abs, verb, prec, size) i := 0 k := 0 - // No need to check i < len(b) as we always have an 'e'. - for { + beforeDot := 1 + for i < len(b) { if c := b[i]; '0' <= c && c <= '9' { b[k] = c - '0' k++ - } else if c != '.' { + d.Exp += int32(beforeDot) + } else if c == '.' { + beforeDot = 0 + d.Exp = int32(k) + } else { break } i++ } d.Digits = b[:k] - i += len("e") - pSign := i - exp := 0 - for i++; i < len(b); i++ { - exp *= 10 - exp += int(b[i] - '0') + if i != len(b) { + i += len("e") + pSign := i + exp := 0 + for i++; i < len(b); i++ { + exp *= 10 + exp += int(b[i] - '0') + } + if b[pSign] == '-' { + exp = -exp + } + d.Exp = int32(exp) + 1 } - if b[pSign] == '-' { - exp = -exp - } - d.Exp = int32(exp) + 1 } func (d *Decimal) fillIntDigits(x uint64) {
diff --git a/internal/number/decimal_test.go b/internal/number/decimal_test.go index b017b39..ecb695a 100644 --- a/internal/number/decimal_test.go +++ b/internal/number/decimal_test.go
@@ -265,16 +265,15 @@ {uint32(234), scale2, "234"}, {uint64(234), scale2, "234"}, {uint(234), scale2, "234"}, - {-0.001, scale2, "-0"}, + {-0.001, scale2, "-0.00"}, // not normalized {-1e9, scale2, "-1000000000.00"}, - {0.234, scale2, "0.23"}, - {0.234, scale2away, "0.24"}, + {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"}, + {0.025, inc0_05, "0.00"}, // not normalized {0.075, inc0_05, "0.10"}, {325, inc50, "300"}, {375, inc50, "400"},
diff --git a/internal/number/format.go b/internal/number/format.go index 44116fa..4104789 100755 --- a/internal/number/format.go +++ b/internal/number/format.go
@@ -56,12 +56,16 @@ // given language. func (f *Formatter) InitScientific(t language.Tag) { f.init(t, tagToScientific) + f.Pattern.MinFractionDigits = 0 + f.Pattern.MaxFractionDigits = -1 } // InitEngineering initializes a Formatter using the default Pattern for the // given language. func (f *Formatter) InitEngineering(t language.Tag) { f.init(t, tagToScientific) + f.Pattern.MinFractionDigits = 0 + f.Pattern.MaxFractionDigits = -1 f.Pattern.MaxIntegerDigits = 3 f.Pattern.MinIntegerDigits = 1 } @@ -303,13 +307,6 @@ if numInt == 0 { numInt = 1 } - maxSig := int(r.MaxFractionDigits) + numInt - minSig := int(r.MinFractionDigits) + numInt - - if maxSig > 0 { - // TODO: really round to zero? - n.round(ToZero, maxSig) - } // If a maximum number of integers is specified, the minimum must be 1 // and the exponent is grouped by this number (e.g. for engineering) @@ -326,10 +323,14 @@ numInt += d } + if maxSig := int(r.MaxFractionDigits); maxSig >= 0 { + n.round(r.Mode, maxSig+numInt) + } + n.Comma = uint8(numInt) n.End = int32(len(n.Digits)) - if n.End < int32(minSig) { - n.End = int32(minSig) + if minSig := int32(r.MinFractionDigits) + int32(numInt); n.End < minSig { + n.End = minSig } return n }
diff --git a/internal/number/format_test.go b/internal/number/format_test.go index fc7c36d..1273818 100755 --- a/internal/number/format_test.go +++ b/internal/number/format_test.go
@@ -289,22 +289,25 @@ pattern: "##0E00", test: pairs{ "100": "100\u202f×\u202f10⁰⁰", - "12345": "10\u202f×\u202f10⁰³", - "123.456": "100\u202f×\u202f10⁰⁰", + "12345": "12\u202f×\u202f10⁰³", + "123.456": "123\u202f×\u202f10⁰⁰", }, }, { pattern: "##0.###E00", test: pairs{ - "100": "100\u202f×\u202f10⁰⁰", - "12345": "12.34\u202f×\u202f10⁰³", - "123.456": "123.4\u202f×\u202f10⁰⁰", + "100": "100\u202f×\u202f10⁰⁰", + "12345": "12.345\u202f×\u202f10⁰³", + "123456": "123.456\u202f×\u202f10⁰³", + "123.456": "123.456\u202f×\u202f10⁰⁰", + "123.4567": "123.457\u202f×\u202f10⁰⁰", }, }, { pattern: "##0.000E00", test: pairs{ - "100": "100.0\u202f×\u202f10⁰⁰", - "12345": "12.34\u202f×\u202f10⁰³", - "123.456": "123.4\u202f×\u202f10⁰⁰", + "100": "100.000\u202f×\u202f10⁰⁰", + "12345": "12.345\u202f×\u202f10⁰³", + "123.456": "123.456\u202f×\u202f10⁰⁰", + "12.3456": "12.346\u202f×\u202f10⁰⁰", }, }, { pattern: "@@E0", @@ -498,7 +501,8 @@ }{ {f.InitDecimal, "123456.78", "123,456.78"}, {f.InitScientific, "123456.78", "1.23\u202f×\u202f10⁵"}, - {f.InitEngineering, "123456.78", "123\u202f×\u202f10³"}, + {f.InitEngineering, "123456.78", "123.46\u202f×\u202f10³"}, + {f.InitEngineering, "1234", "1.23\u202f×\u202f10³"}, {f.InitPercent, "0.1234", "12.34%"}, {f.InitPerMille, "0.1234", "123.40‰"},
diff --git a/internal/number/pattern.go b/internal/number/pattern.go index 89c2a1c..4c3e815 100644 --- a/internal/number/pattern.go +++ b/internal/number/pattern.go
@@ -55,11 +55,11 @@ // It contains all information needed to determine the "visible digits" as // required by the pluralization rules. type RoundingContext struct { - Increment uint32 // if > 0, round to Increment * 10^-scale() + Increment uint32 // TODO: unify these two fields so that there is a more unambiguous meaning // of how precision is handled. - MaxSignificantDigits int16 // -1 is infinite precision - MaxFractionDigits uint16 + MaxSignificantDigits int16 // -1 is unlimited + MaxFractionDigits int16 // -1 is unlimited Mode RoundingMode @@ -88,7 +88,7 @@ // SetScale fixes the RoundingContext to a fixed number of fraction digits. func (r *RoundingContext) SetScale(scale int) { r.MinFractionDigits = uint8(scale) - r.MaxFractionDigits = uint16(scale) + r.MaxFractionDigits = int16(scale) } func (r *RoundingContext) SetPrecision(prec int) { @@ -414,7 +414,7 @@ func (p *parser) normalizeSigDigitsWithExponent() state { p.MinIntegerDigits, p.MaxIntegerDigits = 1, 1 p.MinFractionDigits = p.MinSignificantDigits - 1 - p.MaxFractionDigits = uint16(p.MaxSignificantDigits) - 1 + p.MaxFractionDigits = p.MaxSignificantDigits - 1 p.MinSignificantDigits, p.MaxSignificantDigits = 0, 0 return p.exponent }
diff --git a/internal/number/pattern_test.go b/internal/number/pattern_test.go index f551456..af08379 100644 --- a/internal/number/pattern_test.go +++ b/internal/number/pattern_test.go
@@ -154,6 +154,17 @@ }, }, }, { + "##0.###E00", + &Pattern{ + FormatWidth: 10, + RoundingContext: RoundingContext{ + MinIntegerDigits: 1, + MaxIntegerDigits: 3, + MaxFractionDigits: 3, + MinExponentDigits: 2, + }, + }, +}, { "##00.0#E0", &Pattern{ FormatWidth: 9,
diff --git a/message/fmt_test.go b/message/fmt_test.go index b22420d..2110bb5 100755 --- a/message/fmt_test.go +++ b/message/fmt_test.go
@@ -475,10 +475,8 @@ {"%.4b", float32(1.0), "8388608p-23"}, {"%.4b", -1.0, "-4503599627370496p-52"}, // Test correct f.intbuf boundary checks. - // TODO: the following cases won't work because of rounding errors. We can - // fix this if we expose the internals of strconv. - // {"%.68f", 1.0, zeroFill("1.", 68, "")}, // TODO(bug): rounding error - // {"%.68f", -1.0, zeroFill("-1.", 68, "")}, // TODO(bug): rounding error + {"%.68f", 1.0, zeroFill("1.", 68, "")}, + {"%.68f", -1.0, zeroFill("-1.", 68, "")}, // float infinites and NaNs {"%f", posInf, "∞"}, {"%.1f", negInf, "-∞"},
diff --git a/message/print.go b/message/print.go index 7e4642b..07feaa2 100644 --- a/message/print.go +++ b/message/print.go
@@ -284,7 +284,7 @@ f.MinIntegerDigits = 1 f.MaxIntegerDigits = 0 f.MinFractionDigits = uint8(minFrac) - f.MaxFractionDigits = uint16(maxFrac) + f.MaxFractionDigits = int16(maxFrac) p.setFlags(f) f.PadRune = 0 if p.fmt.widPresent { @@ -313,7 +313,7 @@ } else { f.SetPrecision(maxFrac + 1) f.MinFractionDigits = uint8(minFrac) - f.MaxFractionDigits = uint16(maxFrac) + f.MaxFractionDigits = int16(maxFrac) } f.MinExponentDigits = 2 p.setFlags(f)