-
Notifications
You must be signed in to change notification settings - Fork 17.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/compile: truncates constants #11326
Comments
This is related to a math/big bug. See issue #11341 . Once issue #11341 is fixed, re-vendor math/big to src/cmd/compile/internal. The respective method call in the compiler is in mparith3.go:156 (function mpatoflt). Depending on the math/big fix, the value may become +Inf (and then we're all done), or we may need to add a range check. |
I am a bit worried about deferring to math/big on this. For example, the value should not become +Inf, since that does not mean anything inside the compiler. More generally, math/big's job is to do what is asked, and the compiler has the added requirement of not running forever. The two are different, so it makes sense for the compiler to have some checks of its own. I will send a CL rejecting numbers with too many exponent digits. That should defend against a whole class of problems. |
CL https://golang.org/cl/11673 mentions this issue. |
I'm going to cut the size of the accepted exponents in the gc compiler for Go 1.5 to no more than 8 digits. It's not perfect but it eliminates the simplest problems. We can revisit both correctness and performance for Go 1.6. In particular, if I write:
then I should get an error that that doesn't fit in a float64, but instead the error says a completely different number doesn't fit in the float64:
|
@rsc sounds ok. This does appear very much like a math/big issue. Will investigate. |
For #11326 (but not a fix). Change-Id: Ic51814f5cd7357427c3fd990a5522775d05e7987 Reviewed-on: https://go-review.googlesource.com/11673 Reviewed-by: Robert Griesemer <gri@golang.org>
#11341 was fixed and addresses this issue as well. Cutting exponent size to 8 digits (mpatoflt in mparith3.go) is no longer needed. However, Float.SetString appears to not accept a full 32bit exponent:
leads to the error (after removing cutting the exponent size):
implying that the constant internally turned into a (Float) infinity. Keep this open for now. |
#11341 also fixed this issue. The spec mentions ( http://golang.org/ref/spec#Constants ) as an implementation restriction
A 32-bit exponent corresponds to a maximum value of approx. 1e646456992, not 1e2147483646 (== 0.5e2147483647) -- the 32bits are for a binary (not decimal) exponent. (This was also misunderstood in the test cases: test/fixedbugs/issue11326.go, test/fixedbugs/issue11326b.go) |
CL https://golang.org/cl/14830 mentions this issue. |
The following program:
prints:
That's a wrong answer.
go version devel +514014c Thu Jun 18 15:54:35 2015 +0200 linux/amd64
The text was updated successfully, but these errors were encountered: