FAQ
Reviewers: rsc, iant, r, ken2,

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

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


Description:
spec: clarify re-use of underlying arrays in slice operations

Documents the status quo; not a language change. That said,
please note the slight rewording for append, where the old
language (almost) implied that the underlying array is shared
if large enough. The slight rewording makes it clear that we
do not require that property.

Fixes issue 5818.

Please review this at https://codereview.appspot.com/14419054/

Affected files (+17, -24 lines):
    M doc/go_spec.html


Index: doc/go_spec.html
===================================================================
--- a/doc/go_spec.html
+++ b/doc/go_spec.html
@@ -1,6 +1,6 @@
   <!--{
    "Title": "The Go Programming Language Specification",
- "Subtitle": "Version of Oct 7, 2013",
+ "Subtitle": "Version of Oct 9, 2013",
    "Path": "/ref/spec"
   }-->

@@ -839,7 +839,7 @@
   <h3 id="Slice_types">Slice types</h3>

   <p>
-A slice is a descriptor for a contiguous segment of an array and
+A slice is a descriptor for a contiguous segment of an <i>underlying
array</i> and
   provides access to a numbered sequence of elements from that array.
   A slice type denotes the set of all slices of arrays of its element type.
   The value of an uninitialized slice is <code>nil</code>.
@@ -879,26 +879,18 @@
   made using the built-in function
   <a href="#Making_slices_maps_and_channels"><code>make</code></a>,
   which takes a slice type
-and parameters specifying the length and optionally the capacity:
-</p>
-
-<pre>
-make([]T, length)
+and parameters specifying the length and optionally the capacity.
+A slice created with <code>make</code> always allocates a new, hidden array
+to which the returned slice value refers. That is, executing
+</p>
+
+<pre>
   make([]T, length, capacity)
   </pre>

   <p>
-A call to <code>make</code> allocates a new, hidden array to which the
returned
-slice value refers. That is, executing
-</p>
-
-<pre>
-make([]T, length, capacity)
-</pre>
-
-<p>
-produces the same slice as allocating an array and slicing it, so these
two examples
-result in the same slice:
+produces the same slice as allocating an array and <a
href="#Slice_expressions">slicing</a>
+it, so these two examples result in the same slice:
   </p>

   <pre>
@@ -910,8 +902,8 @@
   Like arrays, slices are always one-dimensional but may be composed to
construct
   higher-dimensional objects.
   With arrays of arrays, the inner arrays are, by construction, always the
same length;
-however with slices of slices (or arrays of slices), the lengths may vary
dynamically.
-Moreover, the inner slices must be allocated individually (with
<code>make</code>).
+however with slices of slices (or arrays of slices), the inner lengths may
vary dynamically.
+Moreover, the inner slices must be initialized individually.
   </p>

   <h3 id="Struct_types">Struct types</h3>
@@ -2707,7 +2699,8 @@

   <p>
   If the sliced operand of a valid slice expression is a <code>nil</code>
slice, the result
-is a <code>nil</code> slice.
+is a <code>nil</code> slice. Otherwise, the result shares its underlying
array with the
+operand.
   </p>

   <h4>Full slice expressions</h4>
@@ -5360,10 +5353,10 @@
   </pre>

   <p>
-If the capacity of <code>s</code> is not large enough to fit the additional
+The resulting slice may refer to a different underlying array then
<code>s</code>.
+For instance, if the capacity of <code>s</code> is not large enough to fit
the additional
   values, <code>append</code> allocates a new, sufficiently large slice that
fits
-both the existing slice elements and the additional values. Thus, the
returned
-slice may refer to a different underlying array.
+both the existing slice elements and the additional values.
   </p>

   <pre>


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • R at Oct 9, 2013 at 11:00 pm
    LGTM but wait for others


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

    https://codereview.appspot.com/14419054/diff/6001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array then <code>s</code>.
    s/then/than/

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Gri at Oct 9, 2013 at 11:05 pm
    PTAL


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

    https://codereview.appspot.com/14419054/diff/6001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array then <code>s</code>.
    On 2013/10/09 23:00:26, r wrote:
    s/then/than/
    Done.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Iant at Oct 10, 2013 at 12:00 am
    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode893
    doc/go_spec.html:893: it, so these two examples result in the same
    slice:
    This sounds weird to me. They don't actually result in the *same*
    *slice*. Both operations allocate an array and then slice it and the
    result is in some sense equivalent but the slices are not the same.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Minux Ma at Oct 10, 2013 at 12:41 am
    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array than <code>s</code>.
    i don't understand why we remove the implementation restriction.
    why allocate a new backing array and copy all the data instead of reuse
    a sufficiently large original backing array?

    i think it's a Go idiom to pre-allocate a backing array for a slice, and
    then append to it in a loop.

    Removing the restriction makes this idiom very inefficient (instead of
    allocating O(N) memory, it might allocate up to O(N^2) memory, not to
    mention the excessive copying overhead incurred during the process.)

    Am I missing something?

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Gri at Oct 10, 2013 at 12:54 am
    PTAL


    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode893
    doc/go_spec.html:893: it, so these two examples result in the same
    slice:
    On 2013/10/10 00:00:43, iant wrote:
    This sounds weird to me. They don't actually result in the *same* *slice*.
    Both operations allocate an array and then slice it and the result is in some
    sense equivalent but the slices are not the same.
    This was old language, existed before. I had noticed it and planned to
    change it, but then forgot.

    Fixed.

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array than <code>s</code>.
    On 2013/10/10 00:41:30, minux wrote:
    i don't understand why we remove the implementation restriction.
    why allocate a new backing array and copy all the data instead of reuse
    a sufficiently large original backing array?
    i think it's a Go idiom to pre-allocate a backing array for a slice, and
    then append to it in a loop.
    Removing the restriction makes this idiom very inefficient (instead of
    allocating O(N) memory, it might allocate up to O(N^2) memory, not to
    mention the excessive copying overhead incurred during the process.)
    Am I missing something?
    This is _not_ removing an implementation restriction (nowhere does it
    say "implementation restriction"...).

    Also, we have never required this. The resulting slice _may_ refer to a
    different underlying array, but doesn't have to - the word "may" is
    crucial here.

    I don't think we want to _require_ that the underlying array be re-used.
    Code that explicitly relies on this feature must know that the slice is
    large enough before calling append (otherwise the code couldn't rely on
    it). Thus, that code can always be written w/o the use of append
    (explicit indexing into existing slice or use of copy).

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Iant at Oct 10, 2013 at 12:58 am

    On 2013/10/10 00:41:30, minux wrote:
    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying
    array than <code>s</code>.
    i don't understand why we remove the implementation restriction.
    why allocate a new backing array and copy all the data instead of reuse
    a sufficiently large original backing array?
    i think it's a Go idiom to pre-allocate a backing array for a slice, and
    then append to it in a loop.
    Removing the restriction makes this idiom very inefficient (instead of
    allocating O(N) memory, it might allocate up to O(N^2) memory, not to
    mention the excessive copying overhead incurred during the process.)
    Am I missing something?
    I don't think we are actually changing anything in the spec. If we are
    changing something, then what we are changing is a requirement that the
    implementation always reuse the existing backing array if there is
    space. Of course in most cases the implementation will reuse the
    existing array, and of course it will always be the case that appending
    in a loop will be efficient. But for example if you write
          a := make([]byte, 0, 1)
          a = append(a, 'b')
    then I think it is OK if the implementation guesses that more append
    calls are coming and preemptively creates a new, larger, backing array.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Minux at Oct 10, 2013 at 1:13 am

    On Wed, Oct 9, 2013 at 8:58 PM, wrote:

    I don't think we are actually changing anything in the spec. If we are
    changing something, then what we are changing is a requirement that the
    implementation always reuse the existing backing array if there is
    space. Of course in most cases the implementation will reuse the
    existing array, and of course it will always be the case that appending
    in a loop will be efficient. But for example if you write
    a := make([]byte, 0, 1)
    a = append(a, 'b')
    then I think it is OK if the implementation guesses that more append
    calls are coming and preemptively creates a new, larger, backing array.
    i expect this to be a surprise to the programmer, i've allocated enough
    space, why allocate more?

    if there are further appends to a, then this optimization of course is
    plausible, but if written as is, i'd rather make a use the available space.

    could we do some experiments before this change?
    let's modify the runtime to do optimization like this, and see if there are
    real world breakages?

    to me, this change does remove one (however implicit) guarantee, and
    thus goes against the Go 1 stability guarantee.

    just my 2 cents.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Iant at Oct 10, 2013 at 12:59 am
    LGTM

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Minux Ma at Oct 10, 2013 at 1:04 am
    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array than <code>s</code>.
    On 2013/10/10 00:54:10, gri wrote:
    I don't think we want to _require_ that the underlying array be
    re-used. Code
    that explicitly relies on this feature must know that the slice is
    large enough
    before calling append (otherwise the code couldn't rely on it). Thus, that code
    can always be written w/o the use of append (explicit indexing into existing
    slice or use of copy).
    yes, the code that relies on append reusing the array could be
    rewritten.
    but my point is the loop append idiom is sufficient widespread, we
    probably
    shouldn't loose the requirement here.

    also, consider what if you don't know exactly the amount of storage to
    be used,
    now we can determine a typical value, say, N, and preallocate a slice
    with cap
    N, and then do a loop append.

    if we change to don't rely on this restriction, the loop will be more
    complicated, as it has to keep track of cap(s) and len(s), and use
    simple
    indexing assignment or append depends on the order of len(s) and cap(s).

    Is there any optimization opportunity that will be missed when we
    enforce the
    array reuse implementation restriction?

    PS: ok, i chose the wrong word. it's not implementation restriction. but
    the
    original wording does imply to reuse the storage if it's enough. so this
    change
    actually removes that implication.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Gri at Oct 10, 2013 at 1:14 am
    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/12001/doc/go_spec.html#newcode5356
    doc/go_spec.html:5356: The resulting slice may refer to a different
    underlying array than <code>s</code>.
    On 2013/10/10 01:04:21, minux wrote:
    On 2013/10/10 00:54:10, gri wrote:
    I don't think we want to _require_ that the underlying array be
    re-used. Code
    that explicitly relies on this feature must know that the slice is
    large
    enough
    before calling append (otherwise the code couldn't rely on it).
    Thus, that
    code
    can always be written w/o the use of append (explicit indexing into
    existing
    slice or use of copy).
    yes, the code that relies on append reusing the array could be
    rewritten.
    but my point is the loop append idiom is sufficient widespread, we probably
    shouldn't loose the requirement here.
    also, consider what if you don't know exactly the amount of storage to be used,
    now we can determine a typical value, say, N, and preallocate a slice with cap
    N, and then do a loop append.
    if we change to don't rely on this restriction, the loop will be more
    complicated, as it has to keep track of cap(s) and len(s), and use simple
    indexing assignment or append depends on the order of len(s) and cap(s).
    Is there any optimization opportunity that will be missed when we
    enforce the
    array reuse implementation restriction?
    PS: ok, i chose the wrong word. it's not implementation restriction. but the
    original wording does imply to reuse the storage if it's enough. so
    this change
    actually removes that implication.
    Sorry, I didn't mean to be facetious.

    I still maintain that we do not want to _require_ this. Of course, a
    sensible implementation will use the underlying array. Note that the
    previous wording didn't say re-use is required. Is was unclear.

    The new wording does not require it either, but it clear about it.

    Making it a requirement would be a change to the spec. That's ok, but
    that's not the intent of this CL.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Minux at Oct 10, 2013 at 1:21 am

    On Wed, Oct 9, 2013 at 9:14 PM, wrote:

    sensible implementation will use the underlying array. Note that the
    previous wording didn't say re-use is required. Is was unclear.

    The new wording does not require it either, but it clear about it.
    Making it a requirement would be a change to the spec. That's ok, but
    that's not the intent of this CL.
    ok, i see. making the spec more clear is the way to go. I just pose the
    question
    as clearly i read something unintended from the spec, and i wonder if others
    are also get the same.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Dan Kortschak at Oct 10, 2013 at 1:46 am
    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html#newcode5357
    doc/go_spec.html:5357: For instance, if the capacity of <code>s</code>
    is not large enough to fit the additional
    I don't think this yet addresses the concern raised in Issue 5180. It
    would if s/For instance, if/For instance, iff/ though. I know that you
    have addresses this in that issue, but like minux, I think the idiom
    depending on this behaviour is very widely used and probably expected by
    most gophers.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Gri at Oct 10, 2013 at 5:35 am
    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html#newcode5357
    doc/go_spec.html:5357: For instance, if the capacity of <code>s</code>
    is not large enough to fit the additional
    On 2013/10/10 01:45:59, kortschak wrote:
    I don't think this yet addresses the concern raised in Issue 5180. It would if
    s/For instance, if/For instance, iff/ though. I know that you have addresses
    this in that issue, but like minux, I think the idiom depending on this
    behaviour is very widely used and probably expected by most gophers.
    The spec should not prescribe a specific implementation if not
    absolutely necessary, hence the _may_. Of course any sensible
    implementation _will_ (at the moment) reuse the underlying array if
    there's enough capacity since that is (usually) the most efficient
    approach. Furthermore, it's a very straight-forward approach, which is
    another reason why there's no need to prescribe it (it's a different
    story if it were difficult to achieve with so much code that relies on
    it for efficiency - we don't have that case).

    It's important that it's not prescribed, though. Here's a (admittedly
    somewhat contrived, but illustrative) example why: A future smarter
    compiler might optimize the statement

    copy(dst, append(src, x))

    such that the generated code directly copies the result of append(src,
    x) into dst, _without_ storing that intermediate result in the
    underlying array of src, even if there would be enough space for it. If
    we require append to _always_ reuse the underlying array independent of
    context, we give up this optimization (and others that we might not be
    thinking about now). We don't want to restrict implementations this way.

    Furthermore, code that _does_ rely on the result of append reusing the
    underlying array if there's enough space should be explicit about it by
    not using append in these cases. That code _must_ know that space is
    available and so _can_ use easy alternatives to append (say, copy).

    In short, requiring re-use when possible is a mistake. Any sensible
    implementation should be free to choose the best possible approach.

    https://codereview.appspot.com/14419054/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Roger peppe at Oct 10, 2013 at 10:04 am
    I appreciate the arguments for keeping this undefined, but I
    couldn't guarantee that I haven't written some code along
    these kinds of lines in the past:

    For example, I might easily write this, and I doubt
    I'd have called it out in a code review:

    // fill populates the given slice with values from the given map. The
    // slice and the map are of the same length.
    func fill(xs []int, m map[string]int) {
         xs := xs[:0]
         for _, x := range m {
             xs = append(xs, x)
         }
    }

    rather than this:

    func fill(xs []int, m map[string]int) {
         i := 0
         for _, x := range m {
             xs[i] = x
             i++
         }
    }

    If a compiler *were* to implement different behaviour in
    this respect, I suspect it could lead to some very subtle
    breakages. I'd prefer to have it defined, even if it
    leads to some potential inefficiencies in the future.

    The copy(dst, append(src, x)) optimisation will still be
    possible in some circumstances, I think.
    On 10 October 2013 06:35, wrote:

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html#newcode5357
    doc/go_spec.html:5357: For instance, if the capacity of <code>s</code>
    is not large enough to fit the additional
    On 2013/10/10 01:45:59, kortschak wrote:

    I don't think this yet addresses the concern raised in Issue 5180. It would if
    s/For instance, if/For instance, iff/ though. I know that you have addresses
    this in that issue, but like minux, I think the idiom depending on this
    behaviour is very widely used and probably expected by most gophers.

    The spec should not prescribe a specific implementation if not
    absolutely necessary, hence the _may_. Of course any sensible
    implementation _will_ (at the moment) reuse the underlying array if
    there's enough capacity since that is (usually) the most efficient
    approach. Furthermore, it's a very straight-forward approach, which is
    another reason why there's no need to prescribe it (it's a different
    story if it were difficult to achieve with so much code that relies on
    it for efficiency - we don't have that case).

    It's important that it's not prescribed, though. Here's a (admittedly
    somewhat contrived, but illustrative) example why: A future smarter
    compiler might optimize the statement

    copy(dst, append(src, x))

    such that the generated code directly copies the result of append(src,
    x) into dst, _without_ storing that intermediate result in the
    underlying array of src, even if there would be enough space for it. If
    we require append to _always_ reuse the underlying array independent of
    context, we give up this optimization (and others that we might not be
    thinking about now). We don't want to restrict implementations this way.

    Furthermore, code that _does_ rely on the result of append reusing the
    underlying array if there's enough space should be explicit about it by
    not using append in these cases. That code _must_ know that space is
    available and so _can_ use easy alternatives to append (say, copy).

    In short, requiring re-use when possible is a mistake. Any sensible
    implementation should be free to choose the best possible approach.


    https://codereview.appspot.com/14419054/

    --

    ---You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Robert Griesemer at Oct 10, 2013 at 4:25 pm
    I'd argue that the fill function should return the filled slice. The
    comment does say that the slice and map are of the same length (capacity
    rather, for the slice), but there's no check in that code that that's true.
    So this may have silently failed if there's a bug elsewhere.

    Once you add the respective checks to the code, it becomes easier to not
    rely on it.

    The purpose of this CL is make it clearer that something that happens to be
    true for some implementations shouldn't be relied on in the first place. I
    think once that's become part of the mindset when operating with append,
    code like this is more likely to raise eyebrows.

    - gri

    On Thu, Oct 10, 2013 at 3:04 AM, roger peppe wrote:

    I appreciate the arguments for keeping this undefined, but I
    couldn't guarantee that I haven't written some code along
    these kinds of lines in the past:

    For example, I might easily write this, and I doubt
    I'd have called it out in a code review:

    // fill populates the given slice with values from the given map. The
    // slice and the map are of the same length.
    func fill(xs []int, m map[string]int) {
    xs := xs[:0]
    for _, x := range m {
    xs = append(xs, x)
    }
    }

    rather than this:

    func fill(xs []int, m map[string]int) {
    i := 0
    for _, x := range m {
    xs[i] = x
    i++
    }
    }

    If a compiler *were* to implement different behaviour in
    this respect, I suspect it could lead to some very subtle
    breakages. I'd prefer to have it defined, even if it
    leads to some potential inefficiencies in the future.

    The copy(dst, append(src, x)) optimisation will still be
    possible in some circumstances, I think.
    On 10 October 2013 06:35, wrote:

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html
    File doc/go_spec.html (right):

    https://codereview.appspot.com/14419054/diff/13001/doc/go_spec.html#newcode5357
    doc/go_spec.html:5357: For instance, if the capacity of <code>s</code>
    is not large enough to fit the additional
    On 2013/10/10 01:45:59, kortschak wrote:

    I don't think this yet addresses the concern raised in Issue 5180. It would if
    s/For instance, if/For instance, iff/ though. I know that you have addresses
    this in that issue, but like minux, I think the idiom depending on this
    behaviour is very widely used and probably expected by most gophers.

    The spec should not prescribe a specific implementation if not
    absolutely necessary, hence the _may_. Of course any sensible
    implementation _will_ (at the moment) reuse the underlying array if
    there's enough capacity since that is (usually) the most efficient
    approach. Furthermore, it's a very straight-forward approach, which is
    another reason why there's no need to prescribe it (it's a different
    story if it were difficult to achieve with so much code that relies on
    it for efficiency - we don't have that case).

    It's important that it's not prescribed, though. Here's a (admittedly
    somewhat contrived, but illustrative) example why: A future smarter
    compiler might optimize the statement

    copy(dst, append(src, x))

    such that the generated code directly copies the result of append(src,
    x) into dst, _without_ storing that intermediate result in the
    underlying array of src, even if there would be enough space for it. If
    we require append to _always_ reuse the underlying array independent of
    context, we give up this optimization (and others that we might not be
    thinking about now). We don't want to restrict implementations this way.

    Furthermore, code that _does_ rely on the result of append reusing the
    underlying array if there's enough space should be explicit about it by
    not using append in these cases. That code _must_ know that space is
    available and so _can_ use easy alternatives to append (say, copy).

    In short, requiring re-use when possible is a mistake. Any sensible
    implementation should be free to choose the best possible approach.


    https://codereview.appspot.com/14419054/

    --

    ---You received this message because you are subscribed to the Google Groups
    "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an
    email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Roger peppe at Oct 10, 2013 at 6:42 pm

    On 10 October 2013 17:25, Robert Griesemer wrote:
    I'd argue that the fill function should return the filled slice. The comment
    does say that the slice and map are of the same length (capacity rather, for
    the slice), but there's no check in that code that that's true. So this may
    have silently failed if there's a bug elsewhere.
    I agree that the fill function should return the filled slice (and
    I would almost certainly write it that way).

    But I can't guarantee that something like this code doesn't exist
    in the wild, so I'm worried that changing something this fundamental
    will probably break existing code in subtle and hard to diagnose
    ways, something that would be hard to countenance, given the
    usual Go compatibility guarantees.
    The purpose of this CL is make it clearer that something that happens to be
    true for some implementations shouldn't be relied on in the first place. I
    think once that's become part of the mindset when operating with append,
    code like this is more likely to raise eyebrows.
    Isn't it a bit late for that?

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Russ Cox at Oct 10, 2013 at 6:50 pm
    FWIW I agree with Minux and Roger: we should define (if we haven't already)
    that append only reallocates when there was not enough capacity in the
    original slice.

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • peterGo at Oct 10, 2013 at 7:16 pm
    As a practical matter, it's important for Go performance to minimize
    allocations. For slices, it often helps to make reasonable capacity
    estimates. For example,

    package main

    import "fmt"

    func main() {
         estimate, actual := 50, 42
         a := make([]int, 0, estimate)
         for i := 0; i < actual; i++ {
             a = append(a, 42)
         }
         fmt.Println(len(a), cap(a))
    }

    Output:

    42 50

    The append function should only reallocate the underlying array if the
    capacity is insufficient.

    Peter
    On Wednesday, October 9, 2013 6:54:40 PM UTC-4, gri wrote:

    Reviewers: rsc, iant, r, ken2,

    Message:
    Hello r...@golang.org <javascript:>, ia...@golang.org <javascript:>,
    r...@golang.org <javascript:>, k...@golang.org <javascript:> (cc:
    golan...@googlegroups.com <javascript:>),

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


    Description:
    spec: clarify re-use of underlying arrays in slice operations

    Documents the status quo; not a language change. That said,
    please note the slight rewording for append, where the old
    language (almost) implied that the underlying array is shared
    if large enough. The slight rewording makes it clear that we
    do not require that property.

    Fixes issue 5818.

    Please review this at https://codereview.appspot.com/14419054/

    Affected files (+17, -24 lines):
    M doc/go_spec.html


    Index: doc/go_spec.html
    ===================================================================
    --- a/doc/go_spec.html
    +++ b/doc/go_spec.html
    @@ -1,6 +1,6 @@
    <!--{
    "Title": "The Go Programming Language Specification",
    - "Subtitle": "Version of Oct 7, 2013",
    + "Subtitle": "Version of Oct 9, 2013",
    "Path": "/ref/spec"
    }-->

    @@ -839,7 +839,7 @@
    <h3 id="Slice_types">Slice types</h3>

    <p>
    -A slice is a descriptor for a contiguous segment of an array and
    +A slice is a descriptor for a contiguous segment of an <i>underlying
    array</i> and
    provides access to a numbered sequence of elements from that array.
    A slice type denotes the set of all slices of arrays of its element
    type.
    The value of an uninitialized slice is <code>nil</code>.
    @@ -879,26 +879,18 @@
    made using the built-in function
    <a href="#Making_slices_maps_and_channels"><code>make</code></a>,
    which takes a slice type
    -and parameters specifying the length and optionally the capacity:
    -</p>
    -
    -<pre>
    -make([]T, length)
    +and parameters specifying the length and optionally the capacity.
    +A slice created with <code>make</code> always allocates a new, hidden
    array
    +to which the returned slice value refers. That is, executing
    +</p>
    +
    +<pre>
    make([]T, length, capacity)
    </pre>

    <p>
    -A call to <code>make</code> allocates a new, hidden array to which the
    returned
    -slice value refers. That is, executing
    -</p>
    -
    -<pre>
    -make([]T, length, capacity)
    -</pre>
    -
    -<p>
    -produces the same slice as allocating an array and slicing it, so these
    two examples
    -result in the same slice:
    +produces the same slice as allocating an array and <a
    href="#Slice_expressions">slicing</a>
    +it, so these two examples result in the same slice:
    </p>

    <pre>
    @@ -910,8 +902,8 @@
    Like arrays, slices are always one-dimensional but may be composed to
    construct
    higher-dimensional objects.
    With arrays of arrays, the inner arrays are, by construction, always the

    same length;
    -however with slices of slices (or arrays of slices), the lengths may vary

    dynamically.
    -Moreover, the inner slices must be allocated individually (with
    <code>make</code>).
    +however with slices of slices (or arrays of slices), the inner lengths
    may
    vary dynamically.
    +Moreover, the inner slices must be initialized individually.
    </p>

    <h3 id="Struct_types">Struct types</h3>
    @@ -2707,7 +2699,8 @@

    <p>
    If the sliced operand of a valid slice expression is a <code>nil</code>

    slice, the result
    -is a <code>nil</code> slice.
    +is a <code>nil</code> slice. Otherwise, the result shares its underlying

    array with the
    +operand.
    </p>

    <h4>Full slice expressions</h4>
    @@ -5360,10 +5353,10 @@
    </pre>

    <p>
    -If the capacity of <code>s</code> is not large enough to fit the
    additional
    +The resulting slice may refer to a different underlying array then
    <code>s</code>.
    +For instance, if the capacity of <code>s</code> is not large enough to
    fit
    the additional
    values, <code>append</code> allocates a new, sufficiently large slice
    that
    fits
    -both the existing slice elements and the additional values. Thus, the
    returned
    -slice may refer to a different underlying array.
    +both the existing slice elements and the additional values.
    </p>

    <pre>

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 9, '13 at 10:54p
activeOct 10, '13 at 7:16p
posts19
users8
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase