code review 2226047: go spec: clarifications for range clause

go spec: clarifications for range clause

[gri, 2010-09-27 23:46:36]

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

I'd like you to review this change.

[r, 2010-09-27 23:56:39]

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3950
doc/go_spec.html:3950: iterates sequentially through all entries of an array,
slice, string or map,
s/sequentially //

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3953
doc/go_spec.html:3953: <i>iteration variable</i> - or the current (index,
element) or (key, value) pair
the -'s should be ems.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3954
doc/go_spec.html:3954: to a pair of iteration variables - and then executes the
block.
the grouping makes this hard to parse. i think you need to break it down, with
maybe a sentence for one var, a sentence for two.

actually, after reading the description below, i think you can just toss most of
this and replace it with something very general

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: array, slice, string or map, or it may be a channel.
must/may? just say ... map, or channel.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3971
doc/go_spec.html:3971: array, slice, or string element or map value.
this makes it sound too complex.  maybe use a table.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3979
doc/go_spec.html:3979: On each loop iteration, the values produced by the range
clause are assigned to
s/loop //

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode4003
doc/go_spec.html:4003: If map entries that have not yet been processed are
deleted during iteration,
"processed" isn't the right word.  maybe scanned? iterated? reached?

[iant, 2010-09-28 00:03:31]

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3574
doc/go_spec.html:3574: The operand must be <a
href="#Address_operators">addressable</a>.
This is a change--both 6g and gccgo currently accept
    m := make(map[int]int)
    m[0]++
but map index expressions are not addressable.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3965
doc/go_spec.html:3965: Except for channels, the left-hand side expression list
must contain one or two
This is a little confusing because you say "except for channels" but the channel
case is far down.  I think you should say instead something like "Channels are
described below."

[rsc, 2010-09-28 00:42:24]

I don't believe the range is a left-over.
x[i] is valid when x is a pointer to an array,
so I think for i = range x should continue
to be valid too.

Russ

[gri, 2010-09-28 01:58:05]

PTAL.

Significantly simplified by using a table.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3574
doc/go_spec.html:3574: The operand must be <a
href="#Address_operators">addressable</a>.
On 2010/09/28 00:03:32, iant wrote:
> This is a change--both 6g and gccgo currently accept
>     m := make(map[int]int)
>     m[0]++
> but map index expressions are not addressable.

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3950
doc/go_spec.html:3950: iterates sequentially through all entries of an array,
slice, string or map,
On 2010/09/27 23:56:39, r wrote:
> s/sequentially //

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3953
doc/go_spec.html:3953: <i>iteration variable</i> - or the current (index,
element) or (key, value) pair
On 2010/09/27 23:56:39, r wrote:
> the -'s should be ems.

section removed

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3954
doc/go_spec.html:3954: to a pair of iteration variables - and then executes the
block.
On 2010/09/27 23:56:39, r wrote:
> the grouping makes this hard to parse. i think you need to break it down, with
> maybe a sentence for one var, a sentence for two.
> 
> actually, after reading the description below, i think you can just toss most
of
> this and replace it with something very general

section removed

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: array, slice, string or map, or it may be a channel.
On 2010/09/27 23:56:39, r wrote:
> must/may? just say ... map, or channel.

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3965
doc/go_spec.html:3965: Except for channels, the left-hand side expression list
must contain one or two
On 2010/09/28 00:03:32, iant wrote:
> This is a little confusing because you say "except for channels" but the
channel
> case is far down.  I think you should say instead something like "Channels are
> described below."

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3971
doc/go_spec.html:3971: array, slice, or string element or map value.
On 2010/09/27 23:56:39, r wrote:
> this makes it sound too complex.  maybe use a table.

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode3979
doc/go_spec.html:3979: On each loop iteration, the values produced by the range
clause are assigned to
On 2010/09/27 23:56:39, r wrote:
> s/loop //

Done.

http://codereview.appspot.com/2226047/diff/3/doc/go_spec.html#newcode4003
doc/go_spec.html:4003: If map entries that have not yet been processed are
deleted during iteration,
On 2010/09/27 23:56:39, r wrote:
> "processed" isn't the right word.  maybe scanned? iterated? reached?

Done.

[r, 2010-09-28 04:17:39]

a lot of comments but i think it's much better

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

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3951
doc/go_spec.html:3951: iterates through all entries of an array, pointer to an
array, slice, string or map,
i know what you mean but there are no entries in a pointer to an array. i think
it's hard to fix inline. it might be simpler to write a second sentence about
pointer to array but maybe you'll see a way.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3953
doc/go_spec.html:3953: to corresponding <i>iteration variables</i> and then
executes the block.
the iteration variable might not be a variable, although i don't have a better
term.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: It may be an array, pointer to an array, slice, string,
map, or channel.
pointer to array is OK here.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3983
doc/go_spec.html:3983: string   s  string type    index    i  int       code
point at s[i]  int
"code point at" isn't right.  maybe use a footnote or extra line of the table to
clarify

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3995
doc/go_spec.html:3995: increasing order, starting with index 0.
is there a new variable each loop, or is the variable re-used? (affects behavior
of closures)  "values are produced" doesn't clarify the issue.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode4020
doc/go_spec.html:4020: before the channel is closed.
reference close?

[gri, 2010-09-28 17:40:13]

PTAL

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

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3951
doc/go_spec.html:3951: iterates through all entries of an array, pointer to an
array, slice, string or map,
On 2010/09/28 04:17:40, r wrote:
> i know what you mean but there are no entries in a pointer to an array. i
think
> it's hard to fix inline. it might be simpler to write a second sentence about
> pointer to array but maybe you'll see a way.

I think I can talk about arrays in general here and leave the pointer to array
away since we specify the range type in detail below.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3953
doc/go_spec.html:3953: to corresponding <i>iteration variables</i> and then
executes the block.
On 2010/09/28 04:17:40, r wrote:
> the iteration variable might not be a variable, although i don't have a better
> term.

This was the term used before.
Again, we are just giving an overview of the range clause here. I think this
should be ok.

(As an aside, if we would use the notion of a  "variable" for anything where one
can store something into, several places could be simplified, I think. Some
variables (the ones we declare) simply have explicit names. Others are anonymous
(when allocated on the heap) or are denoted through primary expressions.)

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: It may be an array, pointer to an array, slice, string,
map, or channel.
On 2010/09/28 04:17:40, r wrote:
> pointer to array is OK here.

Done.

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3983
doc/go_spec.html:3983: string   s  string type    index    i  int       code
point at s[i]  int
On 2010/09/28 04:17:40, r wrote:
> "code point at" isn't right.  maybe use a footnote or extra line of the table
to
> clarify
changed

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3995
doc/go_spec.html:3995: increasing order, starting with index 0.
On 2010/09/28 04:17:40, r wrote:
> is there a new variable each loop, or is the variable re-used? (affects
behavior
> of closures)  "values are produced" doesn't clarify the issue.
clarified below

http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode4020
doc/go_spec.html:4020: before the channel is closed.
On 2010/09/28 04:17:40, r wrote:
> reference close?

Done.

[gri, 2010-09-28 17:45:12]

PTAL (I think I sent it out before but it's not showing up).
- gri

On Mon, Sep 27, 2010 at 9:17 PM, <r@golang.org> wrote:

> a lot of comments but i think it's much better
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html
>
> File doc/go_spec.html (right):
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3951
> doc/go_spec.html:3951: iterates through all entries of an array, pointer
> to an array, slice, string or map,
> i know what you mean but there are no entries in a pointer to an array.
> i think it's hard to fix inline. it might be simpler to write a second
> sentence about pointer to array but maybe you'll see a way.
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3953
> doc/go_spec.html:3953: to corresponding <i>iteration variables</i> and
> then executes the block.
> the iteration variable might not be a variable, although i don't have a
> better term.
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3963
> doc/go_spec.html:3963: It may be an array, pointer to an array, slice,
> string, map, or channel.
> pointer to array is OK here.
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3983
> doc/go_spec.html:3983: string   s  string type    index    i  int
> code point at s[i]  int
> "code point at" isn't right.  maybe use a footnote or extra line of the
> table to clarify
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3995
> doc/go_spec.html:3995: increasing order, starting with index 0.
> is there a new variable each loop, or is the variable re-used? (affects
> behavior of closures)  "values are produced" doesn't clarify the issue.
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode4020
> doc/go_spec.html:4020: before the channel is closed.
> reference close?
>
>
> http://codereview.appspot.com/2226047/
>

[iant, 2010-09-28 17:57:15]

LGTM

Much nicer.

[r, 2010-09-28 18:09:02]

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: It may be an array, pointer to an array, slice, string,
map, or channel.
what is "It"? the type or the expression?

The expression may be of type array, pointer to array, ...

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3965
doc/go_spec.html:3965: As with an assignment, the left-hand side operands must
be
in the previous paragraph it was a "right-hand expression"; here it's a
"left-hand side operand".  sounds like you're talking about a side operand.  at
least drop "side", but the wording is poor. how about "operands on the left" and
"expression on the right"?

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3967
doc/go_spec.html:3967: denote the iteration variables. If the range type is a
channel, only
iteration variables: italic?

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3968
doc/go_spec.html:3968: one iteration variable is permitted.
otherwise there may be one or two.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3972
doc/go_spec.html:3972: The expression on the right hand side is evaluated once
before beginning the loop.
s/hand side // here and below.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3988
doc/go_spec.html:3988: For an array or slice value, the index iteration values
are produced in
or string

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode4028
doc/go_spec.html:4028: statement; they are (re-)used in each iteration.
s/()//

[gri, 2010-09-28 19:04:55]

PTAL

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: It may be an array, pointer to an array, slice, string,
map, or channel.
On 2010/09/28 18:09:02, r wrote:
> what is "It"? the type or the expression?
> 
> The expression may be of type array, pointer to array, ...

Done.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3965
doc/go_spec.html:3965: As with an assignment, the left-hand side operands must
be
On 2010/09/28 18:09:02, r wrote:
> in the previous paragraph it was a "right-hand expression"; here it's a
> "left-hand side operand".  sounds like you're talking about a side operand. 
at
> least drop "side", but the wording is poor. how about "operands on the left"
and
> "expression on the right"?

Done.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3967
doc/go_spec.html:3967: denote the iteration variables. If the range type is a
channel, only
On 2010/09/28 18:09:02, r wrote:
> iteration variables: italic?
defined before

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3968
doc/go_spec.html:3968: one iteration variable is permitted.
On 2010/09/28 18:09:02, r wrote:
> otherwise there may be one or two.

Done.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3972
doc/go_spec.html:3972: The expression on the right hand side is evaluated once
before beginning the loop.
On 2010/09/28 18:09:02, r wrote:
> s/hand side // here and below.

Done.

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode3988
doc/go_spec.html:3988: For an array or slice value, the index iteration values
are produced in
On 2010/09/28 18:09:02, r wrote:
> or string

expanded in string

http://codereview.appspot.com/2226047/diff/10003/doc/go_spec.html#newcode4028
doc/go_spec.html:4028: statement; they are (re-)used in each iteration.
On 2010/09/28 18:09:02, r wrote:
> s/()//

Done.

[rog, 2010-09-28 19:52:35]

FWIW, i'd prefer it if the iteration variables were declared
anew each time through the loop. the only down side
is that if you take the address of it, you'll produce
more garbage; but that's probably relatively rare.

On 28 September 2010 18:40,  <gri@golang.org> wrote:
> PTAL
>
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3951
> doc/go_spec.html:3951: iterates through all entries of an array, pointer
> to an array, slice, string or map,
> On 2010/09/28 04:17:40, r wrote:
>>
>> i know what you mean but there are no entries in a pointer to an
>
> array. i think
>>
>> it's hard to fix inline. it might be simpler to write a second
>
> sentence about
>>
>> pointer to array but maybe you'll see a way.
>
> I think I can talk about arrays in general here and leave the pointer to
> array away since we specify the range type in detail below.
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3953
> doc/go_spec.html:3953: to corresponding <i>iteration variables</i> and
> then executes the block.
> On 2010/09/28 04:17:40, r wrote:
>>
>> the iteration variable might not be a variable, although i don't have
>
> a better
>>
>> term.
>
> This was the term used before.
> Again, we are just giving an overview of the range clause here. I think
> this should be ok.
>
> (As an aside, if we would use the notion of a  "variable" for anything
> where one can store something into, several places could be simplified,
> I think. Some variables (the ones we declare) simply have explicit
> names. Others are anonymous (when allocated on the heap) or are denoted
> through primary expressions.)
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3963
> doc/go_spec.html:3963: It may be an array, pointer to an array, slice,
> string, map, or channel.
> On 2010/09/28 04:17:40, r wrote:
>>
>> pointer to array is OK here.
>
> Done.
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3983
> doc/go_spec.html:3983: string   s  string type    index    i  int
> code point at s[i]  int
> On 2010/09/28 04:17:40, r wrote:
>>
>> "code point at" isn't right.  maybe use a footnote or extra line of
>
> the table to
>>
>> clarify
>
> changed
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode3995
> doc/go_spec.html:3995: increasing order, starting with index 0.
> On 2010/09/28 04:17:40, r wrote:
>>
>> is there a new variable each loop, or is the variable re-used?
>
> (affects behavior
>>
>> of closures)  "values are produced" doesn't clarify the issue.
>
> clarified below
>
> http://codereview.appspot.com/2226047/diff/14001/doc/go_spec.html#newcode4020
> doc/go_spec.html:4020: before the channel is closed.
> On 2010/09/28 04:17:40, r wrote:
>>
>> reference close?
>
> Done.
>
> http://codereview.appspot.com/2226047/
>

[rsc, 2010-09-28 19:55:32]

On Tue, Sep 28, 2010 at 15:52, roger peppe <rogpeppe@gmail.com> wrote:
> FWIW, i'd prefer it if the iteration variables were declared
> anew each time through the loop. the only down side
> is that if you take the address of it, you'll produce
> more garbage; but that's probably relatively rare.

This is a well-trodden topic on golang-nuts.
This CL is about clarifying the spec, not changing it.

Russ

[rog, 2010-09-28 20:00:08]

ahem, yes, sorry.

On 28 September 2010 20:55, Russ Cox <rsc@golang.org> wrote:
> On Tue, Sep 28, 2010 at 15:52, roger peppe <rogpeppe@gmail.com> wrote:
>> FWIW, i'd prefer it if the iteration variables were declared
>> anew each time through the loop. the only down side
>> is that if you take the address of it, you'll produce
>> more garbage; but that's probably relatively rare.
>
> This is a well-trodden topic on golang-nuts.
> This CL is about clarifying the spec, not changing it.
>
> Russ
>

[r, 2010-09-28 20:22:56]

LGTM

http://codereview.appspot.com/2226047/diff/25001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2226047/diff/25001/doc/go_spec.html#newcode3963
doc/go_spec.html:3963: 
d

[rsc1, 2010-09-28 21:11:54]

LGTM

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

http://codereview.appspot.com/2226047/diff/32001/doc/go_spec.html#newcode4023
doc/go_spec.html:4023: The iteration variables may be declared by the "range"
clause (":="). In this
probably <code>:=</code>

[gri, 2010-09-28 21:44:26]

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

go spec: clarifications for range clause

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