Issue 91230043: code review 91230043: spec: more precise description of select statement (Closed)
Descriptionspec: more precise description of select statement
- use previously defined terms (with links) throughout
- specify evaluation order more precisely (in particular,
the evaluation time of rhs expressions in receive cases
was not specified)
- added extra example case
Not a language change.
Description matches observed behavior of code compiled
with gc and gccgo.
Fixes issue 7669.
Patch Set 1 #Patch Set 2 : diff -r a792685e097b https://code.google.com/p/go/ #Patch Set 3 : diff -r 1ce8d70162aa https://code.google.com/p/go/ #Patch Set 4 : diff -r 1ce8d70162aa https://code.google.com/p/go/ #
Total comments: 8
Patch Set 5 : diff -r e473e77e84ff https://code.google.com/p/go/ #
Total comments: 4
Patch Set 6 : diff -r e473e77e84ff https://code.google.com/p/go/ #
Total comments: 8
Patch Set 7 : diff -r 2c7ba8c04515 https://code.google.com/p/go/ #
Total comments: 11
Patch Set 8 : diff -r 6e10ddc0bbc2 https://code.google.com/p/go/ #
Total comments: 2
Patch Set 9 : diff -r 6e10ddc0bbc2 https://code.google.com/p/go/ #Patch Set 10 : diff -r a24b19322888 https://code.google.com/p/go/ #MessagesTotal messages: 15
Hello r@golang.org, rsc@golang.org, iant@golang.org, ken@golang.org (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
Sign in to reply to this message.
https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4883 doc/go_spec.html:4883: <a href="#Receive_operator">receive operations</a> or Here you say "receive operations" and "send statements" but just below we see RecvStmt and SendStmt. I understand why you don't want to say "receive statement" here, but perhaps you could say "a set of possible send and receive operations". Or "channel operations." I'm not sure. https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4885 doc/go_spec.html:4885: It looks similar to a "switch" statement but with the Link to #Switch_statements? https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4886 doc/go_spec.html:4886: cases all referring to communication operations. s/communication/channel/ ? https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4932 doc/go_spec.html:4932: The communication operation (if present) of the selected case is executed. By "if present" are you referring to the default case? If so, be more explicit, because otherwise this suggests that there is some way to have a case without a communication operation.
Sign in to reply to this message.
PTAL https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4883 doc/go_spec.html:4883: <a href="#Receive_operator">receive operations</a> or On 2014/05/07 21:10:38, iant wrote: > Here you say "receive operations" and "send statements" but just below we see > RecvStmt and SendStmt. I understand why you don't want to say "receive > statement" here, but perhaps you could say "a set of possible send and receive > operations". Or "channel operations." I'm not sure. Done. https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4885 doc/go_spec.html:4885: It looks similar to a "switch" statement but with the On 2014/05/07 21:10:37, iant wrote: > Link to #Switch_statements? Done. https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4886 doc/go_spec.html:4886: cases all referring to communication operations. On 2014/05/07 21:10:38, iant wrote: > s/communication/channel/ ? There are other channel "operations" such as close(), len(), select, etc. Communication is more clearly referring to send and receive. Leaving as is. https://codereview.appspot.com/91230043/diff/60001/doc/go_spec.html#newcode4932 doc/go_spec.html:4932: The communication operation (if present) of the selected case is executed. On 2014/05/07 21:10:37, iant wrote: > By "if present" are you referring to the default case? If so, be more explicit, > because otherwise this suggests that there is some way to have a case without a > communication operation. I am. Rephrased.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html#newcode4968 doc/go_spec.html:4968: case a[f()] = <-c4: s/</</ https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html#newcode4970 doc/go_spec.html:4970: // case t := <-c4 s/</</
Sign in to reply to this message.
PTAL https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html#newcode4968 doc/go_spec.html:4968: case a[f()] = <-c4: On 2014/05/07 23:02:52, josharian wrote: > s/</</ Done. https://codereview.appspot.com/91230043/diff/60002/doc/go_spec.html#newcode4970 doc/go_spec.html:4970: // case t := <-c4 On 2014/05/07 23:02:53, josharian wrote: > s/</</ Done.
Sign in to reply to this message.
https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4925 doc/go_spec.html:4925: After excluding any <code>nil</code> channels from this result set, if any of we talk about operations proceeding, not channels. it's a minor point but avoids some awkwardness on the next line about a channel having a corresponding communication operation (a channel might be listed multiple times). i think you can deemphasize the part about nil, because nil channels are no longer special to select. (they behave the same as in ordinary sends and receives.) If any of the communications can proceed, a single one is chosen via a uniform pseudo-random selection. Otherwise, if there is a default case, that case is chosen. If there is no default case, the select statement blocks until a communication can proceed. https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4939 doc/go_spec.html:4939: If the selected case contains a RecvStmt with an assignment, the left-hand side s/contains/is/ https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4944 doc/go_spec.html:4944: The statements in the statement list of the selected case are executed. The statement list of the selected case is executed. ? https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4949 doc/go_spec.html:4949: If there are no cases with non-<code>nil</code> channels, and no default case, Note that since communication on nil channels can never proceed, a select with no non-nil channels and no default case blocks forever.
Sign in to reply to this message.
PTAL https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4925 doc/go_spec.html:4925: After excluding any <code>nil</code> channels from this result set, if any of On 2014/05/08 14:14:25, rsc wrote: > we talk about operations proceeding, not channels. it's a minor point but avoids > some awkwardness on the next line about a channel having a corresponding > communication operation (a channel might be listed multiple times). i think you > can deemphasize the part about nil, because nil channels are no longer special > to select. (they behave the same as in ordinary sends and receives.) > > If any of the communications can proceed, a single one is chosen via a uniform > pseudo-random selection. > Otherwise, if there is a default case, that case is chosen. > If there is no default case, the select statement blocks until a communication > can proceed. Done. https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4939 doc/go_spec.html:4939: If the selected case contains a RecvStmt with an assignment, the left-hand side On 2014/05/08 14:14:25, rsc wrote: > s/contains/is/ Done. https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4944 doc/go_spec.html:4944: The statements in the statement list of the selected case are executed. On 2014/05/08 14:14:25, rsc wrote: > The statement list of the selected case is executed. > ? Done. https://codereview.appspot.com/91230043/diff/80001/doc/go_spec.html#newcode4949 doc/go_spec.html:4949: If there are no cases with non-<code>nil</code> channels, and no default case, On 2014/05/08 14:14:25, rsc wrote: > Note that since communication on nil channels can never proceed, a select with > no non-nil channels and no default case blocks forever. Done.
Sign in to reply to this message.
https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4921 doc/go_spec.html:4921: Expressions on the left-hand side of a RecvExpr are not yet evaluated. i think you mean RecvStmt here, and should be clearer: Expressions on the left-hand side of a RecvStmt with a short variable declaration or assignment are not yet evaluated. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4925 doc/go_spec.html:4925: If any of the communications can proceed, s/any/one or more/ https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4926 doc/go_spec.html:4926: a single one is chosen via a uniform pseudo-random selection. s/one/one that can proceed/ https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4938 doc/go_spec.html:4938: If the selected case is a RecvStmt with an assignment, the left-hand side s/assignment/short variable declaration or &/ https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4948 doc/go_spec.html:4948: Note that since communication on <code>nil</code> channels can never proceed, s/Note that s/S/
Sign in to reply to this message.
PTAL https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4921 doc/go_spec.html:4921: Expressions on the left-hand side of a RecvExpr are not yet evaluated. On 2014/05/13 22:14:34, r wrote: > i think you mean RecvStmt here, and should be clearer: > > Expressions on the left-hand side of a RecvStmt with a short variable > declaration or assignment are not yet evaluated. Done. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4925 doc/go_spec.html:4925: If any of the communications can proceed, On 2014/05/13 22:14:34, r wrote: > s/any/one or more/ Done. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4926 doc/go_spec.html:4926: a single one is chosen via a uniform pseudo-random selection. On 2014/05/13 22:14:34, r wrote: > s/one/one that can proceed/ Done. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4926 doc/go_spec.html:4926: a single one is chosen via a uniform pseudo-random selection. On 2014/05/13 22:14:34, r wrote: > s/one/one that can proceed/ Done. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4938 doc/go_spec.html:4938: If the selected case is a RecvStmt with an assignment, the left-hand side On 2014/05/13 22:14:34, r wrote: > s/assignment/short variable declaration or &/ Done. https://codereview.appspot.com/91230043/diff/100001/doc/go_spec.html#newcode4948 doc/go_spec.html:4948: Note that since communication on <code>nil</code> channels can never proceed, On 2014/05/13 22:14:34, r wrote: > s/Note that s/S/ Done.
Sign in to reply to this message.
LGTM https://codereview.appspot.com/91230043/diff/120001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/120001/doc/go_spec.html#newcode4930 doc/go_spec.html:4930: one of the communications can proceed. s/one/at least one/ ?
Sign in to reply to this message.
PTAL. rsc? https://codereview.appspot.com/91230043/diff/120001/doc/go_spec.html File doc/go_spec.html (right): https://codereview.appspot.com/91230043/diff/120001/doc/go_spec.html#newcode4930 doc/go_spec.html:4930: one of the communications can proceed. On 2014/05/13 22:57:12, r wrote: > s/one/at least one/ ? Done.
Sign in to reply to this message.
LGTM
Sign in to reply to this message.
*** Submitted as https://code.google.com/p/go/source/detail?r=c0c00145e2d3 *** spec: more precise description of select statement - use previously defined terms (with links) throughout - specify evaluation order more precisely (in particular, the evaluation time of rhs expressions in receive cases was not specified) - added extra example case Not a language change. Description matches observed behavior of code compiled with gc and gccgo. Fixes issue 7669. LGTM=iant, r, rsc R=r, rsc, iant, ken, josharian CC=golang-codereviews https://codereview.appspot.com/91230043
Sign in to reply to this message.
|
