Fix Base.OneTo(::StaticInt)

[cscherrer]

Fixes #40

This is not yet ready to go, because

julia> collect(Base.OneTo(static(3)))
ERROR: TypeError: in typeassert, expected StaticInt{3}, got a value of type StaticInt{1}
Stacktrace:
 [1] setindex!(A::Vector{StaticInt{3}}, x::StaticInt{1}, i1::Int64)
   @ Base ./array.jl:903
 [2] vcat(rs::Base.OneTo{StaticInt{3}})
   @ Base ./range.jl:1280
 [3] collect(r::Base.OneTo{StaticInt{3}})
   @ Base ./range.jl:1287
 [4] top-level scope
   @ REPL[9]:1

[codecov]

Codecov Report

Merging #41 (d024b81) into master (1566c58) will decrease coverage by 0.20%.
The diff coverage is 0.00%.

❗ Current head d024b81 differs from pull request most recent head 4d4030d. Consider uploading reports for the commit 4d4030d to get more accurate results

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
- Coverage   98.11%   97.91%   -0.21%     
==========================================
  Files           8        8              
  Lines         478      479       +1     
==========================================
  Hits          469      469              
- Misses          9       10       +1     

Impacted Files	Coverage Δ
src/int.jl	98.79% <0.00%> (-1.21%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1566c58...4d4030d. Read the comment docs.

[chriselrod]

The problem is

function vcat(rs::AbstractRange{T}...) where T
    n::Int = 0
    for ra in rs
        n += length(ra)
    end
    a = Vector{T}(undef, n)
    i = 1
    for ra in rs, x in ra
        @inbounds a[i] = x
        i += 1
    end
    return a
end

[Tokazama]

Seems odd to overwrite and arrays eltype. Would it make more sense to do Base.OneTo(n::StaticInt) = Base.Oneto(Int(n)) makes more sense to me?

[cscherrer]

I was hoping to get to a statically-sized interval. The benefit of Base.OneTo(n) instead of 1:n is that the one is known statically. It seems strange to me to reverse this pattern and have Base.OneTo(static(3)) be more dynamic than static(1):static(3)

[cscherrer]

Oh but this isn't good

julia> myrange = Base.OneTo(static(3))
Base.OneTo(static(3))

julia> typeof(myrange)
OneTo{StaticInt{3}}

julia> supertype(ans)
AbstractUnitRange{StaticInt{3}}

I think the right solution might be to create a new range type allowing start and end to have different types, and make things like Base.OneTo(static(3)) and static(1):static(3) call that

[cscherrer]

Hmm, it looks like ArrayInterface also covers this, with lots of type piracy. Is there a reason this code should be there instead of here?
https://github.com/JuliaArrays/ArrayInterface.jl/blob/master/src/ranges.jl

[Tokazama]

I just haven't figured out how to cleanly disentangle those types from methods specific to ArrayInterface.

[cscherrer]

From this

Base.:(:)(L::Integer, ::StaticInt{U}) where {U} = OptionallyStaticUnitRange(L, StaticInt(U))
Base.:(:)(::StaticInt{L}, U::Integer) where {L} = OptionallyStaticUnitRange(StaticInt(L), U)
function Base.:(:)(::StaticInt{L}, ::StaticInt{U}) where {L,U}
    return OptionallyStaticUnitRange(StaticInt(L), StaticInt(U))
end

I'd think OptionallyStaticUnitRange should move to this package.

Maybe we do that, and then make

Base.OneTo(s::StaticInt{N}) where {N} = static(1):s

What do you think?

[Tokazama]

I think it makes total sense to have those defined in the same place. But then we also need to move OptionallyStaticStepRange too and I'm not sure what we should do about code like this:

@propagate_inbounds function Base.getindex(
    r::OptionallyStaticUnitRange,
    s::AbstractUnitRange{<:Integer},
)
    @boundscheck checkbounds(r, s)
    f = ArrayInterface.static_first(r)
    fnew = f - one(f)
    return (fnew+ArrayInterface.static_first(s)):(fnew+ArrayInterface.static_last(s))
end

[cscherrer]

Why not have static_first in Static? If you keep pulling the thread, I guess the only real weirdness is if you'd need to move a function to Static, but

• That function depends on some ArrayInterface.foo, and
• foo is (and should be) exported from ArrayInterface, and
• foo is array-related, but has nothing to do with static values

Does the refactoring ever hit a point like this?

[Tokazama]

That's exactly what I ran into when I tried to hash this out one afternoon. I was actually trying to do a PR to Base and then realized that I didn't have the time to really sell StaticInt along with everything else. This JuliaArrays/ArrayInterface.jl#211 (comment) recent comment briefly tells why I've dragged my feet on figuring out where to put everything.

[cscherrer]

I don't want to derail that discussion, but this is painful:

The argument was that things like constant propagation are the job of the optimizer, which Julia and LLVM can do very efficiently, while the type system does not.
It is thus an abuse of the type system, using it for something it was not designed or optimized for.

I believe it, but... ouch. We need a way to be sure some things are evaluated statically, and the type system (with generated functions) seems like the only way to go. I had assumed this would become idiomatic and gain support - the alternative is kind of scary.

[Tokazama]

As I later comment in that issue, I think it's unclear what the implications of that comment are for developers. Even if we should be able to propagate size information, what if you want to specialize on 4x4 arrays? If that's not a trait then is it supposed to be an implementation as an additional pass in the compiler? That's gonna be rough for anyone that wants to write very exact matrix manipulations without spending some serious time learning how the compiler works.

[Tokazama]

@cscherrer we have proper range construction here now. Does that solve the issue motivating this PR