From 041d31b8820b62996cf1aa7b6fff77a818f2d94d Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Sun, 17 Feb 2019 15:35:52 -0800 Subject: [PATCH] cmd/compile: don't mix internal float/complex constants of different precision There are several places where a new (internal) complex constant is allocated via new(Mpcplx) rather than newMpcmplx(). The problem with using new() is that the Mpcplx data structure's Real and Imag components don't get initialized with an Mpflt of the correct precision (they have precision 0, which may be adjusted later). In all cases but one, the components of those complex constants are set using a Set operation which "inherits" the correct precision from the value that is being set. But when creating a complex value for an imaginary literal, the imaginary component is set via SetString which assumes 64bits of precision by default. As a result, the internal representation of 0.01i and complex(0, 0.01) was not correct. Replaced all used of new(Mpcplx) with newMpcmplx() and added a new test. Fixes #30243. Change-Id: Ife7fd6ccd42bf887a55c6ce91727754657e6cb2d Reviewed-on: https://go-review.googlesource.com/c/163000 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/gc/const.go | 8 +++---- src/cmd/compile/internal/gc/mpfloat.go | 2 ++ src/cmd/compile/internal/gc/noder.go | 2 +- src/cmd/compile/internal/gc/swt_test.go | 2 +- src/cmd/compile/internal/gc/typecheck.go | 2 +- test/fixedbugs/issue30243.go | 27 ++++++++++++++++++++++++ 6 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 test/fixedbugs/issue30243.go diff --git a/src/cmd/compile/internal/gc/const.go b/src/cmd/compile/internal/gc/const.go index 3a9080e67d..f2035bf9a8 100644 --- a/src/cmd/compile/internal/gc/const.go +++ b/src/cmd/compile/internal/gc/const.go @@ -427,13 +427,13 @@ bad: func tocplx(v Val) Val { switch u := v.U.(type) { case *Mpint: - c := new(Mpcplx) + c := newMpcmplx() c.Real.SetInt(u) c.Imag.SetFloat64(0.0) v.U = c case *Mpflt: - c := new(Mpcplx) + c := newMpcmplx() c.Real.Set(u) c.Imag.SetFloat64(0.0) v.U = c @@ -845,7 +845,7 @@ Outer: case CTCPLX: x, y := x.U.(*Mpcplx), y.U.(*Mpcplx) - u := new(Mpcplx) + u := newMpcmplx() u.Real.Set(&x.Real) u.Imag.Set(&x.Imag) switch op { @@ -900,7 +900,7 @@ func unaryOp(op Op, x Val, t *types.Type) Val { case CTCPLX: x := x.U.(*Mpcplx) - u := new(Mpcplx) + u := newMpcmplx() u.Real.Set(&x.Real) u.Imag.Set(&x.Imag) u.Real.Neg() diff --git a/src/cmd/compile/internal/gc/mpfloat.go b/src/cmd/compile/internal/gc/mpfloat.go index 846ce4cca7..b3a9af452a 100644 --- a/src/cmd/compile/internal/gc/mpfloat.go +++ b/src/cmd/compile/internal/gc/mpfloat.go @@ -32,12 +32,14 @@ type Mpcplx struct { Imag Mpflt } +// Use newMpflt (not new(Mpflt)!) to get the correct default precision. func newMpflt() *Mpflt { var a Mpflt a.Val.SetPrec(Mpprec) return &a } +// Use newMpcmplx (not new(Mpcplx)!) to get the correct default precision. func newMpcmplx() *Mpcplx { var a Mpcplx a.Real = *newMpflt() diff --git a/src/cmd/compile/internal/gc/noder.go b/src/cmd/compile/internal/gc/noder.go index 3aa303c0c1..3fab95b917 100644 --- a/src/cmd/compile/internal/gc/noder.go +++ b/src/cmd/compile/internal/gc/noder.go @@ -1327,7 +1327,7 @@ func (p *noder) basicLit(lit *syntax.BasicLit) Val { return Val{U: x} case syntax.ImagLit: - x := new(Mpcplx) + x := newMpcmplx() x.Imag.SetString(strings.TrimSuffix(s, "i")) return Val{U: x} diff --git a/src/cmd/compile/internal/gc/swt_test.go b/src/cmd/compile/internal/gc/swt_test.go index 74419596d2..2f73ef7b99 100644 --- a/src/cmd/compile/internal/gc/swt_test.go +++ b/src/cmd/compile/internal/gc/swt_test.go @@ -16,7 +16,7 @@ func nodrune(r rune) *Node { } func nodflt(f float64) *Node { - v := new(Mpflt) + v := newMpflt() v.SetFloat64(f) return nodlit(Val{v}) } diff --git a/src/cmd/compile/internal/gc/typecheck.go b/src/cmd/compile/internal/gc/typecheck.go index 63e0d78273..e22fd6445a 100644 --- a/src/cmd/compile/internal/gc/typecheck.go +++ b/src/cmd/compile/internal/gc/typecheck.go @@ -1589,7 +1589,7 @@ func typecheck1(n *Node, top int) (res *Node) { if l.Op == OLITERAL && r.Op == OLITERAL { // make it a complex literal - c := new(Mpcplx) + c := newMpcmplx() c.Real.Set(toflt(l.Val()).U.(*Mpflt)) c.Imag.Set(toflt(r.Val()).U.(*Mpflt)) setconst(n, Val{c}) diff --git a/test/fixedbugs/issue30243.go b/test/fixedbugs/issue30243.go new file mode 100644 index 0000000000..51fd204cbc --- /dev/null +++ b/test/fixedbugs/issue30243.go @@ -0,0 +1,27 @@ +// run + +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Compile-time constants, even if they cannot be represented +// accurately, should remain the same in operations that don't +// affect their values. + +package main + +import "fmt" + +func main() { + const x = 0.01 + const xi = 0.01i + const xc = complex(0, x) + + if imag(xi) != x { + fmt.Printf("FAILED: %g != %g\n", imag(xi), x) + } + + if xi != complex(0, x) { + fmt.Printf("FAILED: %g != %g\n", xi, complex(0, x)) + } +} -- 2.50.0