code review 8090046: cmd/gc: don't generate garbage in m[string(byteSlice)]

cmd/gc: don't generate garbage in m[string(byteSlice)]

Fixes issue 3512

[bradfitz, 2013-04-01 22:15:51]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[remyoudompheng, 2013-04-01 22:34:17]

I wouldn't have done like that. I think I would have introduced a new node type
OSTRARRAYBYTENOP that does the no-op conversion, instrumented it in racewalk,
and implemented it in gen.c like cgen_slice.

But it's a more intrusive way of doing.

A test should be added in runtime/race to check that the race is correctly
detected.

Is it totally guaranteed that the hashmap will not keep the key inside? The
lazy-growth-on-map-access frightens me.

[bradfitz, 2013-04-01 22:44:53]

On Mon, Apr 1, 2013 at 3:34 PM, <remyoudompheng@gmail.com> wrote:

> I wouldn't have done like that. I think I would have introduced a new
> node type OSTRARRAYBYTENOP that does the no-op conversion, instrumented
> it in racewalk, and implemented it in gen.c like cgen_slice.
>
> But it's a more intrusive way of doing.
>

I'm new here.  :-)

I considered that, but didn't know conventions, and like you said--- it
seemed more intrusive.


> A test should be added in runtime/race to check that the race is
> correctly detected.
>

Will do.

Is it totally guaranteed that the hashmap will not keep the key inside?
>

Yes.


> The lazy-growth-on-map-access frightens me.
>

That's a bug and is being fixed.  golang.org/issue/5179

The only thing this impacts is per-map hash caching (golang.org/issue/5147)
which retains the key per-M between lookups to avoid future hashes.  But
after implementing that, the numbers suggest we'll need compiler support to
give hints about when it's beneficial anyway, so the compiler can not pass
the "it is beneficial" hint to the lookup function here when nocopy is in
use.

[bradfitz, 2013-04-01 23:01:52]

Hello golang-dev@googlegroups.com, remyoudompheng@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[albert.strasheim, 2013-04-02 05:48:13]

Here's a fun one from the wild:

type Uuid []byte
type UuidKey string

func (u Uuid) Key() string { return string(u) }

id := ...
m := make(map[UuidKey]struct{})
m[id.Key()] = struct{}{}

Is the compiler smart enough for this one?

Aside, we're thinking of changing Uuid to be a [16]byte or even [2]uint64, but
there are a few places where it was useful to have a nil-able thing.

[bradfitz, 2013-05-17 20:36:49]

*** Abandoned ***