Non-mutating versions of pop, popfirst, etc. (#66)

[Tokazama]

Some of this would be a lot cleaner if the Static type from #61 were available, but it's at least a start at creating generic versions for #66.

[chriselrod]

I can split off the Static changes into a separate PR if you want.

[Tokazama]

That would be great. Thanks

[Tokazama]

Two concerns at this point:

1. I have only included generic definitions of these and haven't copied over anything defined in StaticArrays. Should I copy those methods over here but for the ArrayInterface version of the methods? Alternatively, this may be a good point to start getting other packages to depend on ArrayInterface directly instead of using Requires.
2. Are the generic versions acceptable?

[chriselrod]

Alternatively, this may be a good point to start getting other packages to depend on ArrayInterface directly instead of using Requires.

I'd be in favor of that.

Are the generic versions acceptable?

Out of curiosity, what is the use case for these non-mutating functions?

pop! and popfirst! return the element you "pop off" the collection, so that one could write a program by pushing tasks/items to a vector, and then popping off work items.
That is, I thought returning the removed value was an important/convenient aspect of the API missing here.

Also, for AbstractVectors, an optimization might be to have pop return views.

If static sizing is involved, then type instability issues will make efficient use of such methods difficult.

It would've been cool if this'd be fast:

Base.@propagate_inbounds popfirst(x::AbstractVector) = (x[begin], @view(x[begin+1:end]))

function mysum(xorig)
    x = @view(xorig[begin:end])
    s = zero(eltype(x))
    @inbounds @fastmath while length(x) > 0
        xᵢ, x = popfirst(x)
        s += xᵢ
    end
    s
end

but it fails to vectorize. Perhaps all the checks in the creation of new views are a problem.
Maybe playing around with it more, e.g. adding a special popfirst method:

Base.@propagate_inbounds function popfirst(x::SubArray{T,1,Vector{T},Tuple{UnitRange{Int64}},true}) where {T}
    inds = first(x.indices)
    xnew = SubArray{T,1,Vector{T},Tuple{UnitRange{Int64}},true}(parent(x), (first(inds)+1:last(inds),), x.offset1, x.stride1)
    x[begin], xnew
end

This does a little better, but still no SIMD.

[Tokazama]

My initial goal here was to present the most generic version of those methods previously defined in StaticArrays as talking points. Here's a more opinionated breakdown:

• insert/deleteat have a similar utility as setindex (i.e. sometimes you want to preserve the original collection but have a separate instance given the mutating outcome). I think these can be pretty useful to have around. I'd be in favor of creating specific methods for tuples if others feel good about including these functions.
• pushfirst/push: I think this is intended to be a type stable version of vcat(item, colletion)/vcat(collection, item) for static vectors. However, I don't see why someone can't just define a type stable version of vcat instead given item should be a known length of 1. It's not even shorter syntax so I'm not sure what the merit really is.
• popfirst/pop: The benefit of this one is that the equivalent type stable version available right now would probably be x[(firstindex(x) - Static(1)):lastindex(x)]/x[firstindex(x):(lastindex(x) + Static(1))]. Which is a bit tedious to type. I've used this a couple of times, but TBH I typically only use pop!/popfirst! methods in interactive programming where type stability isn't important so x[begin:end-1] is probably just as good. I think you touched on the biggest issue that the non mutating version can't produce the "popped" element and the clipped collection in a similar way as the mutating version does. I'd actually like to see Base.front and Base.tail expanded to support ordered collections instead of this approach.

[Tokazama]

I deleted the push/pop related methods for now, as I'm not sure if it's particular helpful to have them at this point. I also added support for tuples on both the deleteat and insert.

I happened upon some bugs with OptionallyStaticUnitRange and while fixing them cleaned up the code a bit. I don't anticipate any problems resulting from those changes but I can revert some of it if it could cause some friction.

[Tokazama]

Should I move the changes to OptionallyStaticUnitRange and Static to a separate PR so that those fixes/changes can move forward?

[chriselrod]

I think this looks good. If you're more or less ready, I can go ahead and merge to save you from splitting off the Static changes.

[chriselrod]

I would split this into separate loops. The compiler may do so automatically, but I'd rather not rely on it.
Something like:

@inbounds for i in firstindex(ret):index-1
    ret[i] = collection[i]
end
@inbounds ret[index] = item
@inbounds for i in index+1:lastindex(ret)
    ret[i] = collection[i-1]
end

to make sure it SIMDs well if possible.

[chriselrod]

Maybe we even want to define aliases like const Zero = Static{0}?

[Tokazama]

I looked at the @code_native output and I think it's working, but TBH I'm still getting used to reading it. I also added the constants Zero and One.

[chriselrod]

I said const Zero = Static{0}(), but what I meant was const Zero = Static{0}, so that we can use ::Zero or ::One in method signatures.

[Tokazama]

Thanks for the feedback! Hopefully, anything else?