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

spec: in array and slice literals, the index/key can be non-integer type #23529

Open
guangda-hu opened this Issue Jan 24, 2018 · 14 comments

Comments

Projects
None yet
9 participants
@guangda-hu

guangda-hu commented Jan 24, 2018

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

Go playground

What operating system and processor architecture are you using (go env)?

package main

const k float64 = 3.0

var _ = []int{k: 3}
var _ = [...]int{k: 3}

func main() {}

See https://play.golang.org/p/x3keRiD0fVT. This compiles without an error.

What did you expect to see?

From the spec:

For array and slice literals the following rules apply:

  • Each element has an associated integer index marking its position in the array.
  • An element with a key uses the key as its index. 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.
  • An element without a key uses the previous element's index plus one. If the first element has no key, its index is zero.

It mentions that if the index is typed, it must be of integer type. However, the above code compiles even if the index has type float64. Probably the spec should be saying "can be converted to int" or something, or the compiler should report an error?

@ALTree

This comment has been minimized.

Member

ALTree commented Jan 24, 2018

At the beginning of September 2016, the spec simply said

the key must be a constant integer expression.

Which was vague but -as I read it- allowed things like const k float64 = 3.0 to be keys, since k is, arguably, a "constant integer".

The spec was updated in CL 30474, fix to issue #16679, and now says:

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.

This seems more "strict", in the sense that it explicitly says that typed constants need to be of integer type, and looking at the spec, the part about constant declarations:

If the type is present, all constants take the type specified,

it seems to me that a constant declared with type specifier (as float64 above) is not an untyped constant, which means it is typed, which means that if we follow the new spec phrasing, it should not be allowed to be used as an array key.

So it seems that the new spec phrasing introduced in September 2016 made the rules more strict, but the compiler was not updated accordingly.

cc @griesemer

@ALTree ALTree changed the title from spec: In array and slice literals, the index/key can be non-integer type to spec: in array and slice literals, the index/key can be non-integer type Jan 24, 2018

@bradfitz bradfitz added this to the Go1.11 milestone Jan 24, 2018

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jan 24, 2018

What does gccgo do, @ianlancetaylor?

@griesemer

This comment has been minimized.

Contributor

griesemer commented Jan 24, 2018

This seems to me like an oversight (== bug) in the compiler. The index key is supposed to follow the exact same rules as an index for a slice/array. It's not valid to use a float64 index i in s[i] even if the value of i is representable as an integer.

go/types complains in all these cases.

@griesemer griesemer self-assigned this Jan 24, 2018

@ALTree

This comment has been minimized.

Member

ALTree commented Jan 24, 2018

Btw I just tried with gccgo (7.2) and, like gc, it accepts the program above without complaints.

@griesemer

This comment has been minimized.

Contributor

griesemer commented Jan 24, 2018

I'm worried that we can't fix this in the compilers at this point, as it might break existing programs (unless they were checked also against go vet, which complains). This will need a decision.

cc: @ianlancetaylor @rsc

@j7b

This comment has been minimized.

j7b commented Jan 25, 2018

@griesemer couldn't go fix mitigate the problem by redeclaring the constant as untyped in blocks where the constant is used for indexing?

@griesemer

This comment has been minimized.

Contributor

griesemer commented Jan 25, 2018

@j7b Yes, but that would still require that source to be changed and thus possibly brake existing programs. Or perhaps I misunderstood what you meant.

@j7b

This comment has been minimized.

j7b commented Jan 25, 2018

What I was thinking is mechanically complicated, it'd be easier to have go fix explicitly convert non-int constants to int, that shouldn't break anything that would compile otherwise.

@rsc

This comment has been minimized.

Contributor

rsc commented Jan 29, 2018

Vet is going to start yelling about this (because it uses go/types) at some point (maybe Go 1.11; Go 1.10 ignores go/types failures unfortunately). After that point maybe we can fix the compilers.

@rsc rsc added the NeedsFix label Jan 29, 2018

@rsc rsc modified the milestones: Go1.11, Go1.12 Jan 29, 2018

@ak2196

This comment has been minimized.

ak2196 commented Aug 1, 2018

@griesemer @rsc go/types misses the case where the index to an array/slice is a typed constant with an underlying type int - same with composite array/slice literals. This should not work according to the spec:

https://play.golang.org/p/dn62_WXGxem

type foo float64
correctly produces an error while indexing the array - non-integer array index bar

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Aug 1, 2018

@ak2196 I'm sure I'm missing something but why do you say that your example should not work? A type defined to int is still an integer type.

@ak2196

This comment has been minimized.

ak2196 commented Aug 1, 2018

@ianlancetaylor maybe I am confused or reading the spec wrong

bar's type is foo, foo's underlying type is int but the spec says for array literals:

"An element with a key uses the key as its index. 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."

In this case the key is typed - the type is not 'int' it's foo but foo's underlying type is int. I guess it depends on what "integer type" means. Does it mean just int or anything that has int as the underlying type?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Aug 1, 2018

@ak2196 "integer type" does not mean just int. The spec does not seem to define it clearly (though I may be missing something) but certainly wherever the spec says "integer type" it accepts int, int8, int16, uint, etc., as well as any other type defined to one of those types.

@ak2196

This comment has been minimized.

ak2196 commented Aug 1, 2018

@ianlancetaylor That definitely makes sense. Thanks for clearing it up.

@griesemer griesemer modified the milestones: Go1.12, Go1.13 Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment