code review 6845113: cmd/gc: Error out on division by constant zero.

cmd/gc: Error out on division by constant zero.

Fixes issue 4264.

[DMorsing, 2012-12-02 10:57:27]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[Charlie Dorian, 2012-12-03 01:52:01]

Dividing a floating point value or a complex value by zero should not be an
error. It's a way to generate an infinity.

[DMorsing, 2012-12-03 09:19:45]

On 2012/12/03 01:52:01, Charlie Dorian wrote:
> Dividing a floating point value or a complex value by zero should not be an
> error. It's a way to generate an infinity.

Quoting from the spec:
"If the divisor is a constant, it must not be zero. If the divisor is zero at
run time, a run-time panic occurs."

For constant expressions, the spec also says that a divisor must not be zero (gc
already implements this).

The spec also says:
"The result of a floating-point or complex division by zero is not specified
beyond the IEEE-754 standard; whether a run-time panic occurs is
implementation-specific."

So some ambiguity there. I'd like to hear from others as to what they think
about this change.

[DMorsing, 2012-12-18 20:55:23]

Hello golang-dev@googlegroups.com, cldorian@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-12-22 15:12:10]

Thanks for working on this, but I'm not 100% sure the spec is correct.

The code here is fine, although it would help to have a test case checking for
the error.

I am worried about the backwards incompatibilities here. I replied on CL 6710045
about those.

https://codereview.appspot.com/6845113/diff/16001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (right):

https://codereview.appspot.com/6845113/diff/16001/src/cmd/gc/typecheck.c#newc...
src/cmd/gc/typecheck.c:641: if((op == ODIV || op == OMOD) && r->op == OLITERAL)
&& isconst(r, CTINT)
can take the place of the OLITERAL and the next line

https://codereview.appspot.com/6845113/diff/16001/test/fixedbugs/bug410.go
File test/fixedbugs/bug410.go (right):

https://codereview.appspot.com/6845113/diff/16001/test/fixedbugs/bug410.go#ne...
test/fixedbugs/bug410.go:21: x[i] ^= k[i-arr[s].num%1]
might as well make this something the compiler cannot optimize away, like %3.

[DMorsing, 2013-01-26 16:37:28]

I completely forgot about this one. PTAL.

Added a test and addressed the comments.

Did we figure out if we wanted this feature?

https://codereview.appspot.com/6845113/diff/16001/src/cmd/gc/typecheck.c
File src/cmd/gc/typecheck.c (right):

https://codereview.appspot.com/6845113/diff/16001/src/cmd/gc/typecheck.c#newc...
src/cmd/gc/typecheck.c:641: if((op == ODIV || op == OMOD) && r->op == OLITERAL)
On 2012/12/22 15:12:10, rsc wrote:
> && isconst(r, CTINT)
> can take the place of the OLITERAL and the next line

Done.

https://codereview.appspot.com/6845113/diff/16001/test/fixedbugs/bug410.go
File test/fixedbugs/bug410.go (right):

https://codereview.appspot.com/6845113/diff/16001/test/fixedbugs/bug410.go#ne...
test/fixedbugs/bug410.go:21: x[i] ^= k[i-arr[s].num%1]
On 2012/12/22 15:12:10, rsc wrote:
> might as well make this something the compiler cannot optimize away, like %3.

Done.

[rsc, 2013-01-30 17:21:16]

LGTM

Still not convinced we should do this, but go ahead and submit it.

[DMorsing, 2013-01-30 19:21:20]

*** Submitted as https://code.google.com/p/go/source/detail?r=b90cad8349e0 ***

cmd/gc: Error out on division by constant zero.

Fixes issue 4264.

R=cldorian, rsc
CC=golang-dev
https://codereview.appspot.com/6845113