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

Open
mdempsky opened this Issue Feb 7, 2018 · 4 comments

Comments

Projects
None yet
4 participants
@mdempsky
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

This comment has been minimized.

Member

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

This comment has been minimized.

Contributor

randall77 commented Feb 7, 2018

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

This comment has been minimized.

Member

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 label Feb 12, 2018

@mdempsky mdempsky added this to the Go1.11 milestone Feb 12, 2018

@go101

This comment has been minimized.

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

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