code review 673042: Go spec: map indexing never raises a runtime exception;

Go spec: map indexing never raises a runtime exception.
Also: Actual map key must be assignment-compatible with
formal map key type.

Fixes issue 357.

[gri, 2010-03-23 01:18:19]

Hello golang-dev@googlegroups.com,

I'd like you to review this change.

[rsc1, 2010-03-23 01:25:01]

http://codereview.appspot.com/673042/diff/5001/3002
File doc/go_spec.html (right):

http://codereview.appspot.com/673042/diff/5001/3002#newcode2311
doc/go_spec.html:2311: Field selectors automatically dereference pointers.
maybe this text or a shortened version should move up into the first <li>
clause,
since this doesn't apply to interfaces.

it does seem to apply to methods (on non-interface types) though.

[r, 2010-03-23 04:39:28]

LGTM

http://codereview.appspot.com/673042/diff/5001/3002
File doc/go_spec.html (right):

http://codereview.appspot.com/673042/diff/5001/3002#newcode2311
doc/go_spec.html:2311: Field selectors automatically dereference pointers.
This is a strange sentence because it doesn't say which pointers.
Field selectors whose left hand operand is of pointer type implicitly
dereference the pointer.  That is, ...

http://codereview.appspot.com/673042/diff/5001/3002#newcode2420
doc/go_spec.html:2420: <li><code>x</code>'s type must be compatible with the key
type of <code>M</code></li>
what kind of compatible?

[r2, 2010-03-23 04:42:20]

i did not mean to say LGTM.  why does this keep happening to me?  i think it's
just reflex.

-rob

On Mar 22, 2010, at 9:39 PM, r@golang.org wrote:

> LGTM
> 
> 
> http://codereview.appspot.com/673042/diff/5001/3002
> File doc/go_spec.html (right):
> 
> http://codereview.appspot.com/673042/diff/5001/3002#newcode2311
> doc/go_spec.html:2311: Field selectors automatically dereference
> pointers.
> This is a strange sentence because it doesn't say which pointers.
> Field selectors whose left hand operand is of pointer type implicitly
> dereference the pointer.  That is, ...
> 
> http://codereview.appspot.com/673042/diff/5001/3002#newcode2420
> doc/go_spec.html:2420: <li><code>x</code>'s type must be compatible with
> the key type of <code>M</code></li>
> what kind of compatible?
> 
> http://codereview.appspot.com/673042/show

[iant, 2010-03-23 14:05:41]

FYI

http://codereview.appspot.com/673042/diff/5001/3002
File doc/go_spec.html (right):

http://codereview.appspot.com/673042/diff/5001/3002#newcode2321
doc/go_spec.html:2321: contains a field <code>f</code>, <code>x.f</code> is a
shortcut for
The comma is misplaced; it should come after <code>*A</code> rather than after
<code>A</code>.

That said, the phrase "contains a field <code>f</code>" reads oddly to me, since
A may be of any type.  I'm not sure this change actually adds anything; I think
the statement is true whether or not the new clause is true.  The new clause is
trying to clarify some cases where you will get an error, but I think it is more
confusing than helpful.

[gri, 2010-03-23 20:17:29]

PTAL.

I have removed the changes applying to auto-dereferencing for now. This is
only the maps change. I have addressed Rob's comment with respect to "what
kind of compatibility". The other comments were related to the now removed
sections (for another CL).

- Robert

On Tue, Mar 23, 2010 at 7:05 AM, <iant@golang.org> wrote:

> FYI
>
>
>
> http://codereview.appspot.com/673042/diff/5001/3002
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/673042/diff/5001/3002#newcode2321
> doc/go_spec.html:2321: contains a field <code>f</code>, <code>x.f</code>
> is a shortcut for
> The comma is misplaced; it should come after <code>*A</code> rather than
> after <code>A</code>.
>
> That said, the phrase "contains a field <code>f</code>" reads oddly to
> me, since A may be of any type.  I'm not sure this change actually adds
> anything; I think the statement is true whether or not the new clause is
> true.  The new clause is trying to clarify some cases where you will get
> an error, but I think it is more confusing than helpful.
>
>
> http://codereview.appspot.com/673042/show
>

[rsc1, 2010-03-23 20:24:54]

http://codereview.appspot.com/673042/diff/13001/14001
File doc/go_spec.html (right):

http://codereview.appspot.com/673042/diff/13001/14001#newcode2421
doc/go_spec.html:2421: <a href="#Assignment_compatibility">assignment
compatible</a>
This is a spec change too.
I think it's a good one; just pointing it out.

Fixes issue 357?
http://code.google.com/p/go/issues/detail?id=357

; cat x.go
package main

var m = make(map[interface{}]int)

func main() {
	println(m[1])
}
; 8g x.go
x.go:6: invalid map index 1 - need type interface { }
; 

Again, I'm in favor of keeping the new wording and
fixing the compiler.

[r2, 2010-03-23 20:52:33]

LGTM

[r2, 2010-03-23 20:54:01]

On Mar 23, 2010, at 1:24 PM, rsc@google.com wrote:

>
>
> Again, I'm in favor of keeping the new wording and
> fixing the compiler.

agreed. assignment compatibility seems correct to me.

-rob

[rsc, 2010-03-23 20:55:05]

LGTM

[gri, 2010-03-23 21:01:56]

*** Submitted as http://code.google.com/p/go/source/detail?r=12b597435d21 ***

Go spec: map indexing never raises a runtime exception.
Also: Actual map key must be assignment-compatible with
formal map key type.

Fixes issue 357.

R=r, iant, rsc, ken2
CC=golang-dev
http://codereview.appspot.com/673042

http://codereview.appspot.com/673042/diff/5001/3002
File doc/go_spec.html (right):

http://codereview.appspot.com/673042/diff/5001/3002#newcode2420
doc/go_spec.html:2420: <li><code>x</code>'s type must be compatible with the key
type of <code>M</code></li>
On 2010/03/23 04:39:29, r wrote:
> what kind of compatible?

Done.