Skip to content

spec: clarify that make does not panic for negative capacity hint #53219

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

Closed
nobishino opened this issue Jun 3, 2022 · 4 comments
Closed

spec: clarify that make does not panic for negative capacity hint #53219

nobishino opened this issue Jun 3, 2022 · 4 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Milestone

Comments

@nobishino
Copy link
Contributor

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

The spec for Go1.18

https://go.dev/ref/spec (Version of March 10, 2022)

Does this issue reproduce with the latest release?

Yes.

What did you do?

I read the following sentence from the spec, and run the following code. (Go playground link is here: https://go.dev/play/p/whw_4ZBzsx_K)

If n is negative or larger than m at run time, a run-time panic occurs.

func main() {
	n := -1
	m := make(map[int]int, n) // does not panic
	fmt.Println(m)
}

What did you expect to see?

I expect the program to panic, because n is negative and the sentence does not exclude the case make is used for creating a map.

What did you see instead?

It did not panic.

I think the spec should clarify that make(map[int]int, n) for negative n does not (necessarily) cause run-time panic.

@seankhliao seankhliao changed the title spec: built-in make function does not panic for negative capacity hint runtime: make does not panic for negative capacity hint Jun 3, 2022
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 3, 2022
@seankhliao
Copy link
Member

cc @golang/runtime

@mknyszek
Copy link
Contributor

mknyszek commented Jun 3, 2022

Given that a negative n does not panic now, I don't think it's possible to make it panic without breaking a whole bunch of valid Go programs. It kind of makes sense for a map, because it's really just an optimization hint. If the hint doesn't make sense, it's always safe to ignore it.

However, I don't think you're wrong about the spec, it does suggest what you say. I'm not sure what the right answer is here.

CC @ianlancetaylor and @griesemer

@randall77
Copy link
Contributor

If n is negative or larger than m at run time, a run-time panic occurs.

I think this is sentence is intended to describe making slices, not maps. For maps, there is no m. Although I agree it could be clearer.

See #46909 #24308. The consensus there seems to be that with maps, there is a reasonable way to proceed when n is negative, so we don't panic. (Unlike making slices or channels, where there's no reasonable way to continue the program.)

@griesemer griesemer self-assigned this Jun 13, 2022
@griesemer griesemer added Documentation Issues describing a change to documentation. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 13, 2022
@griesemer griesemer changed the title runtime: make does not panic for negative capacity hint spec: clarify that make does not panic for negative capacity hint Jun 13, 2022
@griesemer griesemer added this to the Go1.19 milestone Jun 13, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/411919 mentions this issue: spec: clarify behavior of map size hint for make built-in

@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

6 participants