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: imperfect error message for too large index in slice composite literal #23781

Closed
dotaheor opened this issue Feb 11, 2018 · 5 comments
Closed

Comments

@dotaheor
Copy link

@dotaheor dotaheor commented Feb 11, 2018

What version of Go are you using (go version)?

go version go1.9.3 linux/amd64

Does this issue reproduce with the latest release?

yes

What did you do?

package main

import (
	"fmt"
)

func main() {
	fmt.Println("Hello, playground")
	const k int64 = 1 << 31
	_ = []int{k:1} // error: index must be non-negative integer constant
}

What did you expect to see?

compile ok, or with error

./a.go:10:13: index is too large

What did you see instead?

fail to compile, with error

./a.go:10:13: index must be non-negative integer constant

From Go spec. (in array and slice literals)

The key must be a non-negative constant representable by a value of type int;
and if it is typed it must be of integer type. 

1 << 31 is representable by a value of type int on 64-bit OS, and it is non-negative.

@odeke-em odeke-em added this to the Go1.11 milestone Feb 11, 2018
@dotaheor

This comment has been minimized.

Copy link
Author

@dotaheor dotaheor commented Feb 11, 2018

And this:

package main

func main() {
	s := []byte{1 << 31: 1} // error: index must be non-negative integer constant
	_ = s[1 << 62] // compile ok
	_ = s[1 << 63] // error:  constant 9223372036854775808 overflows int
	// I think the rules for the indexes in literal and the indexes in [] should be consistent.
}
@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Feb 11, 2018

This happens because the smallinstconst function in const.go has the following code:

if !ok || vi.CmpInt64(0) < 0 || vi.Cmp(maxintval[TINT32]) > 0 {
	return -1
}

The key you used fails the last check (vi.Cmp(maxintval[TINT32]) > 0) because maxintval[TINT32] is 0x7fffffff which is smaller than 1<<31, so the function returns -1 to signal a key error.

The problem here is that AFAIK we can't just easily remove the check above, because there are several places in the compiler (both in the frontend and the backend) where those values are assumed to be int32s.

cc @griesemer @randall77

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Feb 11, 2018

I guess we could change smallinstconst to better signal the kind of errors it encounters, and make the callers check the error before printing a must be positive error. We could add a second error that says "too big".

I'll send a patch for 1.11 if this sounds good. There's still an issue with the spec wording, but at least we'll have a better error message.

@griesemer

This comment has been minimized.

Copy link
Contributor

@griesemer griesemer commented Feb 12, 2018

FWIW, both go/types and gccgo accept this code w/o problems. gccgo can also run it (again w/o problems).

@griesemer griesemer modified the milestones: Go1.11, Go1.12 Jun 27, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 28, 2018

Change https://golang.org/cl/151599 mentions this issue: cmd/compile: fix constant index bounds check and error message

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.