|
|
Created:
12 years, 10 months ago by gri Modified:
12 years, 9 months ago Reviewers:
CC:
rsc, r, iant, kevlar, iant2, golang-dev Visibility:
Public. |
Descriptiongo spec: clarify promotion rules for methods/fields of anonymous fields
Fixes issue 3635.
Patch Set 1 #
Total comments: 1
Patch Set 2 : diff -r d263b323e582 https://code.google.com/p/go #Patch Set 3 : diff -r 820ffde8c396 https://code.google.com/p/go #
Total comments: 2
Patch Set 4 : diff -r e92c07f19c3a https://code.google.com/p/go #
Total comments: 4
Patch Set 5 : diff -r d34e1877830c https://code.google.com/p/go #Patch Set 6 : diff -r d34e1877830c https://code.google.com/p/go #
Total comments: 6
Patch Set 7 : diff -r d34e1877830c https://code.google.com/p/go #Patch Set 8 : diff -r d34e1877830c https://code.google.com/p/go #Patch Set 9 : diff -r d34e1877830c https://code.google.com/p/go #Patch Set 10 : diff -r 5eb6fe483779 https://code.google.com/p/go #
Total comments: 10
Patch Set 11 : diff -r 85022208a2aa https://code.google.com/p/go #
Total comments: 3
Patch Set 12 : diff -r 685b1b62a34f https://code.google.com/p/go #Patch Set 13 : diff -r 1b8e032d6ad6 https://code.google.com/p/go #MessagesTotal messages: 29
Hello rsc, r (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
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.
Sign in to reply to this message.
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/
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
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/
Sign in to reply to this message.
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/
Sign in to reply to this message.
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.
Sign in to reply to this message.
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/
Sign in to reply to this message.
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...
Sign in to reply to this message.
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...
Sign in to reply to this message.
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?
Sign in to reply to this message.
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".
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
Any others? - gri On Thu, May 31, 2012 at 5:05 PM, <kevlar@google.com> wrote: > LGTM > > http://codereview.appspot.com/6217045/
Sign in to reply to this message.
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.
Sign in to reply to this message.
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.)
Sign in to reply to this message.
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.
Sign in to reply to this message.
LGTM but wait for r
Sign in to reply to this message.
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.
Sign in to reply to this message.
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
Sign in to reply to this message.
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.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
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/
Sign in to reply to this message.
LGTM yay
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
*** 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
Sign in to reply to this message.
|