math/big: make Rat.Denom() always return a reference

The documentation says so, but in the case of a normalized
integral Rat, the denominator was a new value. Changed the
internal representation to use an Int to represent the
denominator (with the sign ignored), so a reference to it
can always be returned.

Clarified documentation and added test cases.

Fixes #3521.

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/6237045

diff --git a/src/pkg/math/big/rat.go b/src/pkg/math/big/rat.go
index 7bd83fc0fb2820b688f0e23c5c648ad104d6d748..5c2a48654a7185cd75b75012212f7bcf8196535a 100644 (file)
--- a/src/pkg/math/big/rat.go
+++ b/src/pkg/math/big/rat.go
@@ -16,8 +16,10 @@ import (
 // A Rat represents a quotient a/b of arbitrary precision.
 // The zero value for a Rat represents the value 0.
 type Rat struct {
-       a Int
-       b nat // len(b) == 0 acts like b == 1
+       // To make zero values for Rat work w/o initialization,
+       // a zero value of b (len(b) == 0) acts like b == 1.
+       // a.neg determines the sign of the Rat, b.neg is ignored.
+       a, b Int
 }
 
 // NewRat creates a new Rat with numerator a and denominator b.
@@ -36,7 +38,7 @@ func (z *Rat) SetFrac(a, b *Int) *Rat {
                babs = nat(nil).set(babs) // make a copy
        }
        z.a.abs = z.a.abs.set(a.abs)
-       z.b = z.b.set(babs)
+       z.b.abs = z.b.abs.set(babs)
        return z.norm()
 }
 
@@ -50,21 +52,21 @@ func (z *Rat) SetFrac64(a, b int64) *Rat {
                b = -b
                z.a.neg = !z.a.neg
        }
-       z.b = z.b.setUint64(uint64(b))
+       z.b.abs = z.b.abs.setUint64(uint64(b))
        return z.norm()
 }
 
 // SetInt sets z to x (by making a copy of x) and returns z.
 func (z *Rat) SetInt(x *Int) *Rat {
        z.a.Set(x)
-       z.b = z.b.make(0)
+       z.b.abs = z.b.abs.make(0)
        return z
 }
 
 // SetInt64 sets z to x and returns z.
 func (z *Rat) SetInt64(x int64) *Rat {
        z.a.SetInt64(x)
-       z.b = z.b.make(0)
+       z.b.abs = z.b.abs.make(0)
        return z
 }
 
@@ -72,7 +74,7 @@ func (z *Rat) SetInt64(x int64) *Rat {
 func (z *Rat) Set(x *Rat) *Rat {
        if z != x {
                z.a.Set(&x.a)
-               z.b = z.b.set(x.b)
+               z.b.Set(&x.b)
        }
        return z
 }
@@ -97,15 +99,15 @@ func (z *Rat) Inv(x *Rat) *Rat {
                panic("division by zero")
        }
        z.Set(x)
-       a := z.b
+       a := z.b.abs
        if len(a) == 0 {
-               a = a.setWord(1) // materialize numerator
+               a = a.set(natOne) // materialize numerator
        }
        b := z.a.abs
        if b.cmp(natOne) == 0 {
                b = b.make(0) // normalize denominator
        }
-       z.a.abs, z.b = a, b // sign doesn't change
+       z.a.abs, z.b.abs = a, b // sign doesn't change
        return z
 }
 
@@ -121,24 +123,26 @@ func (x *Rat) Sign() int {
 
 // IsInt returns true if the denominator of x is 1.
 func (x *Rat) IsInt() bool {
-       return len(x.b) == 0 || x.b.cmp(natOne) == 0
+       return len(x.b.abs) == 0 || x.b.abs.cmp(natOne) == 0
 }
 
 // Num returns the numerator of x; it may be <= 0.
 // The result is a reference to x's numerator; it
-// may change if a new value is assigned to x.
+// may change if a new value is assigned to x, and vice versa.
+// The sign of the numerator corresponds to the sign of x.
 func (x *Rat) Num() *Int {
        return &x.a
 }
 
 // Denom returns the denominator of x; it is always > 0.
 // The result is a reference to x's denominator; it
-// may change if a new value is assigned to x.
+// may change if a new value is assigned to x, and vice versa.
 func (x *Rat) Denom() *Int {
-       if len(x.b) == 0 {
-               return &Int{abs: nat{1}}
+       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
        }
-       return &Int{abs: x.b}
+       return &x.b
 }
 
 func gcd(x, y nat) nat {
@@ -160,16 +164,20 @@ func (z *Rat) norm() *Rat {
        case len(z.a.abs) == 0:
                // z == 0 - normalize sign and denominator
                z.a.neg = false
-               z.b = z.b.make(0)
-       case len(z.b) == 0:
+               z.b.abs = z.b.abs.make(0)
+       case len(z.b.abs) == 0:
                // z is normalized int - nothing to do
-       case z.b.cmp(natOne) == 0:
+       case z.b.abs.cmp(natOne) == 0:
                // z is int - normalize denominator
-               z.b = z.b.make(0)
+               z.b.abs = z.b.abs.make(0)
        default:
-               if f := gcd(z.a.abs, z.b); f.cmp(natOne) != 0 {
+               if f := gcd(z.a.abs, z.b.abs); f.cmp(natOne) != 0 {
                        z.a.abs, _ = z.a.abs.div(nil, z.a.abs, f)
-                       z.b, _ = z.b.div(nil, z.b, f)
+                       z.b.abs, _ = z.b.abs.div(nil, z.b.abs, f)
+                       if z.b.abs.cmp(natOne) == 0 {
+                               // z is int - normalize denominator
+                               z.b.abs = z.b.abs.make(0)
+                       }
                }
        }
        return z
@@ -207,31 +215,31 @@ func scaleDenom(x *Int, f nat) *Int {
 //   +1 if x >  y
 //
 func (x *Rat) Cmp(y *Rat) int {
-       return scaleDenom(&x.a, y.b).Cmp(scaleDenom(&y.a, x.b))
+       return scaleDenom(&x.a, y.b.abs).Cmp(scaleDenom(&y.a, x.b.abs))
 }
 
 // Add sets z to the sum x+y and returns z.
 func (z *Rat) Add(x, y *Rat) *Rat {
-       a1 := scaleDenom(&x.a, y.b)
-       a2 := scaleDenom(&y.a, x.b)
+       a1 := scaleDenom(&x.a, y.b.abs)
+       a2 := scaleDenom(&y.a, x.b.abs)
        z.a.Add(a1, a2)
-       z.b = mulDenom(z.b, x.b, y.b)
+       z.b.abs = mulDenom(z.b.abs, x.b.abs, y.b.abs)
        return z.norm()
 }
 
 // Sub sets z to the difference x-y and returns z.
 func (z *Rat) Sub(x, y *Rat) *Rat {
-       a1 := scaleDenom(&x.a, y.b)
-       a2 := scaleDenom(&y.a, x.b)
+       a1 := scaleDenom(&x.a, y.b.abs)
+       a2 := scaleDenom(&y.a, x.b.abs)
        z.a.Sub(a1, a2)
-       z.b = mulDenom(z.b, x.b, y.b)
+       z.b.abs = mulDenom(z.b.abs, x.b.abs, y.b.abs)
        return z.norm()
 }
 
 // Mul sets z to the product x*y and returns z.
 func (z *Rat) Mul(x, y *Rat) *Rat {
        z.a.Mul(&x.a, &y.a)
-       z.b = mulDenom(z.b, x.b, y.b)
+       z.b.abs = mulDenom(z.b.abs, x.b.abs, y.b.abs)
        return z.norm()
 }
 
@@ -241,10 +249,10 @@ func (z *Rat) Quo(x, y *Rat) *Rat {
        if len(y.a.abs) == 0 {
                panic("division by zero")
        }
-       a := scaleDenom(&x.a, y.b)
-       b := scaleDenom(&y.a, x.b)
+       a := scaleDenom(&x.a, y.b.abs)
+       b := scaleDenom(&y.a, x.b.abs)
        z.a.abs = a.abs
-       z.b = b.abs
+       z.b.abs = b.abs
        z.a.neg = a.neg != b.neg
        return z.norm()
 }
@@ -286,7 +294,7 @@ func (z *Rat) SetString(s string) (*Rat, bool) {
                }
                s = s[sep+1:]
                var err error
-               if z.b, _, err = z.b.scan(strings.NewReader(s), 10); err != nil {
+               if z.b.abs, _, err = z.b.abs.scan(strings.NewReader(s), 10); err != nil {
                        return nil, false
                }
                return z.norm(), true
@@ -317,11 +325,11 @@ func (z *Rat) SetString(s string) (*Rat, bool) {
        }
        powTen := nat(nil).expNN(natTen, exp.abs, nil)
        if exp.neg {
-               z.b = powTen
+               z.b.abs = powTen
                z.norm()
        } else {
                z.a.abs = z.a.abs.mul(z.a.abs, powTen)
-               z.b = z.b.make(0)
+               z.b.abs = z.b.abs.make(0)
        }
 
        return z, true
@@ -330,8 +338,8 @@ func (z *Rat) SetString(s string) (*Rat, bool) {
 // String returns a string representation of z in the form "a/b" (even if b == 1).
 func (x *Rat) String() string {
        s := "/1"
-       if len(x.b) != 0 {
-               s = "/" + x.b.decimalString()
+       if len(x.b.abs) != 0 {
+               s = "/" + x.b.abs.decimalString()
        }
        return x.a.String() + s
 }
@@ -355,9 +363,9 @@ func (x *Rat) FloatString(prec int) string {
                }
                return s
        }
-       // x.b != 0
+       // x.b.abs != 0
 
-       q, r := nat(nil).div(nat(nil), x.a.abs, x.b)
+       q, r := nat(nil).div(nat(nil), x.a.abs, x.b.abs)
 
        p := natOne
        if prec > 0 {
@@ -365,11 +373,11 @@ func (x *Rat) FloatString(prec int) string {
        }
 
        r = r.mul(r, p)
-       r, r2 := r.div(nat(nil), r, x.b)
+       r, r2 := r.div(nat(nil), r, x.b.abs)
 
        // see if we need to round up
        r2 = r2.add(r2, r2)
-       if x.b.cmp(r2) <= 0 {
+       if x.b.abs.cmp(r2) <= 0 {
                r = r.add(r, natOne)
                if r.cmp(p) >= 0 {
                        q = nat(nil).add(q, natOne)
@@ -396,8 +404,8 @@ const ratGobVersion byte = 1
 
 // GobEncode implements the gob.GobEncoder interface.
 func (x *Rat) GobEncode() ([]byte, error) {
-       buf := make([]byte, 1+4+(len(x.a.abs)+len(x.b))*_S) // extra bytes for version and sign bit (1), and numerator length (4)
-       i := x.b.bytes(buf)
+       buf := make([]byte, 1+4+(len(x.a.abs)+len(x.b.abs))*_S) // extra bytes for version and sign bit (1), and numerator length (4)
+       i := x.b.abs.bytes(buf)
        j := x.a.abs.bytes(buf[0:i])
        n := i - j
        if int(uint32(n)) != n {
@@ -427,6 +435,6 @@ func (z *Rat) GobDecode(buf []byte) error {
        i := j + binary.BigEndian.Uint32(buf[j-4:j])
        z.a.neg = b&1 != 0
        z.a.abs = z.a.abs.setBytes(buf[j:i])
-       z.b = z.b.setBytes(buf[i:])
+       z.b.abs = z.b.abs.setBytes(buf[i:])
        return nil
 }

diff --git a/src/pkg/math/big/rat_test.go b/src/pkg/math/big/rat_test.go
index fbeb596007091516556f3b0f3f519514baeb642f..7c634233ff81e0079c4440443ebc9aaa3ed198cf 100644 (file)
--- a/src/pkg/math/big/rat_test.go
+++ b/src/pkg/math/big/rat_test.go
@@ -443,3 +443,56 @@ func TestIssue2379(t *testing.T) {
                t.Errorf("5) got %s want %s", x, q)
        }
 }
+
+func TestIssue3521(t *testing.T) {
+       a := new(Int)
+       b := new(Int)
+       a.SetString("64375784358435883458348587", 0)
+       b.SetString("4789759874531", 0)
+
+       // 0) a raw zero value has 1 as denominator
+       zero := new(Rat)
+       one := NewInt(1)
+       if zero.Denom().Cmp(one) != 0 {
+               t.Errorf("0) got %s want %s", zero.Denom(), one)
+       }
+
+       // 1a) a zero value remains zero independent of denominator
+       x := new(Rat)
+       x.Denom().Set(new(Int).Neg(b))
+       if x.Cmp(zero) != 0 {
+               t.Errorf("1a) got %s want %s", x, zero)
+       }
+
+       // 1b) 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)
+       }
+
+       // 2a) an integral value becomes a fraction depending on denominator
+       x.SetFrac64(10, 2)
+       x.Denom().SetInt64(3)
+       q53 := NewRat(5, 3)
+       if x.Cmp(q53) != 0 {
+               t.Errorf("2a) got %s want %s", x, q53)
+       }
+
+       // 2b) an integral value becomes a fraction depending on denominator
+       x = NewRat(10, 2)
+       x.Denom().SetInt64(3)
+       if x.Cmp(q53) != 0 {
+               t.Errorf("2b) got %s want %s", x, q53)
+       }
+
+       // 3) changing the numerator/denominator of a Rat changes the Rat
+       x.SetFrac(a, b)
+       a = x.Num()
+       b = x.Denom()
+       a.SetInt64(5)
+       b.SetInt64(3)
+       if x.Cmp(q53) != 0 {
+               t.Errorf("3) got %s want %s", x, q53)
+       }
+}