Skip to content

Commit 473e333

Browse files
committed
Made .Mul() and .Div() more robust in the face of overflows in intermediate operations
1 parent b43ba6a commit 473e333

File tree

4 files changed

+47
-40
lines changed

4 files changed

+47
-40
lines changed

fixed/fixed128/fixed128.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ func Minimum[T fixed.Dx]() Int[T] {
3939
return Int[T]{data: num128.MinInt}
4040
}
4141

42-
// MaxSafeMultiply returns the maximum value that can be safely multiplied without overflow.
43-
func MaxSafeMultiply[T fixed.Dx]() Int[T] {
44-
return Maximum[T]().Div(Multiplier[T]())
45-
}
46-
4742
// MaxDecimalDigits returns the maximum number of digits after the decimal that will be used.
4843
func MaxDecimalDigits[T fixed.Dx]() int {
4944
var t T

fixed/fixed128/fixed128_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -422,11 +422,6 @@ func testBoundaryValues[T fixed.Dx](t *testing.T) {
422422
minValue := fixed128.Minimum[T]()
423423
c.True(maxValue.GreaterThan(minValue))
424424
c.True(minValue.LessThan(maxValue))
425-
426-
// Test MaxSafeMultiply
427-
maxSafe := fixed128.MaxSafeMultiply[T]()
428-
c.True(maxSafe.LessThan(maxValue))
429-
c.True(maxSafe.GreaterThan(minValue))
430425
}
431426

432427
func TestMinMax(t *testing.T) {

fixed/fixed64/int.go

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,24 @@ import (
1818

1919
"github.com/richardwilkes/toolbox/v2/errs"
2020
"github.com/richardwilkes/toolbox/v2/fixed"
21+
"github.com/richardwilkes/toolbox/v2/num128"
2122
"github.com/richardwilkes/toolbox/v2/xstrings"
2223
"golang.org/x/exp/constraints"
2324
"gopkg.in/yaml.v3"
2425
)
2526

26-
const (
27-
// Max holds the maximum value.
28-
Max = math.MaxInt64
29-
// Min holds the minimum value.
30-
Min = math.MinInt64
31-
)
32-
3327
// Int holds a fixed-point value. Values are truncated, not rounded. Values can be added and subtracted directly. For
3428
// multiplication and division, the provided Mul() and Div() methods should be used.
3529
type Int[T fixed.Dx] int64
3630

37-
// MaxSafeMultiply returns the maximum value that can be safely multiplied without overflow.
38-
func MaxSafeMultiply[T fixed.Dx]() Int[T] {
39-
return Int[T](Max / Multiplier[T]())
31+
// Maximum returns the maximum possible value the type can hold.
32+
func Maximum[T fixed.Dx]() Int[T] {
33+
return Int[T](math.MaxInt64)
34+
}
35+
36+
// Minimum returns the minimum possible value the type can hold.
37+
func Minimum[T fixed.Dx]() Int[T] {
38+
return Int[T](math.MinInt64)
4039
}
4140

4241
// MaxDecimalDigits returns the maximum number of digits after the decimal that will be used.
@@ -156,12 +155,32 @@ func AsFloatChecked[T fixed.Dx, TO constraints.Float](f Int[T]) (TO, error) {
156155

157156
// Mul multiplies this value by the passed-in value, returning a new value.
158157
func (f Int[T]) Mul(value Int[T]) Int[T] {
159-
return f * value / Int[T](Multiplier[T]())
158+
return f.mul64(value, Int[T](Multiplier[T]()))
159+
}
160+
161+
func (f Int[T]) mul64(value, div Int[T]) Int[T] {
162+
if f == 0 || value == 0 {
163+
return 0
164+
}
165+
if f == 1 {
166+
return value
167+
}
168+
if value == 1 {
169+
return f
170+
}
171+
result := f * value
172+
if f != math.MinInt64 && value != math.MinInt64 && result/value == f {
173+
return result / div
174+
}
175+
return Int[T](num128.IntFrom64(int64(f)).
176+
Mul(num128.IntFrom64(int64(value))).
177+
Div(num128.IntFrom64(int64(div))).
178+
AsInt64())
160179
}
161180

162181
// Div divides this value by the passed-in value, returning a new value.
163182
func (f Int[T]) Div(value Int[T]) Int[T] {
164-
return f * Int[T](Multiplier[T]()) / value
183+
return f.mul64(Int[T](Multiplier[T]()), value)
165184
}
166185

167186
// Mod returns the remainder after subtracting all full multiples of the passed-in value.

fixed/fixed64/int_test.go

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ package fixed64_test
1212
import (
1313
"encoding/json"
1414
"fmt"
15+
"math"
1516
"strings"
1617
"testing"
1718

1819
"github.com/richardwilkes/toolbox/v2/check"
1920
"github.com/richardwilkes/toolbox/v2/fixed"
2021
"github.com/richardwilkes/toolbox/v2/fixed/fixed64"
22+
"github.com/richardwilkes/toolbox/v2/num128"
2123
"gopkg.in/yaml.v3"
2224
)
2325

@@ -283,24 +285,6 @@ func testYAMLActual[T fixed.Dx](t *testing.T, v fixed64.Int[T]) {
283285
c.Equal(e1, e2)
284286
}
285287

286-
func TestMaxSafeMultiply(t *testing.T) {
287-
testMaxSafeMultiply[fixed.D1](t)
288-
testMaxSafeMultiply[fixed.D2](t)
289-
testMaxSafeMultiply[fixed.D3](t)
290-
testMaxSafeMultiply[fixed.D4](t)
291-
testMaxSafeMultiply[fixed.D5](t)
292-
testMaxSafeMultiply[fixed.D6](t)
293-
}
294-
295-
func testMaxSafeMultiply[T fixed.Dx](t *testing.T) {
296-
c := check.New(t)
297-
298-
// Test MaxSafeMultiply
299-
maxSafe := fixed64.MaxSafeMultiply[T]()
300-
c.True(maxSafe > 0)
301-
c.True(int64(maxSafe) < fixed64.Max)
302-
}
303-
304288
func TestMinMax(t *testing.T) {
305289
testMinMax[fixed.D1](t)
306290
testMinMax[fixed.D2](t)
@@ -687,3 +671,17 @@ func testAdditionalEdgeCases[T fixed.Dx](t *testing.T) {
687671
})
688672
c.HasError(err)
689673
}
674+
675+
func TestMulOverflow(t *testing.T) {
676+
c := check.New(t)
677+
v1 := int64(math.MaxInt64 / 10000)
678+
v1 -= v1 / 10
679+
m1 := int64(110)
680+
d1 := int64(100)
681+
expected := num128.IntFrom64(v1).Mul(num128.IntFrom64(m1)).Div(num128.IntFrom64(d1)).AsInt64()
682+
v2 := fixed64.FromInteger[fixed.D4](v1)
683+
m2 := fixed64.FromInteger[fixed.D4](m1)
684+
d2 := fixed64.FromInteger[fixed.D4](d1)
685+
result := fixed64.AsInteger[fixed.D4, int64](v2.Mul(m2.Div(d2)))
686+
c.Equal(expected, result)
687+
}

0 commit comments

Comments
 (0)