code review 7100046: doc/go_spec.html: clarification about insertion during ...

doc/go_spec.html: clarification about insertion during map iteration

[mdempsky, 2013-01-11 19:38:21]

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html#newcode4415
doc/go_spec.html:4415: the iteration of the original entries won't be affected.
If the map is
I don't think it's necessary to specify that.  It's not visible to the user if
the remaining entries are reordered or not, and the spec already earlier stated
that iteration proceeds over all entries.

[minux1, 2013-01-11 19:44:08]

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html#newcode4415
doc/go_spec.html:4415: the iteration of the original entries won't be affected.
If the map is
On 2013/01/11 19:38:21, mdempsky wrote:
> I don't think it's necessary to specify that.  It's not visible to the user if
> the remaining entries are reordered or not, and the spec already earlier
stated
> that iteration proceeds over all entries.
my reason for adding this clause is that I think by saying
"the behavior is implementation-dependent", it basically
means that all previous guarantees are invalid in this case.

[mdempsky, 2013-01-11 20:01:15]

[FYI, I'm going to drop my CL, and so we can focus on this CL.]

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/2001/doc/go_spec.html#newcode4415
doc/go_spec.html:4415: the iteration of the original entries won't be affected.
If the map is
Perhaps it would be better to clarify what "behavior" means.  Something like:

"It is implementation-dependent whether inserting a map entry causes active
iterations to produce additional iteration values.  If so, each inserted entry
produces at most one set of iteration values."

A go word-smith can probably improve the wording, but this way it's clear that
if you have nested iterations of the same map like:

  for a := range m {
    m[f()] = g()
    for b := range m {
    }
  }

that newly inserted keys are still guaranteed to be visited by the "b"
iteration, even if they're implementation-dependent for the "a" iteration.

[minux1, 2013-01-11 22:54:23]

Hello golang-dev@googlegroups.com, mdempsky@google.com (cc:
golang-dev@googlegroups.com),

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

[minux1, 2013-01-11 22:55:44]

On Sat, Jan 12, 2013 at 4:01 AM, <mdempsky@google.com> wrote:

> "It is implementation-dependent whether inserting a map entry causes
> active iterations to produce additional iteration values.  If so, each
> inserted entry produces at most one set of iteration values."
>
Yeah, it is a good idea to restrict the extent of implementation-dependent
behavior. I adopt your version with slight changes. PTAL.

I want to avoid introducing the concept of "active iterations" as it's not
mentioned elsewhere.

[mdempsky, 2013-01-12 00:02:17]

On 2013/01/11 22:55:44, minux wrote:
> I want to avoid introducing the concept of "active iterations" as it's not
> mentioned elsewhere.

Fair enough.  For what it's worth, it looks like "executing" might be a wording
that would be more consistent with the rest of the spec.  Also, this section
should perhaps say "removed" and "added" instead of "deleted" and "inserted" to
be consistent with the other sections talking about maps.

Your wording seems semantically fine to me though.

[minux1, 2013-01-12 00:11:18]

Hello golang-dev@googlegroups.com, mdempsky@google.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[mdempsky, 2013-01-12 00:12:28]

LGTM

[minux1, 2013-01-12 00:12:43]

On 2013/01/12 00:02:17, mdempsky wrote:
> Fair enough.  For what it's worth, it looks like "executing" might be a
wording
are you suggesting that we replace "the iteration" with "executing"?
sorry, I don't understand here.
> that would be more consistent with the rest of the spec.  Also, this section
> should perhaps say "removed" and "added" instead of "deleted" and "inserted"
to
> be consistent with the other sections talking about maps.
Done.

[minux1, 2013-01-12 00:15:47]

+iant, r, gri, rsc, ken2

[mdempsky, 2013-01-12 00:21:07]

On 2013/01/12 00:12:43, minux wrote:
> are you suggesting that we replace "the iteration" with "executing"?

No, I just meant that saying "executing iterations" might have been better than
"active iterations" since the spec does discuss executing statements.  It's moot
though since we're not going with that proposed wording. :)

[iant, 2013-01-12 02:28:42]

https://codereview.appspot.com/7100046/diff/2003/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/2003/doc/go_spec.html#newcode4413
doc/go_spec.html:4413: added during iteration, it's implementation-dependent
whether the newly
s/it's/it is/

https://codereview.appspot.com/7100046/diff/2003/doc/go_spec.html#newcode4414
doc/go_spec.html:4414: added entry will be produced in the iteration, but if
they do, each added
Sentence is too long and is not parallel ("the newly added entry" doesn't match
"they do").

How about:

If map entries are added during iteration, it is implementation dependent
whether they will be produced in the iteration.  Each newly added entry will be
produced not at all or exactly once.

[minux1, 2013-01-12 08:30:48]

PTAL.

[iant, 2013-01-12 22:27:31]

LGTM

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4415
doc/go_spec.html:4415: all or exactly once.If the map is <code>nil</code>, the
number of iterations
Two spaces after "once."

[gri, 2013-01-12 22:31:34]

FYI.

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode3
doc/go_spec.html:3: "Subtitle": "Version of January 11, 2013",
up-date

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4411
doc/go_spec.html:4411: If map entries that have not yet been reached are removed
during iteration,
I may have missed the prev. discussion but I don't understand why this was
changed. "delete" is an operation defined on maps, but "remove" is not.

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4413
doc/go_spec.html:4413: added during iteration, it is implementation dependent
whether they will be
same: inserted seemed better than 'added'.

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4414
doc/go_spec.html:4414: produced in the iteration.  Each newly added entry will
be produced not at
s/./:/

[gri, 2013-01-12 22:32:45]

ok, I see the discussion about deleted vs removed and added vs
inserted. let me re-read.
- gri

On Sat, Jan 12, 2013 at 2:31 PM,  <gri@golang.org> wrote:
> FYI.
>
>
>
>
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html
> File doc/go_spec.html (right):
>
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode3
> doc/go_spec.html:3: "Subtitle": "Version of January 11, 2013",
> up-date
>
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4411
> doc/go_spec.html:4411: If map entries that have not yet been reached are
> removed during iteration,
> I may have missed the prev. discussion but I don't understand why this
> was changed. "delete" is an operation defined on maps, but "remove" is
> not.
>
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4413
> doc/go_spec.html:4413: added during iteration, it is implementation
>
> dependent whether they will be
> same: inserted seemed better than 'added'.
>
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4414
> doc/go_spec.html:4414: produced in the iteration.  Each newly added
>
> entry will be produced not at
> s/./:/
>
> https://codereview.appspot.com/7100046/

[gri, 2013-01-12 22:35:11]

LGTM

But I think there should be a ':' instead of a '.' (see previous comments).

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4411
doc/go_spec.html:4411: If map entries that have not yet been reached are removed
during iteration,
On 2013/01/12 22:31:34, gri wrote:
> I may have missed the prev. discussion but I don't understand why this was
> changed. "delete" is an operation defined on maps, but "remove" is not.

Ok. 'remove' is fine.

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4413
doc/go_spec.html:4413: added during iteration, it is implementation dependent
whether they will be
On 2013/01/12 22:31:34, gri wrote:
> same: inserted seemed better than 'added'.

'added' is fine.

[mdempsky, 2013-01-13 02:19:19]

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4414
doc/go_spec.html:4414: produced in the iteration.  Each newly added entry will
be produced not at
Is it worth stating somehow that the decision of whether a newly added entry
will produce an iteration value can be made independently for each entry and
each execution?  I can imagine this wording being construed as limiting
implementations to either "new entries always produce iteration values" or "new
entries never produce iteration values".

I'm also curious if the intention is to allow the same key to produce multiple
iteration values if it's removed and re-added during iteration.  E.g., is this
code guaranteed to terminate or is it allowed to infinite loop?

  m := make(map[int]int)
  m[0] = 0
  for k, v := range m {
    delete(m, k)
    m[k] = v
  }

[minux1, 2013-01-17 10:55:39]

On 2013/01/13 02:19:19, mdempsky wrote:
> https://codereview.appspot.com/7100046/diff/9005/doc/go_spec.html#newcode4414
> doc/go_spec.html:4414: produced in the iteration.  Each newly added entry will
> be produced not at
> Is it worth stating somehow that the decision of whether a newly added entry
> will produce an iteration value can be made independently for each entry and
> each execution?  I can imagine this wording being construed as limiting
> implementations to either "new entries always produce iteration values" or
"new
> entries never produce iteration values".
well, i don't know. anyone has comments on this aspect?

> I'm also curious if the intention is to allow the same key to produce multiple
> iteration values if it's removed and re-added during iteration.  E.g., is this
> code guaranteed to terminate or is it allowed to infinite loop?
> 
>   m := make(map[int]int)
>   m[0] = 0
>   for k, v := range m {
>     delete(m, k)
>     m[k] = v
>   }
the status quo: this code always terminate on both gc and gccgo implementations.

[rsc, 2013-01-17 14:27:16]

Suggestion.

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html#newcode4411
doc/go_spec.html:4411: If map entries that have not yet been reached are removed
during iteration,
This should remain "deleted" (not "removed"). The builtin is called delete.

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html#newcode4412
doc/go_spec.html:4412: the corresponding iteration values will not be produced.
If map entries are
If a map entry is created during iteration, that entry may be produced during
the iteration or may be skipped. The choice may vary for each entry created and
from one iteration to the next.

[minux1, 2013-01-17 14:45:53]

PTAL.

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html#newcode4411
doc/go_spec.html:4411: If map entries that have not yet been reached are removed
during iteration,
On 2013/01/17 14:27:16, rsc wrote:
> This should remain "deleted" (not "removed"). The builtin is called delete.
But other part of the spec use the word "remove" for this.

https://codereview.appspot.com/7100046/diff/3006/doc/go_spec.html#newcode4412
doc/go_spec.html:4412: the corresponding iteration values will not be produced.
If map entries are
On 2013/01/17 14:27:16, rsc wrote:
> If a map entry is created during iteration, that entry may be produced during
> the iteration or may be skipped. The choice may vary for each entry created
and
> from one iteration to the next.
Adopted.

[rsc, 2013-01-17 14:56:20]

LGTM

[minux1, 2013-01-17 15:11:45]

*** Submitted as https://code.google.com/p/go/source/detail?r=9c036de21ee7 ***

doc/go_spec.html: clarification about insertion during map iteration

R=mdempsky, iant, r, gri, rsc, ken
CC=golang-dev
https://codereview.appspot.com/7100046