golang/go

gccgo: does not see stupidness of shift count

dvyukov opened this issue · 8 comments

Both gc and go/types reject the following program, but gccgo accepts:

package a
var g = 0>>1080

gccgo should reject it as well.
gcc version 6.0.0 2015070 (experimental) (GCC)

rsc commented

Not sure why gccgo must reject this.

I agree with rsc: the language spec does not require us to reject this program. I think gccgo is fine.

First, such differences will make porting of large code based between compilers hard (this will actually have to be explicit porting process like in C++). Second, if it is a worthwhile diagnostic then both compilers should do it, otherwise no compiler should bother user.
There is really no objective reasons for differences between compilers in this place. Why do you want to keep this difference?

If gccgo does reject this program, it isn't clear to me how many shifts would be allowed before it becomes a stupid shift. gc and go/types already disagree on this point, with go1.5 putting the limit on stupidness at 512 bits and go/types limiting it around 1024 bits. It's weird that if the expression was 1 >> 1000, now go/types and gccgo would accept it while gc would not. If we wanted to fix the differences between the compilers, what's the right thing to do here?

No real program is going to write this kind of code. So I reject the argument that this will complicate porting of large code bases between compilers.

This is not, in my opinion, a worthwhile diagnostic. The gc compiler implements it to simplify its own internal handling of untyped constants.

The only possible principled approach is for all compilers to implement the complete language described in the spec. The spec permits large shift counts. Therefore the compilers should too. But, again, no actual code is going to use this, so it doesn't matter that gc rejects it.

I don't think there is anything to do here.

I'd like to disagree with the opinion that, "No real program is going to write this kind of code."

While writing tests on denormals for an fma function, I had to write:
var fmaTests = []struct {
x, y, z, out float64
}{
{1.0 + 1.0/(1<<27), 1.0 - 1.0/(1<<27), -Ldexp(1, -1074), 1.0 - 1.0/(1<<53)}, // 4.9406564584124654418e-324 but 1.0/(1<<1074) => "stupid shift"
}

@cldorian I tend to agree. It would be nice to set the shift max to a value that permits the direct expression of the extremes of float64. It doesn't cost in the compiler. A tad over 1000 as limit should be ok.