Skip to content

Commit

Permalink
math/big: call norm when returning success from Rat SetString
Browse files Browse the repository at this point in the history
After CL 24430, reflect.DeepEqual no longer returns true when comparing
a *Rat built with (*Rat).SetString("0") with one built with
(*Rat).SetInt64(0).
These should be equivalent, but because (*Rat).SetString does not call
norm() when returning the zero value, the result of reflect.DeepEqual
will be false.

One could suggest that developers should use (*Rat).Cmp instead
of relying on reflect.DeepEqual, but if a (*Rat) is part of a
larger struct that is being compared, this can be cumbersome.

This is fixed by calling z.norm() when returning zero in SetString.

Fixes #50944

Change-Id: Ib84ae975bf82fe02d1203aa9668a01960c0fd59d
Reviewed-on: https://go-review.googlesource.com/c/go/+/364434
Reviewed-by: Katie Hockman <[email protected]>
Trust: Katie Hockman <[email protected]>
Trust: Ian Lance Taylor <[email protected]>
  • Loading branch information
shuLhan authored and ianlancetaylor committed Mar 31, 2022
1 parent cc46cac commit a80070e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/math/big/ratconv.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (z *Rat) SetString(s string) (*Rat, bool) {

// special-case 0 (see also issue #16176)
if len(z.a.abs) == 0 {
return z, true
return z.norm(), true
}
// len(z.a.abs) > 0

Expand Down
9 changes: 9 additions & 0 deletions src/math/big/ratconv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"io"
"math"
"reflect"
"strconv"
"strings"
"testing"
Expand Down Expand Up @@ -205,6 +206,14 @@ func TestRatSetString(t *testing.T) {
}
}

func TestRatSetStringZero(t *testing.T) {
got, _ := new(Rat).SetString("0")
want := new(Rat).SetInt64(0)
if !reflect.DeepEqual(got, want) {
t.Errorf("got %#+v, want %#+v", got, want)
}
}

func TestRatScan(t *testing.T) {
var buf bytes.Buffer
for i, test := range setStringTests {
Expand Down

0 comments on commit a80070e

Please sign in to comment.