From 7416265fc1303d4673ba85ae3d480be2ebe481a4 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 3 Feb 2023 17:20:20 -0800 Subject: [PATCH] cty: Refine the ranges of arithmetic results If we are performing addition, subtraction, or multiplication on unknown numbers with known numeric bounds then we can propagate bounds to the result by performing interval arithmetic. This is not as complete as it could be because of trying to share a single implementation across all of the functions while still dealing with all of their panic edge cases. --- cty/msgpack/unknown.go | 4 +- cty/unknown_refinement.go | 4 +- cty/value_ops.go | 12 ++- cty/value_ops_test.go | 192 +++++++++++++++++++++++++++++++++++++- cty/value_range.go | 66 ++++++++++++- 5 files changed, 264 insertions(+), 14 deletions(-) diff --git a/cty/msgpack/unknown.go b/cty/msgpack/unknown.go index 667bef8a..b189ae8d 100644 --- a/cty/msgpack/unknown.go +++ b/cty/msgpack/unknown.go @@ -63,7 +63,7 @@ func marshalUnknownValue(rng cty.ValueRange, path cty.Path, enc *msgpack.Encoder lower, lowerInc := rng.NumberLowerBound() upper, upperInc := rng.NumberUpperBound() boundTy := cty.Tuple([]cty.Type{cty.Number, cty.Bool}) - if lower.IsKnown() { + if lower.IsKnown() && lower != cty.NegativeInfinity { mapLen++ refnEnc.EncodeInt(int64(unknownValNumberMin)) marshal( @@ -73,7 +73,7 @@ func marshalUnknownValue(rng cty.ValueRange, path cty.Path, enc *msgpack.Encoder refnEnc, ) } - if upper.IsKnown() { + if upper.IsKnown() && upper != cty.PositiveInfinity { mapLen++ refnEnc.EncodeInt(int64(unknownValNumberMax)) marshal( diff --git a/cty/unknown_refinement.go b/cty/unknown_refinement.go index bd2c4493..d90bcbc3 100644 --- a/cty/unknown_refinement.go +++ b/cty/unknown_refinement.go @@ -632,10 +632,10 @@ func (r *refinementNumber) rawEqual(other unknownValRefinement) bool { func (r *refinementNumber) GoString() string { var b strings.Builder b.WriteString(r.refinementNullable.GoString()) - if r.min != NilVal { + if r.min != NilVal && r.min != NegativeInfinity { fmt.Fprintf(&b, ".NumberLowerBound(%#v, %t)", r.min, r.minInc) } - if r.max != NilVal { + if r.max != NilVal && r.max != PositiveInfinity { fmt.Fprintf(&b, ".NumberUpperBound(%#v, %t)", r.max, r.maxInc) } return b.String() diff --git a/cty/value_ops.go b/cty/value_ops.go index 85dac769..171a2391 100644 --- a/cty/value_ops.go +++ b/cty/value_ops.go @@ -593,7 +593,8 @@ func (val Value) Add(other Value) Value { if shortCircuit := mustTypeCheck(Number, Number, val, other); shortCircuit != nil { shortCircuit = forceShortCircuitType(shortCircuit, Number) - return (*shortCircuit).RefineNotNull() + ret := shortCircuit.RefineWith(numericRangeArithmetic(Value.Add, val.Range(), other.Range())) + return ret.RefineNotNull() } ret := new(big.Float) @@ -612,7 +613,8 @@ func (val Value) Subtract(other Value) Value { if shortCircuit := mustTypeCheck(Number, Number, val, other); shortCircuit != nil { shortCircuit = forceShortCircuitType(shortCircuit, Number) - return (*shortCircuit).RefineNotNull() + ret := shortCircuit.RefineWith(numericRangeArithmetic(Value.Subtract, val.Range(), other.Range())) + return ret.RefineNotNull() } return val.Add(other.Negate()) @@ -646,7 +648,8 @@ func (val Value) Multiply(other Value) Value { if shortCircuit := mustTypeCheck(Number, Number, val, other); shortCircuit != nil { shortCircuit = forceShortCircuitType(shortCircuit, Number) - return (*shortCircuit).RefineNotNull() + ret := shortCircuit.RefineWith(numericRangeArithmetic(Value.Multiply, val.Range(), other.Range())) + return ret.RefineNotNull() } // find the larger precision of the arguments @@ -691,6 +694,9 @@ func (val Value) Divide(other Value) Value { if shortCircuit := mustTypeCheck(Number, Number, val, other); shortCircuit != nil { shortCircuit = forceShortCircuitType(shortCircuit, Number) + // TODO: We could potentially refine the range of the result here, but + // we don't right now because our division operation is not monotone + // if the denominator could potentially be zero. return (*shortCircuit).RefineNotNull() } diff --git a/cty/value_ops_test.go b/cty/value_ops_test.go index 0714d4ec..7a765a09 100644 --- a/cty/value_ops_test.go +++ b/cty/value_ops_test.go @@ -1767,6 +1767,66 @@ func TestValueAdd(t *testing.T) { UnknownVal(Number), UnknownVal(Number).RefineNotNull(), }, + { + NumberIntVal(1), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(3), true). + NewValue(), + }, + { + Zero, + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(4), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(3), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NumberRangeUpperBound(NumberIntVal(3), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(3), true). + NumberRangeUpperBound(NumberIntVal(5), true). + NewValue(), + }, { UnknownVal(Number), UnknownVal(Number), @@ -1803,7 +1863,7 @@ func TestValueAdd(t *testing.T) { t.Run(fmt.Sprintf("%#v.Add(%#v)", test.LHS, test.RHS), func(t *testing.T) { got := test.LHS.Add(test.RHS) if !got.RawEquals(test.Expected) { - t.Fatalf("Add returned %#v; want %#v", got, test.Expected) + t.Fatalf("Wrong result\ngot: %#v\nwant: %#v", got, test.Expected) } }) } @@ -1840,6 +1900,63 @@ func TestValueSubtract(t *testing.T) { UnknownVal(Number), UnknownVal(Number).RefineNotNull(), }, + { + NumberIntVal(1), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeUpperBound(NumberIntVal(-1), true). + NewValue(), + }, + { + Zero, + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeUpperBound(NumberIntVal(-2), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + UnknownVal(Number).RefineNotNull(), // We don't currently refine this case + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), true). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeUpperBound(NumberIntVal(0), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NumberRangeUpperBound(NumberIntVal(3), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(-2), true). + NumberRangeUpperBound(NumberIntVal(0), true). + NewValue(), + }, { NumberIntVal(1), DynamicVal, @@ -1871,7 +1988,7 @@ func TestValueSubtract(t *testing.T) { t.Run(fmt.Sprintf("%#v.Subtract(%#v)", test.LHS, test.RHS), func(t *testing.T) { got := test.LHS.Subtract(test.RHS) if !got.RawEquals(test.Expected) { - t.Fatalf("Subtract returned %#v; want %#v", got, test.Expected) + t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.Expected) } }) } @@ -1908,7 +2025,7 @@ func TestValueNegate(t *testing.T) { t.Run(fmt.Sprintf("%#v.Negate()", test.Receiver), func(t *testing.T) { got := test.Receiver.Negate() if !got.RawEquals(test.Expected) { - t.Fatalf("Negate returned %#v; want %#v", got, test.Expected) + t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.Expected) } }) } @@ -1945,6 +2062,71 @@ func TestValueMultiply(t *testing.T) { UnknownVal(Number), UnknownVal(Number).RefineNotNull(), }, + { + NumberIntVal(3), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(6), true). + NewValue(), + }, + { + Zero, + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).RefineNotNull(), // We can't currently refine this case + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(4), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(8), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(3), true). + NumberRangeUpperBound(NumberIntVal(4), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(6), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(2), false). + NumberRangeUpperBound(NumberIntVal(3), false). + NewValue(), + UnknownVal(Number).Refine(). + NotNull(). + NumberRangeLowerBound(NumberIntVal(2), true). + NumberRangeUpperBound(NumberIntVal(6), true). + NewValue(), + }, + { + UnknownVal(Number).Refine(). + NumberRangeLowerBound(NumberIntVal(1), true). + NumberRangeUpperBound(NumberIntVal(2), false). + NewValue(), + Zero, + Zero, // deduced by refinement + }, { NumberIntVal(1), DynamicVal, @@ -1986,7 +2168,7 @@ func TestValueMultiply(t *testing.T) { t.Run(fmt.Sprintf("%#v.Multiply(%#v)", test.LHS, test.RHS), func(t *testing.T) { got := test.LHS.Multiply(test.RHS) if !got.RawEquals(test.Expected) { - t.Fatalf("Multiply returned %#v; want %#v", got, test.Expected) + t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.Expected) } }) } @@ -2064,7 +2246,7 @@ func TestValueDivide(t *testing.T) { t.Run(fmt.Sprintf("%#v.Divide(%#v)", test.LHS, test.RHS), func(t *testing.T) { got := test.LHS.Divide(test.RHS) if !got.RawEquals(test.Expected) { - t.Fatalf("Divide returned %#v; want %#v", got, test.Expected) + t.Fatalf("wrong result\ngot: %#v\nwant: %#v", got, test.Expected) } }) } diff --git a/cty/value_range.go b/cty/value_range.go index e512e365..36f21946 100644 --- a/cty/value_range.go +++ b/cty/value_range.go @@ -142,7 +142,7 @@ func (r ValueRange) NumberLowerBound() (min Value, inclusive bool) { } return rfn.min, rfn.minInc } - return UnknownVal(Number), false + return NegativeInfinity, false } // NumberUpperBound returns information about the upper bound of the range of @@ -167,7 +167,7 @@ func (r ValueRange) NumberUpperBound() (max Value, inclusive bool) { } return rfn.max, rfn.maxInc } - return UnknownVal(Number), false + return PositiveInfinity, false } // StringPrefix returns a string that is guaranteed to be the prefix of @@ -332,6 +332,68 @@ func (r ValueRange) Includes(v Value) Value { return unknownResult } +// numericRangeArithmetic is a helper we use to calculate derived numeric ranges +// for arithmetic on refined numeric values. +// +// op must be a monotone operation. numericRangeArithmetic adapts that operation +// into the equivalent interval arithmetic operation. +// +// The result is a superset of the range of the given operation against the +// given input ranges, if it's possible to calculate that without encountering +// an invalid operation. Currently the result is inexact due to ignoring +// the inclusiveness of the input bounds and just always returning inclusive +// bounds. +func numericRangeArithmetic(op func(a, b Value) Value, a, b ValueRange) func(*RefinementBuilder) *RefinementBuilder { + wrapOp := func(a, b Value) (ret Value) { + // Our functions have various panicking edge cases involving incompatible + // uses of infinities. To keep things simple here we'll catch those + // and just return an unconstrained number. + defer func() { + if v := recover(); v != nil { + ret = UnknownVal(Number) + } + }() + return op(a, b) + } + + return func(builder *RefinementBuilder) *RefinementBuilder { + aMin, _ := a.NumberLowerBound() + aMax, _ := a.NumberUpperBound() + bMin, _ := b.NumberLowerBound() + bMax, _ := b.NumberUpperBound() + + v1 := wrapOp(aMin, bMin) + v2 := wrapOp(aMin, bMax) + v3 := wrapOp(aMax, bMin) + v4 := wrapOp(aMax, bMax) + + newMin := mostNumberValue(Value.LessThan, v1, v2, v3, v4) + newMax := mostNumberValue(Value.GreaterThan, v1, v2, v3, v4) + + if isInf := newMin.Equals(NegativeInfinity); isInf.IsKnown() && isInf.False() { + builder = builder.NumberRangeLowerBound(newMin, true) + } + if isInf := newMax.Equals(PositiveInfinity); isInf.IsKnown() && isInf.False() { + builder = builder.NumberRangeUpperBound(newMax, true) + } + return builder + } +} + +func mostNumberValue(op func(i, j Value) Value, v1 Value, vN ...Value) Value { + r := v1 + for _, v := range vN { + more := op(v, r) + if !more.IsKnown() { + return UnknownVal(Number) + } + if more.True() { + r = v + } + } + return r +} + // definitelyNotNull is a convenient helper for the common situation of checking // whether a value could possibly be null. //