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

x/exp/slices: add Replace #55358

Closed
DeedleFake opened this issue Sep 23, 2022 · 14 comments
Closed

x/exp/slices: add Replace #55358

DeedleFake opened this issue Sep 23, 2022 · 14 comments

Comments

@DeedleFake
Copy link

DeedleFake commented Sep 23, 2022

I have on multiple occasions had a need to replace elements of a slice. This is easily accomplished by calling slices.Delete() followed by slices.Insert(), but this results in more copying than is necessary. Well, unless the compiler's capable of eliminating the extra work involved, but I have no idea if it can or not.

Regardless, my proposal's simple: Add the following function to the slices package:

func Replace[S ~[]E, E any](s S, i, j int, v ...E) S

This would function exactly like

s = slices.Delete(s, i, j)
s = slices.Inset(s, i, v...)

except that it would be a bit more efficient.

@gopherbot gopherbot added this to the Proposal milestone Sep 23, 2022
@go101
Copy link

go101 commented Sep 23, 2022

I often want a combine(s ...[]T) function, which is helpful for this too:

combine(s[:i], v, s[j:])

@rsc
Copy link
Contributor

rsc commented Sep 28, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@carlmjohnson
Copy link
Contributor

carlmjohnson commented Sep 29, 2022

@go101 I suggested slices.Concat during some phase of this process. :-) slices.Concat is still my preferred spelling for this. func Concat[S ~[]E, E any](dst S, slices ...S) S s = slices.Concat(s[:0], s[:i], v, s[j:])

@carlmjohnson
Copy link
Contributor

carlmjohnson commented Sep 29, 2022

Concat would also be able to work in place or allocating a new slice.

@rsc
Copy link
Contributor

rsc commented Oct 5, 2022

Given that we have Delete and Insert, Replace seems like it belongs.

@rsc rsc changed the title proposal: x/exp/slices: add Replace() proposal: x/exp/slices: add Replace Oct 12, 2022
@rsc
Copy link
Contributor

rsc commented Oct 12, 2022

It sounds like people are in favor of:

func Replace[S ~[]E, E any](s S, i, j int, v ...E) S

@rsc
Copy link
Contributor

rsc commented Oct 12, 2022

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@go101
Copy link

go101 commented Oct 13, 2022

It looks Concat gets more votes.

@carlmjohnson
Copy link
Contributor

carlmjohnson commented Oct 13, 2022

Should I open a separate issue for Concat?

@rsc
Copy link
Contributor

rsc commented Oct 20, 2022

This issue is about Replace. Feel free to open a separate issue about Concat.

Votes alone are not really the metric either - Replace fits next to the existing Insert and Delete.

@rsc
Copy link
Contributor

rsc commented Oct 20, 2022

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: x/exp/slices: add Replace x/exp/slices: add Replace Oct 20, 2022
@rsc rsc modified the milestones: Proposal, Backlog Oct 20, 2022
@gopherbot
Copy link

gopherbot commented Oct 21, 2022

Change https://go.dev/cl/444682 mentions this issue: slices: add Replace

@DeedleFake
Copy link
Author

DeedleFake commented Oct 21, 2022

At a glance, it looks like the implementation above could allocate twice because of the double append(). I know what append() allocates extra space, but I'm not familiar with how exactly it works, especially when appending multiple items in a single call. Are there actually any circumstances where it does two allocations?

Edit: The implementation at the time of writing:

t := s[j:]
s = append(s[:i], v...)
s = append(s, t...)
return s

@carlmjohnson
Copy link
Contributor

carlmjohnson commented Oct 21, 2022

Probably better to take that to the CL comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

5 participants