code review 6217045: go spec: clarify promotion rules for methods/fields of ...

go spec: clarify promotion rules for methods/fields of anonymous fields

Fixes issue 3635.

[gri, 2012-05-17 20:22:56]

Hello rsc, r (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[r, 2012-05-17 20:47:01]

LGTM

[rsc, 2012-05-17 20:56:47]

LGTM

[iant, 2012-05-17 21:14:48]

FYI

http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html#newcode958
doc/go_spec.html:958: <a href="#Method_declarations">Methods</a> of an anonymous
field are promoted
As long as we're looking at this, it doesn't seem precisely correct to me to say
that methods of a field are promoted to be methods of the struct, since it is
the field that is passed to the method, not the struct.  Perhaps "If an
anonymous field itself has fields or methods, they may be used directly with the
outer struct, when there is no ambiguity."

Or really this is fine, just kibbitzing.

[gri, 2012-05-17 21:20:00]

Arguably what there is now is better than what there was before.

I agree that it's still not 100% precise. I think we don't describe
very well what happens to the receiver when using a selector to invoke
a method. Perhaps that's the place where this needs to go.

- gri

On Thu, May 17, 2012 at 2:14 PM,  <iant@golang.org> wrote:
> FYI
>
>
> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html#newcode958
> doc/go_spec.html:958: <a href="#Method_declarations">Methods</a> of an
> anonymous field are promoted
> As long as we're looking at this, it doesn't seem precisely correct to
> me to say that methods of a field are promoted to be methods of the
> struct, since it is the field that is passed to the method, not the
> struct.  Perhaps "If an anonymous field itself has fields or methods,
> they may be used directly with the outer struct, when there is no
> ambiguity."
>
> Or really this is fine, just kibbitzing.
>
> http://codereview.appspot.com/6217045/

[rsc, 2012-05-17 21:23:25]

On Thu, May 17, 2012 at 5:14 PM,  <iant@golang.org> wrote:
> As long as we're looking at this, it doesn't seem precisely correct to
> me to say that methods of a field are promoted to be methods of the
> struct, since it is the field that is passed to the method, not the
> struct.  Perhaps "If an anonymous field itself has fields or methods,
> they may be used directly with the outer struct, when there is no
> ambiguity."

The methods do get promoted in the sense that the struct gets
additional methods that are exactly like its ordinary methods except
that they were auto-generated.  The fields are not as smooth since you
cannot use them in composite literals.

Russ

[kevlar, 2012-05-17 21:27:45]

To me, the term "promotion" is also ambiguous because it seems to imply that
it's a first-class citizen and thus would conflict with an explicit method of
the same name on the class in which the anonymous field is embedded, which is
not the case.

[iant2, 2012-05-17 23:20:57]

Robert Griesemer <gri@golang.org> writes:

> Arguably what there is now is better than what there was before.

Yes, I'm fine with this change.

Ian

>
> I agree that it's still not 100% precise. I think we don't describe
> very well what happens to the receiver when using a selector to invoke
> a method. Perhaps that's the place where this needs to go.
>
> - gri
>
> On Thu, May 17, 2012 at 2:14 PM,  <iant@golang.org> wrote:
>> FYI
>>
>>
>> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html
>> File doc/go_spec.html (right):
>>
>> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html#newcode958
>> doc/go_spec.html:958: <a href="#Method_declarations">Methods</a> of an
>> anonymous field are promoted
>> As long as we're looking at this, it doesn't seem precisely correct to
>> me to say that methods of a field are promoted to be methods of the
>> struct, since it is the field that is passed to the method, not the
>> struct.  Perhaps "If an anonymous field itself has fields or methods,
>> they may be used directly with the outer struct, when there is no
>> ambiguity."
>>
>> Or really this is fine, just kibbitzing.
>>
>> http://codereview.appspot.com/6217045/

[gri, 2012-05-18 00:51:43]

PTAL.

The previous version - while better than what was there originally -
was still not quite right when it came to the rules for the method set
computation. This is an attempt to make this correct, hopefully
without making it overly complicated.

On Thu, May 17, 2012 at 4:20 PM, Ian Lance Taylor <iant@google.com> wrote:
> Robert Griesemer <gri@golang.org> writes:
>
>> Arguably what there is now is better than what there was before.
>
> Yes, I'm fine with this change.
>
> Ian
>
>>
>> I agree that it's still not 100% precise. I think we don't describe
>> very well what happens to the receiver when using a selector to invoke
>> a method. Perhaps that's the place where this needs to go.
>>
>> - gri
>>
>> On Thu, May 17, 2012 at 2:14 PM,  <iant@golang.org> wrote:
>>> FYI
>>>
>>>
>>> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html
>>> File doc/go_spec.html (right):
>>>
>>> http://codereview.appspot.com/6217045/diff/1/doc/go_spec.html#newcode958
>>> doc/go_spec.html:958: <a href="#Method_declarations">Methods</a> of an
>>> anonymous field are promoted
>>> As long as we're looking at this, it doesn't seem precisely correct to
>>> me to say that methods of a field are promoted to be methods of the
>>> struct, since it is the field that is passed to the method, not the
>>> struct.  Perhaps "If an anonymous field itself has fields or methods,
>>> they may be used directly with the outer struct, when there is no
>>> ambiguity."
>>>
>>> Or really this is fine, just kibbitzing.
>>>
>>> http://codereview.appspot.com/6217045/

[kevlar, 2012-05-18 22:40:21]

http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html#newcode683
doc/go_spec.html:683: consists of all methods with receiver type <code>T</code>.
Maybe:

and all unambiguous methods which are in the method set of an embedded field.

http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html#newcode963
doc/go_spec.html:963: The set of such non-ambiguous methods is also included in
the method set of the
Doesn't the list talk about fields too?  Not sure how to fix the wording here,
but the list seems to cover more than its lead-in.

[gri, 2012-05-19 00:54:55]

PTAL.
- gri

On Fri, May 18, 2012 at 3:40 PM,  <kevlar@google.com> wrote:
>
> http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html#newcode683
> doc/go_spec.html:683: consists of all methods with receiver type
> <code>T</code>.
> Maybe:
>
> and all unambiguous methods which are in the method set of an embedded
> field.

This is not precise either, and I rather don't replicate the text from
the structs section. It's complicated for structs, and it's probably
better to leave that complicated part in one place.

>
> http://codereview.appspot.com/6217045/diff/9001/doc/go_spec.html#newcode963
> doc/go_spec.html:963: The set of such non-ambiguous methods is also
> included in the method set of the
> Doesn't the list talk about fields too?  Not sure how to fix the wording
> here, but the list seems to cover more than its lead-in.

Reformulated.

>
> http://codereview.appspot.com/6217045/

[r, 2012-05-20 00:45:49]

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html#newcode688
doc/go_spec.html:688: containing anonymous fields.
'additional rules apply' should be supplemented with a better hint about where
to look for them.

Further rules apply for struct types containing anonymous fields, as described
<here>.

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html#newcode962
doc/go_spec.html:962: of the outer struct when there is no ambiguity.
i know what you mean but this muddies the issue. the phrasing is complex and
depends on subtle terms defined elsewhere and it isn't clear what ambiguity
you're referring to. the prose you're replacing is much clearer, even if it's
wrong. 

but it's only wrong when the context is a composite literal, isn't it? so maybe
fix it by just mentioning the exception.

Except as the elements of composite literals, fields and methods...

[gri, 2012-05-31 21:33:39]

PTAL.

Emphasized the notion of "promoted" method and field. Hopefully made this clear
now.

- gri

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html#newcode688
doc/go_spec.html:688: containing anonymous fields.
On 2012/05/20 00:45:49, r wrote:
> 'additional rules apply' should be supplemented with a better hint about where
> to look for them.
> 
> Further rules apply for struct types containing anonymous fields, as described
> <here>.

Done.

http://codereview.appspot.com/6217045/diff/12002/doc/go_spec.html#newcode962
doc/go_spec.html:962: of the outer struct when there is no ambiguity.
On 2012/05/20 00:45:49, r wrote:
> i know what you mean but this muddies the issue. the phrasing is complex and
> depends on subtle terms defined elsewhere and it isn't clear what ambiguity
> you're referring to. the prose you're replacing is much clearer, even if it's
> wrong. 
> 
> but it's only wrong when the context is a composite literal, isn't it? so
maybe
> fix it by just mentioning the exception.

No. Even if there's no composite literal, conflicting methods won't be promoted
into the method set.

Rephrased.

> 
> Except as the elements of composite literals, fields and methods...

[kevlar, 2012-05-31 23:04:35]

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode962
doc/go_spec.html:962: struct, are <i>promoted</i> to be ordinary fields and
methods of the struct.
s/,//

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode965
doc/go_spec.html:965: <a href="#Composite_literals">composite literals</a> of
the struct.
Should we mention how you can initialize them? (e.g. by initializing the struct
by name)

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode967
doc/go_spec.html:967: Promoted methods are included in the method set of the
struct as follows
Should we mention that the method is not promoted if the embedding struct also
defines a method with that name?

[gri, 2012-05-31 23:34:56]

PTAL

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode962
doc/go_spec.html:962: struct, are <i>promoted</i> to be ordinary fields and
methods of the struct.
On 2012/05/31 23:04:35, kevlar wrote:
> s/,//

Done.

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode965
doc/go_spec.html:965: <a href="#Composite_literals">composite literals</a> of
the struct.
On 2012/05/31 23:04:35, kevlar wrote:
> Should we mention how you can initialize them? (e.g. by initializing the
struct
> by name)

No. This is not a tutorial.

http://codereview.appspot.com/6217045/diff/19001/doc/go_spec.html#newcode967
doc/go_spec.html:967: Promoted methods are included in the method set of the
struct as follows
On 2012/05/31 23:04:35, kevlar wrote:
> Should we mention that the method is not promoted if the embedding struct also
> defines a method with that name?

That is the definition of "promoted": If the embedding struct defines a method m
and the anonymous field has a method m, the embedded m cannot be used legally -
well, x.m denotes the outer m not the inner one.

This does not need to change. Instead, clarified definition of "promoted".

[kevlar, 2012-06-01 00:05:09]

LGTM

[gri, 2012-06-04 16:49:59]

Any others?
- gri

On Thu, May 31, 2012 at 5:05 PM,  <kevlar@google.com> wrote:
> LGTM
>
> http://codereview.appspot.com/6217045/

[r, 2012-06-04 17:16:22]

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode3
doc/go_spec.html:3: "Subtitle": "Version of June 1, 2012",
4

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode965
doc/go_spec.html:965: Promoted fields and methods effectively act like ordinary
fields and methods
s/effectively //

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode966
doc/go_spec.html:966: of a struct. However, promoted fields cannot be used as
field names in
s/. However,/ except that/

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode970
doc/go_spec.html:970: (for a struct type <code>S</code> and a type named
<code>T</code>):
Given struct type <code>S</code> and a type named <code>T</code>, the rules for
promoting an anonymous field of <code>S</code> of type <code>T</code> or
<code>*T</code>are:

</p>

<ul>
</li>
    If the field has type <code>T</code>, ...
etc.

[rsc, 2012-06-04 17:27:00]

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode976
doc/go_spec.html:976: (the <i>promoted method set</i> of <code>T</code>).
This is not a well-formed concept. The promoted method set depends as much on S
as it does on T, since a method defined on S can inhibit the promotion of a
method from T.  I think if we say only:

--
If S contains an anonymous field T, the method sets of S and *S both include
promoted methods with receiver T. The method set of *S also includes promoted
methods with receiver *T.

If S contains an anonymous field *T, the method sets of S and *S both include
promoted methods with receiver T or *T.
--

Then that is loose enough not to hit this problem, and it avoids a third bullet.
(The original text before this CL has the problem too.)

[gri, 2012-06-04 18:23:57]

PTAL

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode3
doc/go_spec.html:3: "Subtitle": "Version of June 1, 2012",
On 2012/06/04 17:16:22, r wrote:
> 4

Done.

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode965
doc/go_spec.html:965: Promoted fields and methods effectively act like ordinary
fields and methods
On 2012/06/04 17:16:22, r wrote:
> s/effectively //

Done.

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode966
doc/go_spec.html:966: of a struct. However, promoted fields cannot be used as
field names in
On 2012/06/04 17:16:22, r wrote:
> s/. However,/ except that/

Done.

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode970
doc/go_spec.html:970: (for a struct type <code>S</code> and a type named
<code>T</code>):
On 2012/06/04 17:16:22, r wrote:
> Given struct type <code>S</code> and a type named <code>T</code>, the rules
for
> promoting an anonymous field of <code>S</code> of type <code>T</code> or
> <code>*T</code>are:
> 

not quite right - here we care only about promoted methods, and the ones of T
rather than S - reformulated

> </p>
> 
> <ul>
> </li>
>     If the field has type <code>T</code>, ...
> etc.

http://codereview.appspot.com/6217045/diff/26001/doc/go_spec.html#newcode976
doc/go_spec.html:976: (the <i>promoted method set</i> of <code>T</code>).
On 2012/06/04 17:27:00, rsc wrote:
> This is not a well-formed concept. The promoted method set depends as much on
S
> as it does on T, since a method defined on S can inhibit the promotion of a
> method from T.  I think if we say only:
> 
> --
> If S contains an anonymous field T, the method sets of S and *S both include
> promoted methods with receiver T. The method set of *S also includes promoted
> methods with receiver *T.
> 
> If S contains an anonymous field *T, the method sets of S and *S both include
> promoted methods with receiver T or *T.
> --
> 
> Then that is loose enough not to hit this problem, and it avoids a third
bullet.
> (The original text before this CL has the problem too.)

Done.

[rsc, 2012-06-04 18:40:01]

LGTM but wait for r

[r, 2012-06-04 18:43:31]

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html#newcode970
doc/go_spec.html:970: promoted methods are included in the method set of the
struct as follows:
i'm not sure. it this feels circular to me. a promoted method is legal, and it's
legal if it's promoted, in which case it's legal.

[gri, 2012-06-04 19:56:17]

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html#newcode970
doc/go_spec.html:970: promoted methods are included in the method set of the
struct as follows:
On 2012/06/04 18:43:31, r wrote:
> i'm not sure. it this feels circular to me. a promoted method is legal, and
it's
> legal if it's promoted, in which case it's legal.

I don't see the circularity:

- First, we defined what a "promoted" method m is: one for which x.m is legal
(as defined in selectors), and where x.m denotes the m in question (rather than
some other m). This seems fine to me.

- Second, we define how method sets include promoted methods. Those method sets
are not further used in this definition.

Perhaps you're hinting at something else?
- gri

[r, 2012-06-04 20:47:44]

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html#newcode970
doc/go_spec.html:970: promoted methods are included in the method set of the
struct as follows:
Maybe not circular but still hard to follow. Let's be clear about the sequence
of definitions.

A field or <a href="#Method_declarations">method</a> <code>f</code> of an
anonymous field in a struct <code>x</code> is called <i>promoted</i> if
the <a href="#Selectors">selector</a> <code>x.f</code> is legal according
the rules defined above. If it is legal, it denotes the field or method
<code>f</code>
of the embedded type.

Given a struct type <code>S</code> and a type named <code>T</code>,
 promoted methods are included in the method set of the struct as follows:


<ul list goes here>

Promoted fields and methods act like ordinary fields and methods
of a struct except that promoted fields cannot be used as field names in
<a href="#Composite_literals">composite literals</a> of the struct.

[iant, 2012-06-04 20:57:16]

LGTM

[gri, 2012-06-04 21:09:45]

PTAL.

Incorporated some changes per r's suggestion.
- gri

On Mon, Jun 4, 2012 at 1:47 PM,  <r@golang.org> wrote:
>
> http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/6217045/diff/32001/doc/go_spec.html#newcode970
> doc/go_spec.html:970: promoted methods are included in the method set of
> the struct as follows:
> Maybe not circular but still hard to follow. Let's be clear about the
> sequence of definitions.
>
> A field or <a href="#Method_declarations">method</a> <code>f</code> of
> an
> anonymous field in a struct <code>x</code> is called <i>promoted</i> if
> the <a href="#Selectors">selector</a> <code>x.f</code> is legal
> according
> the rules defined above. If it is legal, it denotes the field or method
> <code>f</code>
> of the embedded type.
>
> Given a struct type <code>S</code> and a type named <code>T</code>,
>
>  promoted methods are included in the method set of the struct as
> follows:
>
>
> <ul list goes here>
>
> Promoted fields and methods act like ordinary fields and methods
> of a struct except that promoted fields cannot be used as field names in
>
> <a href="#Composite_literals">composite literals</a> of the struct.
>
> http://codereview.appspot.com/6217045/

[r, 2012-06-04 21:11:40]

LGTM yay

[rsc, 2012-06-04 21:12:34]

LGTM

[gri, 2012-06-04 21:24:29]

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

go spec: clarify promotion rules for methods/fields of anonymous fields

Fixes issue 3635.

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