code review 6567062: go spec: arguments for append may overlap

go spec: arguments for append may overlap

Fixes issue 4142.

[gri, 2012-09-27 18:12:26]

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

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

[gri, 2012-09-27 18:13:57]

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

Please take another look.

[iant, 2012-09-27 18:30:53]

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

https://codereview.appspot.com/6567062/diff/2001/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The underlying arrays of <code>s</code> and a slice
argument <code>x...</code>
Why not just repeat what we say for copy: "Source and destination may overlap." 
Expressing this in terms of the underlying arrays makes it sound like we are
trying to say there is some case which will not work, but I think all cases will
work as expected.

[gri, 2012-09-27 23:22:53]

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

https://codereview.appspot.com/6567062/diff/2001/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The underlying arrays of <code>s</code> and a slice
argument <code>x...</code>
On 2012/09/27 18:30:54, iant wrote:
> Why not just repeat what we say for copy: "Source and destination may
overlap." 
> Expressing this in terms of the underlying arrays makes it sound like we are
> trying to say there is some case which will not work, but I think all cases
will
> work as expected.

Because there's no obvious source and destination. Changed to:

"The arguments may overlap."

(not sure that's precise enough, though).

[r, 2012-09-27 23:45:31]

https://codereview.appspot.com/6567062/diff/6001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6567062/diff/6001/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The arguments may overlap.
The range of addresses represented by the arguments may overlap.

[r, 2012-09-27 23:54:11]

Fixes issue 4142.

[gri, 2012-09-28 00:04:13]

PTAL

https://codereview.appspot.com/6567062/diff/6001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6567062/diff/6001/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The arguments may overlap.
On 2012/09/27 23:45:31, r wrote:
> The range of addresses represented by the arguments may overlap.

Done.

[r, 2012-09-28 00:08:40]

https://codereview.appspot.com/6567062/diff/9002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6567062/diff/9002/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The range of addresses represented by the arguments may
overlap.
should probably say what happens....

, in which case append behaves equivalent to using a staging buffer to copy the
source before it is overwritten.

[gri, 2012-09-28 00:19:38]

That seems to detailed; for one we don't say anything the like for copy,
just below. I was hoping that the added example would make the behavior
clear enough.
- gri


On Thu, Sep 27, 2012 at 5:08 PM, <r@golang.org> wrote:

>
>
https://codereview.appspot.**com/6567062/diff/9002/doc/go_**spec.html<https:/...
> File doc/go_spec.html (right):
>
> https://codereview.appspot.**com/6567062/diff/9002/doc/go_**
>
spec.html#newcode4918<https://codereview.appspot.com/6567062/diff/9002/doc/go_spec.html#newcode4918>
> doc/go_spec.html:4918: The range of addresses represented by the
> arguments may overlap.
> should probably say what happens....
>
> , in which case append behaves equivalent to using a staging buffer to
> copy the source before it is overwritten.
>
>
https://codereview.appspot.**com/6567062/<https://codereview.appspot.com/6567...
>

[r, 2012-09-28 00:33:41]

Perhaps, but I think it's worth saying that the source is read before
being overwritten.

-rob

[gri, 2012-09-28 20:13:34]

PTAL
- gri

https://codereview.appspot.com/6567062/diff/9002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6567062/diff/9002/doc/go_spec.html#newcode4918
doc/go_spec.html:4918: The range of addresses represented by the arguments may
overlap.
On 2012/09/28 00:08:40, r wrote:
> should probably say what happens....
> 
> , in which case append behaves equivalent to using a staging buffer to copy
the
> source before it is overwritten.

Reformulated and factored out so that the same rule applies for append and copy.

[r, 2012-09-28 21:05:02]

LGTM

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

https://codereview.appspot.com/6567062/diff/2003/doc/go_spec.html#newcode4909
doc/go_spec.html:4909: overlap in which case the functions behave equivalent to
using a staging
s/overlap/&,/

[r, 2012-09-28 21:05:42]

I admit this is a clumsy way to express this concept. If someone has a
better idea about how to phrase this, please suggest it.

-rob

[iant, 2012-09-28 21:15:34]

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

https://codereview.appspot.com/6567062/diff/2003/doc/go_spec.html#newcode4909
doc/go_spec.html:4909: overlap in which case the functions behave equivalent to
using a staging
Perhaps: The range of addresses represented by the arguments for both functions
may overlap. The input will be read before it is overwritten by the output.

[remyoudompheng, 2012-09-28 21:15:45]

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

https://codereview.appspot.com/6567062/diff/2003/doc/go_spec.html#newcode4908
doc/go_spec.html:4908: The range of addresses represented by the arguments for
both functions may
I can suggest: "When either of these functions performs a memory copy, the
destination contents after the copy will be identical to the source contents
before the copy, even if the corresponding memory locations overlap. The actual
source and destination of the memory copy are described below."

Maybe it is a bit strong because race conditions may happen too.

[r, 2012-09-28 21:21:14]

Another attempt. Same idea but avoiding the buffer concept.

The range of addresses represented by the source and destination may
overlap, in which case the resulting destination slice is the same as
would result if the same operation were applied to two equal slices
that do not overlap.

[gri, 2012-09-28 22:18:41]

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

Please take another look.

[iant, 2012-09-28 22:27:36]

LGTM

[r, 2012-09-28 22:52:32]

LGTM

https://codereview.appspot.com/6567062/diff/16001/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6567062/diff/16001/doc/go_spec.html#newcode4909
doc/go_spec.html:4909: by slice arguments overlaps.
this reads funny. maybe just s/slice/the/

[gri, 2012-09-28 22:55:42]

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

go spec: arguments for append may overlap

Fixes issue 4142.

R=rsc, r, iant, ken, remyoudompheng
CC=golang-dev
http://codereview.appspot.com/6567062

[rsc, 2012-10-01 19:09:33]

LGTM