math/big: make Rat.Denom side-effect free

A Rat is represented via a quotient a/b where a and b are Int values.
To make it possible to use an uninitialized Rat value (with a and b
uninitialized and thus == 0), the implementation treats a 0 denominator
as 1.

Rat.Num and Rat.Denom return pointers to these values a and b. Because
b may be 0, Rat.Denom used to first initialize it to 1 and thus produce
an undesirable side-effect (by changing the Rat's denominator).

This CL changes Denom to return a new (not shared) *Int with value 1
in the rare case where the Rat was not initialized. This eliminates
the side effect and returns the correct denominator value.

While this is changing behavior of the API, the impact should now be
minor because together with (prior) CL https://golang.org/cl/202997,
which initializes Rats ASAP, Denom is unlikely used to access the
denominator of an uninitialized (and thus 0) Rat. Any operation that
will somehow set a Rat value will ensure that the denominator is not 0.

Fixes #33792.
Updates #3521.

Change-Id: I0bf15ac60513cf52162bfb62440817ba36f0c3fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/203059
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/math/big/rat.go b/src/math/big/rat.go
index 24725bedcc0f00bc76ff2f29b8ba6227d7c66dc3..d35cd4cbd10c6ff74e894a4fd600f2ad6866f1ce 100644 (file)
--- a/src/math/big/rat.go
+++ b/src/math/big/rat.go
@@ -411,14 +411,19 @@ func (x *Rat) Num() *Int {
 }
 
 // Denom returns the denominator of x; it is always > 0.
-// The result is a reference to x's denominator; it
+// The result is a reference to x's denominator, unless
+// x is an uninitialized (zero value) Rat, in which case
+// the result is a new Int of value 1. (To initialize x,
+// any operation that sets x will do, including x.Set(x).)
+// If the result is a reference to x's denominator it
 // may change if a new value is assigned to x, and vice versa.
-// If x's denominator is 1, Denom may materialize the denominator, thereby
-// modifying x.
 func (x *Rat) Denom() *Int {
        x.b.neg = false // the result is always >= 0
        if len(x.b.abs) == 0 {
-               x.b.abs = x.b.abs.set(natOne) // materialize denominator (see issue #33792)
+               // Note: If this proves problematic, we could
+               //       panic instead and require the Rat to
+               //       be explicitly initialized.
+               return &Int{abs: nat{1}}
        }
        return &x.b
 }

diff --git a/src/math/big/rat_test.go b/src/math/big/rat_test.go
index 35bc85c8cdc8e2415dcf1c39525a6547d89a1b91..02569c1b16a33fc90a5e7a81febcc0f75394aab0 100644 (file)
--- a/src/math/big/rat_test.go
+++ b/src/math/big/rat_test.go
@@ -329,18 +329,40 @@ func TestIssue3521(t *testing.T) {
                t.Errorf("0) got %s want %s", zero.Denom(), one)
        }
 
-       // 1a) a zero value remains zero independent of denominator
+       // 1a) the denominator of an (uninitialized) zero value is not shared with the value
+       s := &zero.b
+       d := zero.Denom()
+       if d == s {
+               t.Errorf("1a) got %s (%p) == %s (%p) want different *Int values", d, d, s, s)
+       }
+
+       // 1b) the denominator of an (uninitialized) value is a new 1 each time
+       d1 := zero.Denom()
+       d2 := zero.Denom()
+       if d1 == d2 {
+               t.Errorf("1b) got %s (%p) == %s (%p) want different *Int values", d1, d1, d2, d2)
+       }
+
+       // 1c) the denominator of an initialized zero value is shared with the value
        x := new(Rat)
+       x.Set(x) // initialize x (any operation that sets x explicitly will do)
+       s = &x.b
+       d = x.Denom()
+       if d != s {
+               t.Errorf("1c) got %s (%p) != %s (%p) want identical *Int values", d, d, s, s)
+       }
+
+       // 1d) a zero value remains zero independent of denominator
        x.Denom().Set(new(Int).Neg(b))
        if x.Cmp(zero) != 0 {
-               t.Errorf("1a) got %s want %s", x, zero)
+               t.Errorf("1d) got %s want %s", x, zero)
        }
 
-       // 1b) a zero value may have a denominator != 0 and != 1
+       // 1e) a zero value may have a denominator != 0 and != 1
        x.Num().Set(a)
        qab := new(Rat).SetFrac(a, b)
        if x.Cmp(qab) != 0 {
-               t.Errorf("1b) got %s want %s", x, qab)
+               t.Errorf("1e) got %s want %s", x, qab)
        }
 
        // 2a) an integral value becomes a fraction depending on denominator