Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: spec: accept slices and single elements in one append? #4096

Closed
rsc opened this issue Sep 18, 2012 · 20 comments
Closed

proposal: spec: accept slices and single elements in one append? #4096

rsc opened this issue Sep 18, 2012 · 20 comments
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Sep 18, 2012

Ken points out that using append is harder than it might be.
Right now if you have var xs []string and want to append a, b, string, cs []string, and
d string, you have to do

xs = append(xs, a, b)
xs = append(xs, cs...)
xs = append(xs, d)

or

xs = append(append(append(xs, a, b), cs...), d)

The suggestion is to accept

xs = append(xs, a, b, c, d)

There is an ambiguity for slices of interface{}. If you have

var xs []interface{}
var ys []interface{}

xs = append(xs, ys)

is ambiguous: does it append a single interface{}(ys) or does it append each of the
elements in ys?
We would have to give this the same meaning it has today, meaning the former.

Thus the rules for append to a []T would be that it takes one or more elements z and for
each:
1. If z can be assigned to type T and is not 'z...', it is added as a single element.
2. Else if z is a slice of T, its elements are appended.
3. Else the compiler complains.

We would need to keep
xs = append(xs, ys...)
as a special form for resolving the ambiguity in the other direction when T =
interface{}, and for backwards compatibility.
@speter
Copy link

speter commented Sep 18, 2012

Comment 1:

Do I interpret correctly that under the new syntax these would be equivalent?
xs = append(xs, a, b, cs..., d)
xs = append(append(append(xs, a, b), cs...), d)
And what would the following do?
s := []int{0, 1, 2, 3, 4}
s = append(s[0:2], s[1:3], s[2:4], s[3:5])
If it works the same way as above, it should be equivalent to:
s = append(append(append(s[0:2], s[1:3]...), s[2:4]...), s[3:5]...)
Which yields... well it should be obvious.
To be honest I'm not sure if this will bring more simplicity or more confusion...
(Though it will definitely bring more SliceTricks.)

@rsc
Copy link
Contributor Author

rsc commented Sep 18, 2012

Comment 2:

This probably isn't the right place for a discussion. I just wanted to
record the idea.

@rsc
Copy link
Contributor Author

rsc commented Dec 10, 2012

Comment 3:

Labels changed: added size-xl.

@robpike
Copy link
Contributor

robpike commented Mar 7, 2013

Comment 4:

Labels changed: removed go1.1maybe.

@rsc
Copy link
Contributor Author

rsc commented Nov 27, 2013

Comment 5:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor Author

rsc commented Dec 4, 2013

Comment 7:

Labels changed: added repo-main.

@griesemer
Copy link
Contributor

Comment 8:

Labels changed: added languagechange.

@rsc rsc added accepted LanguageChange Suggested changes to the Go language labels Sep 10, 2014
@rsc rsc self-assigned this Sep 10, 2014
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title spec: accept slices and single elements in one append? proposal: spec: accept slices and single elements in one append? Jun 20, 2017
@rsc rsc added the v2 An incompatible library change label Jun 20, 2017
@ianlancetaylor ianlancetaylor added Proposal NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Nov 29, 2017
@ugorji
Copy link
Contributor

ugorji commented Dec 11, 2017

Maybe a simplification of the model could be:

a=append(a, b, c..., d, e, f...) means append b, all elements of c, d, e, and all elements of f

@ianlancetaylor
Copy link
Contributor

This is a special case of #18605.

@griesemer
Copy link
Contributor

With the upcoming generics proposal, it's almost possible to write append without relying on built-in "magic" (almost because we still have special handling for strings).

I think we should not introduce more special cases, but instead remove them. It's undoubtedly useful to have the proposed more general append functionality, but the need doesn't arise all the often. I'm not convinced that the benefits outweighs the costs.

@josharian
Copy link
Contributor

@griesemer another “almost” is that the runtime picks a new slice capacity that is optimal for the memory allocator. (See somewhat related discussion in #24204 and its progenitor #24163.)

@dotaheor
Copy link

dotaheor commented Feb 6, 2019

A similar problem: #29186

@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
@gopherbot gopherbot added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2019
@ianlancetaylor
Copy link
Contributor

As @griesemer notes above, this can be written using generics, although not necessarily in a fully type-safe way. With the current design draft it would look something like this:

func Append(type Elem)(s []Elem, a ...interface{}) []Elem {
    for _, v := range a {
        switch v := v.(type) {
        case Elem:
            s = append(s, v)
        case []Elem:
            s = append(s, v...)
        default:
            panic("invalid type")
        }
    }
    return s
}

Therefore, we suggest that this not necessary to add to the language itself. Therefore, this is a likely decline. Leaving open for four weeks for final comments.

@ianlancetaylor ianlancetaylor added Proposal-FinalCommentPeriod and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Mar 17, 2020
@ianlancetaylor ianlancetaylor modified the milestones: Unplanned, Proposal Mar 17, 2020
@ianlancetaylor
Copy link
Contributor

No further comments.

@golang golang locked and limited conversation to collaborators Apr 14, 2021
@rsc rsc removed their assignment Jun 22, 2022
@aclements
Copy link
Member

Now that we have significantly more experience with generics, I think we should reconsider this proposal (in the form given in @ugorji's comment). We now have a better handle on the problems that can be solved by generics, but shouldn't necessarily.

@ianlancetaylor pointed out that this is a special case of #18605. That's true in some sense, but allowing more flexible ... arguments to functions raises subtle questions about slice aliasing and efficiency. Doing this just for append does not have these issues.

The exact generic Append function that @ianlancetaylor gave in this comment has come back as a new proposal, so I think it makes sense to consider these two proposals together, as two different ways to solve the same problem.

@randall77
Copy link
Contributor

randall77 commented May 17, 2023

There is a tricky case when we allow appending slices which alias the slice being appended to. See #60138 for gory details about this in the context of slices.{Insert,Replace}.

s = append(s, a, b...)

What happens if b points to the same backing store as s? That is,

s := []int{1,2,3,4}
s = append(s[:2], 7, s[2:]...)

Do we want the result to be 1,2,7,3,4? If so, then we can't just append 7 first, as it overwrites the 3 that we're going to append later.

Maybe we're happy with a result of 1,2,7,7,4, as that's what you would get with append(append(s[:2], 7), s[2:])). But we would need to make this clear in the spec.

@ianlancetaylor
Copy link
Contributor

This proposal is a language change, which has a higher bar than a library change. This change introduces a new syntax that is specific to append: using ... with an argument that is not the last argument. Is this change worth the additional language complexity?

The exact generic Append function that @ianlancetaylor gave in this comment has come back as a new proposal, so I think it makes sense to consider these two proposals together, as two different ways to solve the same problem.

The new proposal is different, though. It just concatenates slices. The idea here is to concatenate either elements or slices.

@aclements
Copy link
Member

There is a tricky case when we allow appending slices which alias the slice being appended to.

My sense is that we should make append do the least surprising thing, which would be as if it copied out the contents of each argument before modifying the result slice, akin to x, y = y, x.

In general, while it's theoretically always possible to do this with only enough scratch space for one element, I think such a schedule can get very complicated (e.g., s = append(s[:0], s[n:], s[:n])). Given that I would expect this type of aliasing to be extremely rare, one option would be to simply always allocate a new slice if there's complicated aliasing (in many cases we can statically avoid this). We could copy the results back, or just return the new allocation.

@aclements
Copy link
Member

aclements commented May 24, 2023

The new proposal is different, though. It just concatenates slices. The idea here is to concatenate either elements or slices.

That's true. Nevertheless, this proposal and #56353 have significant overlap, so I still think we should consider them together. While this proposal would clearly be a language change, doing this in append seems far more ergonomic because it already has a lot of the functionality you want, and can support mixing slices and individual elements. Introducing this as a library function would mean there are multiple ways to append slices, with different trade-offs. I think the one functionality of slices.Concat over a more flexible append is that it could join all of the slices in a slice of slices.

Today, people usually write this as several nested appends, which hurts readability. A quick internal code search shows many examples of nested appends that could be written far more clearly as a single append with a mix of slices and elements.

@ianlancetaylor pointed out that this is a special case of #18605.

To add to this, a generalized append would get most of the benefit of flexible ... for variadic functions, without raising the complicated questions about aliasing #18605 has, especially if we also applied type unification to appends arguments so the first argument could simply be nil. This would let you write something calls like exec.Command("ls", append(nil, "-l", "-a", paths...)...). Compared to #18605, this requires an extra append(nil, and )..., but those also clearly signal the allocation of a new slice.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge LanguageChange Suggested changes to the Go language Proposal Proposal-FinalCommentPeriod v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests