Skip to content
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: stupid shift if right shift #11328

Closed
dvyukov opened this issue Jun 22, 2015 · 5 comments
Closed

cmd/compile: stupid shift if right shift #11328

dvyukov opened this issue Jun 22, 2015 · 5 comments
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Jun 22, 2015

Gc rejects to compile the following program:

package a
var v = 0>>1000

saying:

stupid shift: 1000

gotype compiles it successfully.
Compilers should agree on whether it is a valid Go program or not.

on commit af81789

@griesemer griesemer changed the title x/tools/go/types: stupid shift cmd/compile: stupid shift if right shift Jun 22, 2015
@griesemer
Copy link
Contributor

Compilers may restrict the permissible precision of constants. Per the spec http://tip.golang.org/ref/spec#Constants:

"Implementation restriction: Although numeric constants have arbitrary precision in the language, a compiler may implement them using an internal representation with limited precision. ..."

That said, right shifts should never be a problem: for x >> s where x is not constant, any value s >= 64 can be replaced with 64 (trivial fix); and (optimization) the result of x >> s can be replaced with 0 if we know that x is untyped and s >= 64. For x >> s where both x and s are constant, we can do the analogous once the shift count is over x's bit length.

@griesemer griesemer added this to the Go1.5Maybe milestone Jun 22, 2015
@josharian
Copy link
Contributor

FWIW, quoting @rsc from #9120 (comment):

I have always assumed [the restriction on channel element size] was something like the "stupid shift"
errors, where if you have that big an element size you are probably doing something
wrong. The fact that it took this long for someone to report a problem suggests he was
right.

And this report came from a fuzzer, not a real program (I presume). Given all of that, I'm switching the milestone to Unplanned. Feel free to switch back if you feel strongly.

@josharian josharian modified the milestones: Unplanned, Go1.5Maybe Jun 22, 2015
@griesemer
Copy link
Contributor

I assigned the bug to myself and gave it a Go1.5Maybe because I think it's a trivial fix. Unplanned is fine, too, but I don't think it matters.

@gopherbot
Copy link

CL https://golang.org/cl/13777 mentions this issue.

@griesemer griesemer modified the milestones: Go1.6, Unplanned Aug 21, 2015
@griesemer
Copy link
Contributor

The fix for this bug is trivial: it's deleting some unnecessary code. The math/big package can handle this case just fine. See my comments on https://golang.org/cl/13777 .

@golang golang locked and limited conversation to collaborators Aug 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants