From 47282232b4368f36621f6e4a3b0445bcc42afb9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 3 Nov 2022 15:42:04 +0000 Subject: [PATCH] update to cockroachdb/apd v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main breaking change that apd/v3 brings is that it no longer uses math/big types directly. There are new wrapper types which avoid allocations in many common cases. See the added calls to MathBigInt and SetMathBigInt. The other breaking change happened in Quo and Sqrt, causing the operations in CUE to end up with different precision: === RUN TestMarshalJSON === RUN TestMarshalJSON/22/a:_1.0/1 types_test.go:2758: got {"a":1.00000000000000000000000}; want {"a":1.0} --- FAIL: TestMarshalJSON (0.00s) --- FAIL: TestMarshalJSON/22/a:_1.0/1 (0.00s) Get the old behavior back by overwriting the Quo and Sqrt methods, running a short piece of code to do what v2 used to do. This makes the unit tests pass again. As a small bonus, whenever we use Quo or Rem on an apd float, the resulting Decimal is now consistently reduced, leaving at least one digit after the decimal point. For example, cue/testdata/resolve/006_arithmetic.txtar worked out "4.0 / 2.0" as "2", but now it is "2.0". Similarly, cue/testdata/basicrewrite/002_arithmetic.txtar worked out "1.0T / 2.0" as "5.0000000000E+11", but now it is "5.0E+11". To properly change the behavior of Quo and Rem across the codebase, internal.BaseContext is now of type internal.Context, which embeds apd.Context. internal.BaseContext is now used everywhere, except for cue/literal, as that would result in cyclic imports. For now that is fine, since that package uses neither Quo nor Rem. If we want cue/literal to use the same custom apd.Context as the rest of the codebase, we could always move internal.Context to avoid the cycle. Performance is largely unchanged; there are likely more gains to be made if we reduce the amount of times we convert to math/big types. goos: linux goarch: amd64 pkg: cuelang.org/go/cue/testdata/benchmarks cpu: AMD Ryzen 7 PRO 5850U with Radeon Graphics │ apd-v2-24 │ apd-v3-24 │ │ sec/op │ sec/op vs base │ /decimal.txtar-8 4.982m ± 1% 5.045m ± 0% +1.27% (p=0.004 n=6) │ apd-v2-24 │ apd-v3-24 │ │ B/op │ B/op vs base │ /decimal.txtar-8 1.873Mi ± 0% 1.872Mi ± 0% -0.09% (p=0.002 n=6) │ apd-v2-24 │ apd-v3-24 │ │ allocs/op │ allocs/op vs base │ /decimal.txtar-8 29.72k ± 0% 29.65k ± 0% -0.22% (p=0.002 n=6) Fixes #1872. Signed-off-by: Daniel Martí Change-Id: Iadcb4a50f7c9943601a1124570387f62ed65a6f5 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/543421 TryBot-Result: CUEcueckoo Reviewed-by: Marcel van Lohuizen Unity-Result: CUE porcuepine --- cue/literal/num.go | 3 +- cue/path.go | 2 +- .../basicrewrite/002_arithmetic.txtar | 4 +- cue/testdata/benchmarks/decimal.txtar | 2 +- cue/testdata/resolve/006_arithmetic.txtar | 8 +-- cue/types.go | 15 +++--- encoding/openapi/types.go | 2 +- encoding/protobuf/jsonpb/decoder.go | 2 +- go.mod | 4 +- go.sum | 10 ++-- internal/core/adt/context.go | 2 +- internal/core/adt/decimal.go | 36 +++++-------- internal/core/adt/expr.go | 2 +- internal/core/adt/simplify.go | 16 +++--- internal/core/compile/label.go | 2 +- internal/core/convert/go.go | 12 +++-- internal/core/convert/go_test.go | 2 +- internal/core/export/bounds.go | 2 +- internal/internal.go | 54 +++++++++++++++++-- internal/pkg/context.go | 2 +- pkg/list/math.go | 2 +- pkg/math/manual.go | 21 ++++---- pkg/math/math.go | 26 +++++---- 23 files changed, 135 insertions(+), 96 deletions(-) diff --git a/cue/literal/num.go b/cue/literal/num.go index bb77d5b2f22..a225b4f5841 100644 --- a/cue/literal/num.go +++ b/cue/literal/num.go @@ -17,9 +17,10 @@ package literal import ( "cuelang.org/go/cue/errors" "cuelang.org/go/cue/token" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" ) +// We avoid cuelang.org/go/internal.Context as that would be an import cycle. var baseContext apd.Context func init() { diff --git a/cue/path.go b/cue/path.go index 75ea7792dc5..596c93494dc 100644 --- a/cue/path.go +++ b/cue/path.go @@ -27,7 +27,7 @@ import ( "cuelang.org/go/cue/token" "cuelang.org/go/internal/astinternal" "cuelang.org/go/internal/core/adt" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" ) // SelectorType represents the kind of a selector. It indicates both the label diff --git a/cue/testdata/basicrewrite/002_arithmetic.txtar b/cue/testdata/basicrewrite/002_arithmetic.txtar index 124d1e8eb51..b765ee0e92e 100644 --- a/cue/testdata/basicrewrite/002_arithmetic.txtar +++ b/cue/testdata/basicrewrite/002_arithmetic.txtar @@ -141,7 +141,7 @@ Result: sum: (int){ 1 } div1: (float){ 4.00000000000000000000000 } div2: (float){ 4.00000000000000000000000 } - div3: (float){ 1 } + div3: (float){ 1.0 } divZero: (_|_){ // [eval] divZero: failed arithmetic: division by zero: // ./in.cue:8:10 @@ -168,7 +168,7 @@ Result: // [eval] irem00: division by zero: // ./in.cue:16:9 } - v1: (float){ 5.0000000000E+11 } + v1: (float){ 5.0E+11 } v2: (bool){ true } v3: (float){ 0.666666666666666666666667 } v5: (int){ 0 } diff --git a/cue/testdata/benchmarks/decimal.txtar b/cue/testdata/benchmarks/decimal.txtar index adfdb77a34e..a62bade8079 100644 --- a/cue/testdata/benchmarks/decimal.txtar +++ b/cue/testdata/benchmarks/decimal.txtar @@ -59,7 +59,7 @@ Disjuncts: 11 4: (int){ 5 } 5: (int){ 6 } 6: (int){ 50 } - 7: (float){ 5 } + 7: (float){ 5.0 } 8: (float){ 0.333333333333333333333333 } } } diff --git a/cue/testdata/resolve/006_arithmetic.txtar b/cue/testdata/resolve/006_arithmetic.txtar index 0e9c3bbe3d1..03eda1241d9 100644 --- a/cue/testdata/resolve/006_arithmetic.txtar +++ b/cue/testdata/resolve/006_arithmetic.txtar @@ -40,20 +40,20 @@ Conjuncts: 8 Disjuncts: 7 -- out/eval -- Errors: -e2: conflicting values int and 2 (mismatched types int and float): +e2: conflicting values int and 2.0 (mismatched types int and float): ./in.cue:6:5 ./in.cue:6:11 Result: (_|_){ // [eval] - v1: (float){ 5.0000000000E+11 } + v1: (float){ 5.0E+11 } v2: (bool){ true } n1: (int){ 1 } v5: (float){ 2.0 } - v6: (float){ 1 } + v6: (float){ 1.0 } e2: (_|_){ - // [eval] e2: conflicting values int and 2 (mismatched types int and float): + // [eval] e2: conflicting values int and 2.0 (mismatched types int and float): // ./in.cue:6:5 // ./in.cue:6:11 } diff --git a/cue/types.go b/cue/types.go index 6b2472dd24b..0fab5d40965 100644 --- a/cue/types.go +++ b/cue/types.go @@ -23,7 +23,7 @@ import ( "math/big" "strings" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/cue/ast" "cuelang.org/go/cue/build" @@ -335,7 +335,7 @@ func (v Value) MantExp(mant *big.Int) (exp int, err error) { return 0, ErrInfinite } if mant != nil { - mant.Set(&n.X.Coeff) + mant.Set(n.X.Coeff.MathBigInt()) if n.X.Negative { mant.Neg(mant) } @@ -371,7 +371,7 @@ func (v Value) AppendFloat(buf []byte, fmt byte, prec int) ([]byte, error) { if err != nil { return nil, err } - ctx := apd.BaseContext + ctx := internal.BaseContext nd := int(apd.NumDigits(&n.X.Coeff)) + int(n.X.Exponent) if n.X.Form == apd.Infinite { if n.X.Negative { @@ -380,9 +380,9 @@ func (v Value) AppendFloat(buf []byte, fmt byte, prec int) ([]byte, error) { return append(buf, string('∞')...), nil } if fmt == 'f' && nd > 0 { - ctx.Precision = uint32(nd + prec) + ctx = ctx.WithPrecision(uint32(nd + prec)) } else { - ctx.Precision = uint32(prec) + ctx = ctx.WithPrecision(uint32(prec)) } var d apd.Decimal ctx.Round(&d, &n.X) @@ -415,7 +415,7 @@ func (v Value) Int(z *big.Int) (*big.Int, error) { if n.X.Exponent != 0 { panic("cue: exponent should always be nil for integer types") } - z.Set(&n.X.Coeff) + z.Set(n.X.Coeff.MathBigInt()) if n.X.Negative { z.Neg(z) } @@ -494,8 +494,7 @@ func init() { // math.MaxFloat64: 2**1023 * (2**53 - 1) / 2**52 max = "1.797693134862315708145274237317043567981e+308" ) - ctx := apd.BaseContext - ctx.Precision = 40 + ctx := internal.BaseContext.WithPrecision(40) var err error smallestPosFloat64, _, err = ctx.NewFromString(smallest) diff --git a/encoding/openapi/types.go b/encoding/openapi/types.go index 84a5251c60e..6554c9683f9 100644 --- a/encoding/openapi/types.go +++ b/encoding/openapi/types.go @@ -17,7 +17,7 @@ package openapi import ( "fmt" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/cue" "cuelang.org/go/cue/ast" diff --git a/encoding/protobuf/jsonpb/decoder.go b/encoding/protobuf/jsonpb/decoder.go index 0d7117ff925..ad4ecc39487 100644 --- a/encoding/protobuf/jsonpb/decoder.go +++ b/encoding/protobuf/jsonpb/decoder.go @@ -25,7 +25,7 @@ import ( "cuelang.org/go/cue/literal" "cuelang.org/go/cue/token" "cuelang.org/go/encoding/protobuf/pbinternal" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" ) // Option is an option. diff --git a/go.mod b/go.mod index ad856e0b464..82a4065dc10 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module cuelang.org/go go 1.19 require ( - github.com/cockroachdb/apd/v2 v2.0.2 + github.com/cockroachdb/apd/v3 v3.2.0 github.com/emicklei/proto v1.10.0 github.com/go-quicktest/qt v1.100.0 github.com/google/go-cmp v0.5.9 @@ -27,9 +27,7 @@ require ( require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kr/text v0.2.0 // indirect - github.com/lib/pq v1.0.0 // indirect github.com/mitchellh/go-wordwrap v1.0.1 // indirect - github.com/pkg/errors v0.8.1 // indirect golang.org/x/sys v0.6.0 // indirect gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect ) diff --git a/go.sum b/go.sum index dfeea4d8309..19fe9de1a6b 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/cockroachdb/apd/v2 v2.0.2 h1:weh8u7Cneje73dDh+2tEVLUvyBc89iwepWCD8b8034E= -github.com/cockroachdb/apd/v2 v2.0.2/go.mod h1:DDxRlzC2lo3/vSlmSoS7JkqbbrARPuFOGr0B9pvN3Gw= +github.com/cockroachdb/apd/v3 v3.2.0 h1:79kHCn4tO0VGu3W0WujYrMjBDk8a2H4KEUYcXf7whcg= +github.com/cockroachdb/apd/v3 v3.2.0/go.mod h1:klXJcjp+FffLTHlhIG69tezTDvdP065naDsHzKhYSqc= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/emicklei/proto v1.10.0 h1:pDGyFRVV5RvV+nkBK9iy3q67FBy9Xa7vwrOTE+g5aGw= @@ -20,16 +20,12 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= -github.com/lib/pq v1.0.0 h1:X5PMW56eZitiTeO7tKzZxFCSpbFZJtkMMooicw2us9A= -github.com/lib/pq v1.0.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= +github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw= github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0= github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0= github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de h1:D5x39vF5KCwKQaw+OC9ZPiLVHXz3UFw2+psEX+gYcto= github.com/mpvl/unique v0.0.0-20150818121801-cbe035fff7de/go.mod h1:kJun4WP5gFuHZgRjZUWWuH1DTxCtxbHDOIJsudS8jzY= github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= -github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= -github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= -github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0 h1:sadMIsgmHpEOGbUs6VtHBXRR1OHevnj7hLx9ZcdNGW4= github.com/protocolbuffers/txtpbfmt v0.0.0-20230328191034-3462fbc510c0/go.mod h1:jgxiZysxFPM+iWKwQwPR+y+Jvo54ARd4EisXxKYpB5c= github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index 86472d0de60..1e83674bf1e 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -23,7 +23,7 @@ import ( "sort" "strings" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "golang.org/x/text/encoding/unicode" "cuelang.org/go/cue/ast" diff --git a/internal/core/adt/decimal.go b/internal/core/adt/decimal.go index e7eba385664..4b10e28060d 100644 --- a/internal/core/adt/decimal.go +++ b/internal/core/adt/decimal.go @@ -15,18 +15,10 @@ package adt import ( - "math/big" - - "github.com/cockroachdb/apd/v2" + "cuelang.org/go/internal" + "github.com/cockroachdb/apd/v3" ) -var apdCtx apd.Context - -func init() { - apdCtx = apd.BaseContext - apdCtx.Precision = 24 -} - func (n *Num) Impl() *apd.Decimal { return &n.X } @@ -40,19 +32,19 @@ func (a *Num) Cmp(b *Num) int { } func (c *OpContext) Add(a, b *Num) Value { - return numOp(c, apdCtx.Add, a, b) + return numOp(c, internal.BaseContext.Add, a, b) } func (c *OpContext) Sub(a, b *Num) Value { - return numOp(c, apdCtx.Sub, a, b) + return numOp(c, internal.BaseContext.Sub, a, b) } func (c *OpContext) Mul(a, b *Num) Value { - return numOp(c, apdCtx.Mul, a, b) + return numOp(c, internal.BaseContext.Mul, a, b) } func (c *OpContext) Quo(a, b *Num) Value { - v := numOp(c, apdCtx.Quo, a, b) + v := numOp(c, internal.BaseContext.Quo, a, b) if n, ok := v.(*Num); ok { n.K = FloatKind } @@ -60,7 +52,7 @@ func (c *OpContext) Quo(a, b *Num) Value { } func (c *OpContext) Pow(a, b *Num) Value { - return numOp(c, apdCtx.Pow, a, b) + return numOp(c, internal.BaseContext.Pow, a, b) } type numFunc func(z, x, y *apd.Decimal) (apd.Condition, error) @@ -86,22 +78,22 @@ func numOp(c *OpContext, fn numFunc, x, y *Num) Value { } func (c *OpContext) IntDiv(a, b *Num) Value { - return intDivOp(c, (*big.Int).Div, a, b) + return intDivOp(c, (*apd.BigInt).Div, a, b) } func (c *OpContext) IntMod(a, b *Num) Value { - return intDivOp(c, (*big.Int).Mod, a, b) + return intDivOp(c, (*apd.BigInt).Mod, a, b) } func (c *OpContext) IntQuo(a, b *Num) Value { - return intDivOp(c, (*big.Int).Quo, a, b) + return intDivOp(c, (*apd.BigInt).Quo, a, b) } func (c *OpContext) IntRem(a, b *Num) Value { - return intDivOp(c, (*big.Int).Rem, a, b) + return intDivOp(c, (*apd.BigInt).Rem, a, b) } -type intFunc func(z, x, y *big.Int) *big.Int +type intFunc func(z, x, y *apd.BigInt) *apd.BigInt func intDivOp(c *OpContext, fn intFunc, a, b *Num) Value { if b.X.IsZero() { @@ -109,11 +101,11 @@ func intDivOp(c *OpContext, fn intFunc, a, b *Num) Value { } var x, y apd.Decimal - _, _ = apdCtx.RoundToIntegralValue(&x, &a.X) + _, _ = internal.BaseContext.RoundToIntegralValue(&x, &a.X) if x.Negative { x.Coeff.Neg(&x.Coeff) } - _, _ = apdCtx.RoundToIntegralValue(&y, &b.X) + _, _ = internal.BaseContext.RoundToIntegralValue(&y, &b.X) if y.Negative { y.Coeff.Neg(&y.Coeff) } diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index b138a816df9..a26bf7d5d91 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -20,7 +20,7 @@ import ( "io" "regexp" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/cue/ast" "cuelang.org/go/cue/errors" diff --git a/internal/core/adt/simplify.go b/internal/core/adt/simplify.go index 571800dba02..e1b38333493 100644 --- a/internal/core/adt/simplify.go +++ b/internal/core/adt/simplify.go @@ -15,7 +15,9 @@ package adt import ( - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" + + "cuelang.org/go/internal" ) // SimplifyBounds collapses bounds if possible. The bound values must be @@ -82,21 +84,21 @@ func SimplifyBounds(ctx *OpContext, k Kind, x, y *BoundValue) Value { // Readjust bounds for integers. if x.Op == GreaterEqualOp { // >=3.4 ==> >=4 - _, _ = apdCtx.Ceil(&lo, &a.X) + _, _ = internal.BaseContext.Ceil(&lo, &a.X) } else { // >3.4 ==> >3 - _, _ = apdCtx.Floor(&lo, &a.X) + _, _ = internal.BaseContext.Floor(&lo, &a.X) } if y.Op == LessEqualOp { // <=2.3 ==> <= 2 - _, _ = apdCtx.Floor(&hi, &b.X) + _, _ = internal.BaseContext.Floor(&hi, &b.X) } else { // <2.3 ==> < 3 - _, _ = apdCtx.Ceil(&hi, &b.X) + _, _ = internal.BaseContext.Ceil(&hi, &b.X) } } - cond, err := apd.BaseContext.Sub(&d, &hi, &lo) + cond, err := internal.BaseContext.Sub(&d, &hi, &lo) if cond.Inexact() || err != nil { break } @@ -140,7 +142,7 @@ func SimplifyBounds(ctx *OpContext, k Kind, x, y *BoundValue) Value { case diff == 2: if k&FloatKind == 0 && x.Op == GreaterThanOp && y.Op == LessThanOp { - _, _ = apd.BaseContext.Add(&d, d.SetInt64(1), &lo) + _, _ = internal.BaseContext.Add(&d, d.SetInt64(1), &lo) return ctx.newNum(&d, k&NumKind, x, y) } diff --git a/internal/core/compile/label.go b/internal/core/compile/label.go index cd4d5a706c5..24bed7d4be3 100644 --- a/internal/core/compile/label.go +++ b/internal/core/compile/label.go @@ -15,7 +15,7 @@ package compile import ( - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "golang.org/x/text/unicode/norm" "cuelang.org/go/cue/ast" diff --git a/internal/core/convert/go.go b/internal/core/convert/go.go index d4f856cb85b..2562207bb0e 100644 --- a/internal/core/convert/go.go +++ b/internal/core/convert/go.go @@ -25,7 +25,7 @@ import ( "strconv" "strings" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "golang.org/x/text/encoding/unicode" "cuelang.org/go/cue/ast" @@ -237,12 +237,16 @@ func convertRec(ctx *adt.OpContext, nilIsTop bool, x interface{}) adt.Value { return compileExpr(ctx, v) case *big.Int: - return &adt.Num{Src: src, K: adt.IntKind, X: *apd.NewWithBigInt(v, 0)} + v2 := new(apd.BigInt).SetMathBigInt(v) + return &adt.Num{Src: src, K: adt.IntKind, X: *apd.NewWithBigInt(v2, 0)} case *big.Rat: // should we represent this as a binary operation? n := &adt.Num{Src: src, K: adt.IntKind} - _, err := apd.BaseContext.Quo(&n.X, apd.NewWithBigInt(v.Num(), 0), apd.NewWithBigInt(v.Denom(), 0)) + _, err := internal.BaseContext.Quo(&n.X, + apd.NewWithBigInt(new(apd.BigInt).SetMathBigInt(v.Num()), 0), + apd.NewWithBigInt(new(apd.BigInt).SetMathBigInt(v.Denom()), 0), + ) if err != nil { return ctx.AddErrf("could not convert *big.Rat: %v", err) } @@ -268,7 +272,7 @@ func convertRec(ctx *adt.OpContext, nilIsTop bool, x interface{}) adt.Value { // with this: kind := adt.FloatKind var d apd.Decimal - res, _ := apd.BaseContext.RoundToIntegralExact(&d, v) + res, _ := internal.BaseContext.RoundToIntegralExact(&d, v) if !res.Inexact() { kind = adt.IntKind } diff --git a/internal/core/convert/go_test.go b/internal/core/convert/go_test.go index cf196265565..15726325fec 100644 --- a/internal/core/convert/go_test.go +++ b/internal/core/convert/go_test.go @@ -23,7 +23,7 @@ import ( "testing" "time" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "github.com/google/go-cmp/cmp" "cuelang.org/go/cue/errors" diff --git a/internal/core/export/bounds.go b/internal/core/export/bounds.go index 325f0aae804..90140e26b02 100644 --- a/internal/core/export/bounds.go +++ b/internal/core/export/bounds.go @@ -17,7 +17,7 @@ package export import ( "cuelang.org/go/cue/ast" "cuelang.org/go/internal/core/adt" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" ) // boundSimplifier simplifies bound values into predeclared identifiers, if diff --git a/internal/internal.go b/internal/internal.go index 876e8a92d45..6622458b36b 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -26,7 +26,7 @@ import ( "path/filepath" "strings" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/cue/ast" "cuelang.org/go/cue/ast/astutil" @@ -39,6 +39,54 @@ import ( // Right now Decimal is aliased to apd.Decimal. This may change in the future. type Decimal = apd.Decimal +// Context wraps apd.Context for CUE's custom logic. +// +// Note that it avoids pointers to make it easier to make copies. +type Context struct { + apd.Context +} + +// WithPrecision mirrors upstream, but returning our type without a pointer. +func (c Context) WithPrecision(p uint32) Context { + c.Context = *c.Context.WithPrecision(p) + return c +} + +// apd/v2 used to call Reduce on the result of Quo and Rem, +// so that the operations always trimmed all but one trailing zeros. +// apd/v3 does not do that at all. +// For now, get the old behavior back by calling Reduce ourselves. +// Note that v3's Reduce also removes all trailing zeros, +// whereas v2's Reduce would leave ".0" behind. +// Get that detail back as well, to consistently show floats with decimal points. +// +// TODO: Rather than reducing all trailing zeros, +// we should keep a number of zeros that makes sense given the operation. + +func reduceKeepingFloats(d *apd.Decimal) { + oldExponent := d.Exponent + d.Reduce(d) + // If the decimal had decimal places, like "3.000" and "5.000E+5", + // Reduce gives us "3" and "5E+5", but we want "3.0" and "5.0E+5". + if oldExponent < 0 && d.Exponent >= 0 { + d.Exponent-- + // TODO: we can likely make the NewBigInt(10) a static global to reduce allocs + d.Coeff.Mul(&d.Coeff, apd.NewBigInt(10)) + } +} + +func (c Context) Quo(d, x, y *apd.Decimal) (apd.Condition, error) { + res, err := c.Context.Quo(d, x, y) + reduceKeepingFloats(d) + return res, err +} + +func (c Context) Sqrt(d, x *apd.Decimal) (apd.Condition, error) { + res, err := c.Context.Sqrt(d, x) + reduceKeepingFloats(d) + return res, err +} + // ErrIncomplete can be used by builtins to signal the evaluation was // incomplete. var ErrIncomplete = errors.New("incomplete value") @@ -46,8 +94,8 @@ var ErrIncomplete = errors.New("incomplete value") // MakeInstance makes a new instance from a value. var MakeInstance func(value interface{}) (instance interface{}) -// BaseContext is used as CUEs default context for arbitrary-precision decimals -var BaseContext = apd.BaseContext.WithPrecision(24) +// BaseContext is used as CUE's default context for arbitrary-precision decimals. +var BaseContext = Context{*apd.BaseContext.WithPrecision(24)} // APIVersionSupported is the back version until which deprecated features // are still supported. diff --git a/internal/pkg/context.go b/internal/pkg/context.go index 23768f81245..033420de61f 100644 --- a/internal/pkg/context.go +++ b/internal/pkg/context.go @@ -22,7 +22,7 @@ import ( "cuelang.org/go/cue/token" "cuelang.org/go/internal/core/adt" "cuelang.org/go/internal/value" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" ) // CallCtxt is passed to builtin implementations that need to use a cue.Value. This is an internal type. Its interface may change. diff --git a/pkg/list/math.go b/pkg/list/math.go index 47cec527fdd..602ebd6a936 100644 --- a/pkg/list/math.go +++ b/pkg/list/math.go @@ -17,7 +17,7 @@ package list import ( "fmt" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/internal" ) diff --git a/pkg/math/manual.go b/pkg/math/manual.go index 15579741feb..474bc7d82bf 100644 --- a/pkg/math/manual.go +++ b/pkg/math/manual.go @@ -17,27 +17,28 @@ package math import ( "math/big" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/internal" ) -func roundContext(rounder string) *apd.Context { - c := *apdContext +func roundContext(rounder apd.Rounder) internal.Context { + c := internal.BaseContext c.Rounding = rounder - return &c + return c } // TODO: for now we convert Decimals to int. This allows the desired type to be // conveyed. This has the disadvantage that a number like 1E10000 will need to be // expanded. Eventually it would be better to unify number types and allow // anything that results in an integer to pose as an integer type. +// TODO: this is likely buggy, as we discard d.Exponent entirely. func toInt(d *internal.Decimal) *big.Int { i := &d.Coeff if d.Negative { i.Neg(i) } - return i + return i.MathBigInt() } // Floor returns the greatest integer value less than or equal to x. @@ -49,8 +50,8 @@ func toInt(d *internal.Decimal) *big.Int { // Floor(NaN) = NaN func Floor(x *internal.Decimal) (*big.Int, error) { var d internal.Decimal - _, err := apdContext.Floor(&d, x) - _, _ = apdContext.Quantize(&d, &d, 0) + _, err := internal.BaseContext.Floor(&d, x) + _, _ = internal.BaseContext.Quantize(&d, &d, 0) return toInt(&d), err } @@ -63,8 +64,8 @@ func Floor(x *internal.Decimal) (*big.Int, error) { // Ceil(NaN) = NaN func Ceil(x *internal.Decimal) (*big.Int, error) { var d internal.Decimal - _, err := apdContext.Ceil(&d, x) - _, _ = apdContext.Quantize(&d, &d, 0) + _, err := internal.BaseContext.Ceil(&d, x) + _, _ = internal.BaseContext.Quantize(&d, &d, 0) return toInt(&d), err } @@ -113,7 +114,7 @@ func RoundToEven(x *internal.Decimal) (*big.Int, error) { return toInt(&d), err } -var mulContext = apd.BaseContext.WithPrecision(1) +var mulContext = internal.BaseContext.WithPrecision(1) // MultipleOf reports whether x is a multiple of y. func MultipleOf(x, y *internal.Decimal) (bool, error) { diff --git a/pkg/math/math.go b/pkg/math/math.go index c76eaa1f536..b7b90623135 100644 --- a/pkg/math/math.go +++ b/pkg/math/math.go @@ -21,19 +21,17 @@ package math import ( "math" - "github.com/cockroachdb/apd/v2" + "github.com/cockroachdb/apd/v3" "cuelang.org/go/internal" ) -var apdContext = apd.BaseContext.WithPrecision(24) - // Abs returns the absolute value of x. // // Special case: Abs(±Inf) = +Inf func Abs(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Abs(&d, x) + _, err := internal.BaseContext.Abs(&d, x) return &d, err } @@ -137,7 +135,7 @@ func Atanh(x float64) float64 { // Cbrt(NaN) = NaN func Cbrt(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Cbrt(&d, x) + _, err := internal.BaseContext.Cbrt(&d, x) return &d, err } @@ -178,7 +176,7 @@ var zero = apd.New(0, 0) // Dim(x, NaN) = Dim(NaN, x) = NaN func Dim(x, y *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Sub(&d, x, y) + _, err := internal.BaseContext.Sub(&d, x, y) if err != nil { return nil, err } @@ -245,7 +243,7 @@ func Erfcinv(x float64) float64 { // Very small values underflow to 1. func Exp(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Exp(&d, x) + _, err := internal.BaseContext.Exp(&d, x) return &d, err } @@ -256,7 +254,7 @@ var two = apd.New(2, 0) // Special cases are the same as Exp. func Exp2(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Pow(&d, two, x) + _, err := internal.BaseContext.Pow(&d, two, x) return &d, err } @@ -391,7 +389,7 @@ func Ldexp(frac float64, exp int) float64 { // Log(NaN) = NaN func Log(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Ln(&d, x) + _, err := internal.BaseContext.Ln(&d, x) return &d, err } @@ -399,7 +397,7 @@ func Log(x *internal.Decimal) (*internal.Decimal, error) { // The special cases are the same as for Log. func Log10(x *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Log10(&d, x) + _, err := internal.BaseContext.Log10(&d, x) return &d, err } @@ -407,12 +405,12 @@ func Log10(x *internal.Decimal) (*internal.Decimal, error) { // The special cases are the same as for Log. func Log2(x *internal.Decimal) (*internal.Decimal, error) { var d, ln2 internal.Decimal - _, _ = apdContext.Ln(&ln2, two) - _, err := apdContext.Ln(&d, x) + _, _ = internal.BaseContext.Ln(&ln2, two) + _, err := internal.BaseContext.Ln(&d, x) if err != nil { return &d, err } - _, err = apdContext.Quo(&d, &d, &ln2) + _, err = internal.BaseContext.Quo(&d, &d, &ln2) return &d, err } @@ -493,7 +491,7 @@ func Mod(x, y float64) float64 { // Pow(x, y) = NaN for finite x < 0 and finite non-integer y func Pow(x, y *internal.Decimal) (*internal.Decimal, error) { var d internal.Decimal - _, err := apdContext.Pow(&d, x, y) + _, err := internal.BaseContext.Pow(&d, x, y) return &d, err }