Interface-keyed maps and non-comparable dynamic key values

[Paul Jolly]

I couldn't find anything previously on the question below, but as ever would appreciate any pointers to prior questions/issues etc.

Consider the following (playground):

====

package main

import (

        "fmt"

)

type S struct {

        f func()

}

func (s S) Impl() {}

var _ I = S{}

type I interface {

        Impl()

}

func main() {

        m := make(map[I]bool)

        m[S{}] = true

        fmt.Println(m)

}

====

Per [the spec](https://golang.org/ref/spec#Map_types):

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.

this program compiles but then fails with a runtime panic.

However, in this instance, at compile time we know by type-checking alone the type of the value used to index m (i.e. S{}) and that that type is not comparable... and so can conclude that if this line executes it will panic.

From a spec perspective, is this a case of keeping the implementation and documentation simple, by treating all cases for interface-keyed maps the same, i.e. runtime panic?

Or is there some other reason why this case isn't caught by the compiler/go vet?

Thanks

[Konstantin Khomoutov]

On Tue, 4 Apr 2017 02:40:31 -0700 (PDT)
Paul Jolly <pa...@myitcv.org.uk> wrote:

> *I couldn't find anything previously on the question below, but as
> ever would appreciate any pointers to prior questions/issues etc.*
>
> Consider the following (playground
> <https://play.golang.org/p/MQuEfeG5c9>):

>
> ====
> package main
>
> import (
> "fmt"
> )
>
> type S struct {
> f func()
> }
>
> func (s S) Impl() {}
>
> var _ I = S{}
>
> type I interface {
> Impl()
> }
>
> func main() {
> m := make(map[I]bool)
> m[S{}] = true
> fmt.Println(m)
> }
> ====
>
> Per [the spec](https://golang.org/ref/spec#Map_types):
>

> *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.*

>
>
> this program compiles but then fails with a runtime panic.
>
> However, in this instance, at compile time we know by type-checking
> alone the type of the value used to index m (i.e. S{}) and that that
> type is not comparable... and so can conclude that if this line
> executes it will panic.
>
> From a spec perspective, is this a case of keeping the implementation
> and documentation simple, by treating all cases for interface-keyed
> maps the same, i.e. runtime panic?
>
> Or is there some other reason why this case isn't caught by the
> compiler/go vet?

In the sequence

m := make(map[I]bool)
m[S{}] = true

the map "m" stores values of type I; that's all it considers when you
pass values as keys when performing operations on it.

When you insert a value keyed by S{}, the compiler creates a value of
type I which has S as its dynamic type and S{} as its value.

The program as speficied panics with

panic: runtime error: hash of unhashable type main.S

because instances of S are indeed not comparable (because S.f is a
function type) but you could just as fine do

type X struct {
x int
}

func (x X) Impl() {}

m[X{}] = true

and it would work just fine because values of X are comparable, and
still implement I.

[Paul Jolly]

When you insert a value keyed by S{}, the compiler creates a value of
type I which has S as its dynamic type and S{} as its value.

The program as speficied panics with

Apologies if it wasn't clear from my original email, but I understand why this panics; I further acknowledge the behaviour is consistent with the spec.

My question is why something that could be caught at compile time is not caught at compile time: 

[Ian Lance Taylor]

On Tue, Apr 4, 2017 at 3:41 AM, Paul Jolly <pa...@myitcv.org.uk> wrote:
>> When you insert a value keyed by S{}, the compiler creates a value of
>> type I which has S as its dynamic type and S{} as its value.
>>
>> The program as speficied panics with
>
>
> Apologies if it wasn't clear from my original email, but I understand why
> this panics; I further acknowledge the behaviour is consistent with the
> spec.
>
> My question is why something that could be caught at compile time is not
> caught at compile time:

I suspect mostly because nobody thought of it.

That said, note that this in effect a language change. It is possible
to construct a program that currently works into a program that after
this change is made will fail to compile. So while we could make this
change, it would have to be done by changing the language spec. I
don't know whether it is worth doing; is it a common problem?

Ian

[Konstantin Khomoutov]

IMO implementing such a compile check would be contrary to the "least
WTF" principle. Imagine something like

type S struct {
f func()
}

func (s S) Impl() {}

func Foo() I {
return S{}
}

m[Foo()] = true

The compiler is able to check at compile time that Foo always returns
values of the dynamic type S but that would unnecessary complicate the
analysis.

[Paul Jolly]

IMO implementing such a compile check would be contrary to the "least
WTF" principle.  Imagine something like

  type S struct {
    f func()
  }

  func (s S) Impl() {}

  func Foo() I {
    return S{}
  }

  m[Foo()] = true

The compiler is able to check at compile time that Foo always returns
values of the dynamic type S but that would unnecessary complicate the
analysis.

That's not what's being discussed here; the type of the value used to index m in your example is I. You need further analysis to work out that it's actually S{}

This discussion is restricted to cases where the type is known, by simply type checking alone, to be un-comparable.

[Paul Jolly]

Just realised I accidentally replied to Ian off-list:

> My question is why something that could be caught at compile time is not
> caught at compile time:

I suspect mostly because nobody thought of it.

That said, note that this in effect a language change.  It is possible
to construct a program that currently works into a program that after
this change is made will fail to compile. 

Yes, I can see it would be a change of sorts.

 

So while we could make this
change, it would have to be done by changing the language spec.  I
don't know whether it is worth doing; is it a common problem?

I don't know either... but when I didn't catch this until runtime it "surprised" me.

A candidate for go vet perhaps, to side step the language change if there wasn't a sufficient case for doing so?