slices: add DeleteFunc

[Deleplace]

I propose we add this new func to golang.org/x/exp/slices :

// DeleteFunc removes any elements from s for which del returns true.
// DeleteFunc modifies the contents of the slice s; it does not create a new slice.
// When DeleteFunc removes m elements in total, it might not modify the elements s[len(s)-m:len(s)]. If 
// those elements contain pointers you might consider zeroing those elements so that objects they
// reference can be garbage collected.
func DeleteFunc[S ~[]E, E any](s S, del func(E) bool) S

[Deleplace]

The intent is similar to these existing funcs:

• maps.DeleteFunc
• maps.EqualFunc
• slices.CompactFunc
• slices.CompareFunc
• slices.EqualFunc
• slices.IndexFunc
• slices.IsSortedFunc
• slices.SortFunc

[Deleplace]

In some uncommon cases, it could be useful to know the original index of an element, to decide if it should be removed or not. For this we would need to pass the index as an argument to the del func: del func(int, E) bool.

In my opinion we should not pass such an index because it is ambiguous, as DeleteFunc is destructive and changes the indices. To keep things simple, DeleteFunc would work only when elements are independent from each other, and the indices don't matter.

[earthboundkid]

This is traditionally called Filter or FilterInPlace. The decision was made to deliberately omit map and filter from slices pending discovery of best practices in real experience.

With that said, I think this would be helpful, and maybe even under the name DeleteFunc. Filtering in place without generics is kind of tedious. The idiom I usually use looks like this:

filteredSlice := original[:0]
for _, e := range original {
    if cond(e) {
        filteredSlice = append(filteredSlice, e)
    }
}
original = filteredSlice

It's not so bad, but it's not obvious to people unfamiliar with the idiom how it works, and it's a lot of lines for a simple idea. I would prefer:

slices.DeleteFunc(&original, func(e E) {
    return !cond(e)
})

In terms of the signature, I think DeleteFunc(s *S, del func(E) bool) is better than DeleteFunc(s S, del func(E) bool) S because the original slice isn't very useful after having run DeleteFunc over it, so you have an extra unnecessary assignment. In the case where someone really wants the original slice, they can do filteredSlice := original before calling DeleteFunc. This also better matches maps.DeleteFunc, which does not return the original map.

[Deleplace]

I agree about the original slice not being useful for any common purpose, after having been changed by one of the destructive funcs.

• slices.Compact
• slices.CompactFunc
• slices.Delete
• slices.Insert

all accept S and return S, instead of accepting *S and returning nothing. Maybe they do it that way to mimick append, which is only "half-destructive" (depending on what we want to do with the original slice).

[earthboundkid]

For append, I think it makes sense that it returns a slice instead of taking a pointer because you may want to append to nil or a subslice etc. I don't think it makes as much sense for Compact/Delete/Insert.

[flowchartsman]

Having played around with implementing this generically myself, one thing that occurred to me is that another potential benefit for a generic DeleteFunc might be to go ahead and automatically zero out "deleted" slice elements so that any pointed-to memory can be GCed, avoiding the need to remember the "trick to it".

The simplest way would just be to always zero from len(new) -> len(original), or you could reflect on the elem type and zero out the elements conditionally if that's worth it (not sure, poc: https://go.dev/play/p/foOKrgQ5gxN).

This brings up some more interesting questions. Because, if you have generics, and you want to put generic slice methods in the standard library, of course you would want to do the right thing for your users in one place if the cost isn't too high. For my part, I can definitely say that the documentation for slices.Delete is unsatisfying to me as a Go educator, since it's one more thing to have to explain upfront about a seemingly-simple process, and explaining append is already a heavy lift. It's not intuitive that you can "delete" something, but still have chunks of uncollected memory floating around. Of course this is nothing new, since slice expressions effectively do the same thing, but if you're going to the trouble of making a library for it, might as well have it do something extra. It would certainly add a little something back on the other side of the scale that we lose to the need to call functions like this without a lambda syntax.

Perhaps this is an argument for Delete(in-place, reflective) and Filter(copy), or perhaps it's an argument that copying is the more natural thing, and we should just always copy values and live with that. That's certainly the more conservative, Go-y thing to do, but we are also in a new generic world, and that brings a lot of opportunity for new conventions. It may even be that it will be perfectly fine to have something like DeleteFunc(s *S, del func(E) bool), though it does feel a little unnatural to me right now.

I think, I'd vote for either

• give Delete and DeleteFunc the ability to zero out old capacity or
• don't bother with them at all, and only copy.

[earthboundkid]

You may be interested in #56351 which proposes to add a clear function which would remove all elements from a map and set all values in a slice to the zero value.

[flowchartsman]

Yes, I think DeleteFunc should probably call that behind the scenes, since there's no reasonable way to zero out the elements yourself unless you keep the original slice and also return the range of indices of the leftover len() after all of the shuffling has completed.

[Deleplace]

I agree with the sentiment that slices.Delete, slices.DeleteFunc, slices.Compact, slices.CompactFunc, slices.Clip would all be more useful and less cryptic if they did the extra work of zeroing out the discarded right tail of the original slice. I suggested such behavior but got strong pushback.

@flowchartsman the range of the leftover is always len(new):len(old) but yes leaving this work as an exercise to the developer at call site is not great in my opinion.