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

runtime: map lookup of non-comparable value doesn't panic #23734

Closed
mdempsky opened this issue Feb 7, 2018 · 5 comments
Closed

runtime: map lookup of non-comparable value doesn't panic #23734

mdempsky opened this issue Feb 7, 2018 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Member

mdempsky commented Feb 7, 2018

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

This program prints 0:

package main

func main() {
	var m map[interface{}]int
	var k []int

	println(m[k])
}

The Go spec says:

If the key type is an interface type, these comparison operators must be defined for the dynamic key values; failure will cause a run-time panic.

It seems unspecified what exactly "failure" means, but my interpretation is that using a non-comparable value like k above in the expression m[k] should constitute as "failure".

/cc @griesemer @ianlancetaylor @randall77

@mdempsky
Copy link
Member Author

mdempsky commented Feb 7, 2018

Oh, the issue is because of this short-circuit in mapaccess1:

    if h == nil || h.count == 0 {
            return unsafe.Pointer(&zeroVal[0])
    }

If m is a non-nil non-empty map, the index expression panics.

@randall77
Copy link
Contributor

We could call alg.hash(key, 0) or alg.equal(k, k) inside that short circuit. I'm not sure if this would be worth fixing though. It seems very unlikely that anyone is depending on this behavior.

@mdempsky
Copy link
Member Author

mdempsky commented Feb 7, 2018

alg.hash(k, 0) would be better, because it'll give the same "hash of unhashable type" panic that non-empty maps would raise; alg.equal(k, k) would instead raise "comparing uncomparable type".

That said, I'd expect hash lookups into nil and empty maps might be pretty common, so it'd be desirable to skip the hash operation if possible? The compiler could add another flag to maptype (like reflexivekey) to indicate whether key values are always comparable.

@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 12, 2018
@mdempsky mdempsky added this to the Go1.11 milestone Feb 12, 2018
@go101
Copy link

go101 commented Feb 12, 2018

Same for delete

package main

func main() {
	 var m = map[interface{}]int{}
	 var i interface{} = []int{}
	 delete(m, i)
}

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 10, 2018
@gopherbot
Copy link

Change https://golang.org/cl/155918 mentions this issue: runtime: panic on uncomparable map key, even if map is empty

changkun added a commit to golang-design/under-the-hood that referenced this issue Jan 27, 2019
@golang golang locked and limited conversation to collaborators Dec 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants