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
proposal: expression m[k] on a nil map m should generate a run-time panic instead of returning zero value #39572
Comments
What part of the spec are you referring to? https://golang.org/ref/spec#Map_types
|
A nil map is specifically not like the behaviour you would find in C++ or java and this is for a good reason -- it eliminates the class of defensive programming that library users must employ. |
This will be a backwards incompatible change as it will cause existing programs that run fine (according to the spec) to panic at runtime. nil values in go are useable e.g. a nil slice is as useable as a slice with capacity of zero. nil maps and slices are useful for e.g. performance as they allow to have values of the type that avoid allocations. the current behaviour for nil values in go seems consistent (e.g. nil slices can be copied and appended to without panics too) for go to me. Both nil maps and slices can be ranged over without panics. making nil maps special will cause churn for existing code bases, loss of performance (causing unneeded allocations to maps with no elements) and introduce an inconsistency in the usability of nil values of different types. |
Please note that you should fill https://github.com/golang/proposal/blob/master/go2-language-changes.md when proposing a language change. |
Thanks for all the feedbacks.
@as Note that I was saying the code is correct based on the current Golang spec. I am proposing to change both the implementation and Golang spec.
@davecheney what you said is partially correct. Indeed, developers do not need to check nil map or slice in a range clause, or in build-in function len. But they need to check nil or something similar in many other cases. For example, a runtime panic will occur if the parameter s is a nil slice, func do(s []int) {
fmt.Println(s[len(s)])
} So developers need to add protection as below,
or if len(s) == 0 {
fmt.Println("The length is 0")
}
@martisch Agreed. This is a valid comment. So this proposal is for Golang 2.x instead of 1.x.
@martisch What you said is partially correct. Some current behaviors for slice for nil values are consistent with map, but not all. For example,
@mvdan Thanks for the info. This is my first time to raise proposal for Golang, and I'd like to get some feedbacks here before filling the template. Is that OK? |
This is because a map is a reference value, a slice is not. A nil slice is actually the pointer in the slice value pointing to nil. Assigning nil to a slice variable is assigning nil to the pointer to the slices backing array.
This is because a nil slice has a length of zero, and value of index is therefore out of bounds. |
@davecheney From Golang's internal implementation, I agree with what you said. But I am proposing a Golang spec/API change, so it would be better to think about it from the users' perspective. It's confusing to me for m[k] on a nil map m to return the same values as an empty map. |
Yes there is a difference in direct element access as you point out but I think that is inherent in what a map is in Go instead of an oversight. The map has a comma ok form to check if an element is really there while slices have not. I think making m[k] panic will remove more consistency than it adds. Now a nil map and an empty map behave differently when reading elements from them which means one has to check if the map is initialized more often than before. Usually coders should not care about that similar to how differentiating between nil slices and 0 len slices is discouraged (at least in code bases I often cotribute to). |
@martisch Thanks for the feedback, which makes sense, although I still have a minor comment. Developers have to differentiate a nil map and an empty map, because adding entries into a nil map will cause a panic, while it's ok for an empty map. So it should be OK to differentiate a nil map and an empty map when reading elements as well, but it isn't a big deal. Actually just as @davecheney pointed out, failure of reading from a nil slice is due to out of range instead of nil slice. So it's consistent to support reading a nil map. So I will close this ticket. |
What version of Go are you using (
go version
)?go version go1.14.2 darwin/amd64
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
What did you do?
I wrote a very simple program as below,
The above program printed:
What did you expect to see?
The above result is correct based on the current Golang spec. But I think it makes more sense to generate a run-time panic for expression m[k] on a nil map. It's confusing for m[k] on a nil map m to return the same values as an empty map.
The text was updated successfully, but these errors were encountered: