code review 2289041: gc, spec, tests: no auto-indirect of pointer to interfa...

gc, spec, tests: no auto-indirect of pointer to interface value

Implies no embedding of pointer to interface value either.

[rsc, 2010-09-28 19:29:45]

Hello gri, iant, ken2, r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r, 2010-09-28 20:28:31]

LGTM

[gri, 2010-09-28 20:29:06]

http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode884
doc/go_spec.html:884: (also called an embedded field).
We called them "embedded fields" once - but all fields are embedded in a struct,
aren't they? Which is why we went with anonymous field. But somehow we are still
using both terms. I'd prefer if we just use one consistently. I don't care which
one.

http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode886
doc/go_spec.html:886: a type name <code>T</code> or as a pointer to a
non-interface type name <code>*T</code>,
I guess this is a consequence of the other change?

http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode2263
doc/go_spec.html:2263: Selectors automatically dereference pointers.
This needs to be adjusted as well.

[r2, 2010-09-28 21:38:32]

On Sep 28, 2010, at 1:29 PM, gri@golang.org wrote:

> 
> http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html
> File doc/go_spec.html (right):
> 
> http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode884
> doc/go_spec.html:884: (also called an embedded field).
> We called them "embedded fields" once - but all fields are embedded in a
> struct, aren't they? Which is why we went with anonymous field. But
> somehow we are still using both terms. I'd prefer if we just use one
> consistently. I don't care which one.

let's stick with anonymous, which is more evocative and precise. i use embedded
in talks, but that's a different context.

> http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode886
> doc/go_spec.html:886: a type name <code>T</code> or as a pointer to a
> non-interface type name <code>*T</code>,
> I guess this is a consequence of the other change?
> 
> http://codereview.appspot.com/2289041/diff/1010/doc/go_spec.html#newcode2263
> doc/go_spec.html:2263: Selectors automatically dereference pointers.
> This needs to be adjusted as well.
> 
> http://codereview.appspot.com/2289041/

[gri, 2010-09-29 00:03:12]

Its fine with me, but I think it will make the use of "embedded" more
common.
- Robert

On Tue, Sep 28, 2010 at 5:00 PM, Russ Cox <rsc@golang.org> wrote:

> I am not proposing to change the preferred name.
> The spec calls them anonymous everywhere,
> and that's consistent and good.
>
> However, they do get referred to as "embedded"
> in other prominent documents and discussions,
> despite previous efforts to eliminate use of that
> term.  This change adds exactly one instance
> of that word only so that someone who visits the
> spec and does a text search for "embed" or
> "embedded" will find the right section.
>
> Is that okay to leave in?
>
> Russ
>

[rsc, 2010-09-29 00:19:55]

PTAL

Changed the selector part that Robert found
and changed the text above it to point at it
to avoid duplication.

[r, 2010-09-29 00:23:54]

LGTM

http://codereview.appspot.com/2289041/diff/10001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2289041/diff/10001/doc/go_spec.html#newcode884
doc/go_spec.html:884: (also called an embedded field).
s/also/colloquially/ ?

[rsc, 2010-09-29 00:26:09]

> s/also/colloquially/ ?

Done.

[rsc, 2010-09-29 00:58:44]

I am not proposing to change the preferred name.
The spec calls them anonymous everywhere,
and that's consistent and good.

However, they do get referred to as "embedded"
in other prominent documents and discussions,
despite previous efforts to eliminate use of that
term.  This change adds exactly one instance
of that word only so that someone who visits the
spec and does a text search for "embed" or
"embedded" will find the right section.

Is that okay to leave in?

Russ

[gri, 2010-09-29 03:49:42]

LGTM

On Tue, Sep 28, 2010 at 12:29 PM, <rsc@golang.org> wrote:

> Reviewers: gri, iant, ken2, r,
>
> Message:
> Hello gri, iant, ken2, r (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> gc, spec, tests: no auto-indirect of pointer to interface value
>
> Implies no embedding of pointer to interface value either.
>
> Please review this at http://codereview.appspot.com/2289041/
>
> Affected files:
>  M doc/go_spec.html
>  M src/cmd/gc/dcl.c
>  M src/cmd/gc/reflect.c
>  M src/cmd/gc/typecheck.c
>  M test/hashmap.go
>  M test/interface/embed.go
>  M test/interface/embed2.go
>  M test/method.go
>  M test/method2.go
>
>
>

[rsc, 2010-09-30 18:59:46]

*** Submitted as 54014073d377 ***

gc, spec, tests: no auto-indirect of pointer to interface value

Implies no embedding of pointer to interface value either.

R=gri, iant, ken2, r, r2
CC=golang-dev
http://codereview.appspot.com/2289041