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: slices: functions Shift, Rotate. #69808

Open
markusheukelom opened this issue Oct 8, 2024 · 10 comments
Open

proposal: slices: functions Shift, Rotate. #69808

markusheukelom opened this issue Oct 8, 2024 · 10 comments
Labels
Milestone

Comments

@markusheukelom
Copy link

markusheukelom commented Oct 8, 2024

Proposal Details

// Shift return a slice with n zero values of E inserted at the given position.
func Shift[S ~[]E, E any](s S, at, n int) S 

// Rotate right-rotates the slice by n places. To left-rotate use -n.
func Rotate[S ~[]E, E any](s S, n int) S 

Judging from the source code of the slices package there already is an internal "rotateLeft/Right" function. I am unsure if there is a good reason not to export this useful function (especially as it is not trivial to implement without allocations).

A "Shift" function is not trivial to implement efficiently either, I think (without intermediate allocations, ie. making an zero slice of length and inserting it) . Maybe this can be done using other functions in the package that I am unaware of. In that case just an example would be nice.

(Looking at the implementation of Insert I think a Shift function could simplify its implementation a little. Also Insert checks for overlap but this seems covered by copy already? Although maybe there's more to it then I can see by glancing at it).

Updates after feedback

Apparently shift is commonly used to refer to a different operation on arrays/slices (especially in scripting languages) and bits. Alternative names for Shift such as InsertN or InsertZeros should be considered.

@gopherbot gopherbot added this to the Proposal milestone Oct 8, 2024
@gabyhelp
Copy link

gabyhelp commented Oct 8, 2024

Related Issues and Documentation

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao
Copy link
Member

Duplicate of #64103

@seankhliao seankhliao marked this as a duplicate of #64103 Oct 8, 2024
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2024
@randall77
Copy link
Contributor

The Shift proposed here is different from the Shift proposed in #64103.

@randall77 randall77 reopened this Oct 8, 2024
@seankhliao
Copy link
Member

Ah, sorry, though I struggle to see how useful this definition of Shift is in common code?

@randall77
Copy link
Contributor

I agree, I'm not sure I understand where these would be useful.
(And I wrote slices.rotateLeft/Right :) )

@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Oct 8, 2024
@markusheukelom
Copy link
Author

markusheukelom commented Oct 9, 2024

@randall77 @seankhliao

Regarding usefulness in "common code", here's what I would comment:

Shift is useful because you can make room to insert values into a slice without having to allocate an intermediate slice. This saves an allocation. Say for example I have n slices that I want to insert into some other slice. With Shift I could just make room for those slices and then use Replace to copy the values. But with Insert I have to allocate an intermediate slice using Concat which I then insert using Insert.

Rotate is useful whenever a slice is used to represent a loop-like (data)structure (such as polygons, in my case).

(notice that Shift, Rotate also seem useful in the slices package implementation itself)

If that doesn't make the cut, that's understandable of course. Maybe it needs more evidence of usefulness.

@DeedleFake
Copy link

Why limit Shift() to just zero values? How about

func InsertN[Slice ~[]E, E any](s Slice, i, n int, v E) Slice

instead?

@markusheukelom
Copy link
Author

@DeedleFake

I don't think there's a need, evidenced by the fact that make([]T) supports zero values only as well. Plus, it's trivial to set a value if needed (or maybe a slices.Set function). Then again, I wouldn't mind either.

@earthboundkid
Copy link
Contributor

Not commenting on the rest of the proposals, but Shift is absolutely the wrong name for what's being proposed. Shift is a very common name for popleft: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/shift #64103. This is something else. ISTM if you can do "shift" as just a rotate on a subslice, so it doesn't need its own operation.

@markusheukelom
Copy link
Author

@earthboundkid Thanks, I've added a remark in the proposal regarding the name.

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

No branches or pull requests

7 participants