code review 2627043: go spec: append built-in

go spec: append built-in

[gri, 2010-10-22 21:16:07]

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

I'd like you to review this change.

[r, 2010-10-22 22:07:17]

http://codereview.appspot.com/2627043/diff/1/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2627043/diff/1/doc/go_spec.html#newcode4540
doc/go_spec.html:4540: value must be of the form <code>v...</code> and the type
of <code>v</code> must
i find this very hard to follow.

how about just defining the signature of append and then showing how it's used?

[gri, 2010-10-22 22:33:07]

PTAL

Slightly reformulated. One could just let the append() signature speak for
itself, but we don't do that for copy() either. An alternative would be to
simply provide the corresponding Go code that implements (a version of)
append. Same for copy(). Perhaps this should be in an appendix.

- gri

On Fri, Oct 22, 2010 at 3:07 PM, <r@golang.org> wrote:

>
> http://codereview.appspot.com/2627043/diff/1/doc/go_spec.html
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/2627043/diff/1/doc/go_spec.html#newcode4540
> doc/go_spec.html:4540: value must be of the form <code>v...</code> and
>
> the type of <code>v</code> must
> i find this very hard to follow.
>
> how about just defining the signature of append and then showing how
> it's used?
>
>
> http://codereview.appspot.com/2627043/
>

[rsc, 2010-10-22 23:01:25]

there is no straightforward Go implementation
of copy, because copy handles overlap.
(there's a non-straightforward one that uses
cap tricks, but you don't want to explain that
in the spec.)

[gri, 2010-10-22 23:10:46]

PTAL

On Fri, Oct 22, 2010 at 4:01 PM, Russ Cox <rsc@golang.org> wrote:

> there is no straightforward Go implementation
> of copy, because copy handles overlap.
> (there's a non-straightforward one that uses
> cap tricks, but you don't want to explain that
> in the spec.)
>

[r, 2010-10-22 23:37:06]

LGTM

[rsc1, 2010-10-22 23:37:36]

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4541
doc/go_spec.html:4541: the type of <code>v</code> must be identical to
<code>[]T</code>.
<code>v</code> must have type <code>[]T</code>.

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4551
doc/go_spec.html:4551: both the existing slice elements and the additional
values. Thus, the resulting
s/resulting/returned/

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4552
doc/go_spec.html:4552: slice may have a different underlying array.
s/have/refer to/

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4560
doc/go_spec.html:4560: s4 := append(nil, s1...)   // make a copy of a slice     
s4 == []int{0, 0, 2}
Is this legal?  The typechecking discussion makes the
rules depend on the type of s, but s has no type here.

Is there some word we can use other than copy, just to
avoid confusion with the next paragraph?  
Maybe "duplicate a slice"?

http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4570
doc/go_spec.html:4570: <code>len(src)</code> and <code>len(dst)</code>.
While we're here, can we add (language change):

As a special case, copy accepts a destination argument assignable to type
<code>[]byte</code> with a source argument of type <code>string</code>.  This
form copies
the bytes from the string into the byte slice.

[gri, 2010-10-23 05:01:12]

PTAL.

On Fri, Oct 22, 2010 at 4:37 PM, <rsc@google.com> wrote:

>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html
>
> File doc/go_spec.html (right):
>
>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4541
> doc/go_spec.html:4541: the type of <code>v</code> must be identical to
> <code>[]T</code>.
> <code>v</code> must have type <code>[]T</code>.

This sentence didn't exist anymore in the version that Rob LGTM'ed.


>
>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4551
> doc/go_spec.html:4551: both the existing slice elements and the
>
> additional values. Thus, the resulting
> s/resulting/returned/
>
done

>
>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4552
> doc/go_spec.html:4552: slice may have a different underlying array.
> s/have/refer to/
>
done

>
>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4560
> doc/go_spec.html:4560: s4 := append(nil, s1...)   // make a copy of a
>
> slice      s4 == []int{0, 0, 2}
> Is this legal?  The typechecking discussion makes the
> rules depend on the type of s, but s has no type here.
>
Good point - of course it works in my prototype, but it assumes that type T
== int.
I removed the example. It's always possible to get the same effect via
append([]T{}, s...); i.e. by appending a slice to an empty slice.

>
> Is there some word we can use other than copy, just to
> avoid confusion with the next paragraph?
> Maybe "duplicate a slice"?
>
n/a anymore

>
>
> http://codereview.appspot.com/2627043/diff/5001/doc/go_spec.html#newcode4570
> doc/go_spec.html:4570: <code>len(src)</code> and <code>len(dst)</code>.
> While we're here, can we add (language change):
>
Fine, but let's do this is a separate CL to keep the language changes
apart.

>
> As a special case, copy accepts a destination argument assignable to
> type <code>[]byte</code> with a source argument of type
> <code>string</code>.  This form copies
> the bytes from the string into the byte slice.
>
>
> http://codereview.appspot.com/2627043/
>

[rsc1, 2010-10-25 23:39:53]

http://codereview.appspot.com/2627043/diff/21001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/2627043/diff/21001/doc/go_spec.html#newcode4558
doc/go_spec.html:4558: s4 := append(nil, s1...)   // make a copy of a slice     
s4 == []int{0, 0, 2}
your mail said you removed this example

[gri, 2010-10-25 23:46:33]

oops - version skew (changed at home but then overrode again at work) -
PTAL.

http://codereview.appspot.com/2627043/

On Mon, Oct 25, 2010 at 4:39 PM, <rsc@google.com> wrote:

>
> http://codereview.appspot.com/2627043/diff/21001/doc/go_spec.html
>
> File doc/go_spec.html (right):
>
>
> http://codereview.appspot.com/2627043/diff/21001/doc/go_spec.html#newcode4558
> doc/go_spec.html:4558: s4 := append(nil, s1...)   // make a copy of a
>
> slice      s4 == []int{0, 0, 2}
> your mail said you removed this example
>
>
> http://codereview.appspot.com/2627043/
>

[rsc1, 2010-10-25 23:49:38]

LGTM

[gri, 2010-10-25 23:50:37]

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

go spec: append built-in

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