Unnecessary `Size` definitions?

[jipolanco]

Hi, thank you for this very useful package!

I was curious about the definitions of StaticArrays.Size in core_types.jl and rotation_generator.jl:

Base.@pure StaticArrays.Size(::Type{Rotation{N}}) where {N} = Size(N,N)
Base.@pure StaticArrays.Size(::Type{Rotation{N,T}}) where {N,T} = Size(N,N)
Base.@pure StaticArrays.Size(::Type{R}) where {R<:Rotation} = Size(supertype(R))

Base.@pure StaticArrays.Size(::Type{RotationGenerator{N}}) where {N} = Size(N,N)
Base.@pure StaticArrays.Size(::Type{RotationGenerator{N,T}}) where {N,T} = Size(N,N)
Base.@pure StaticArrays.Size(::Type{R}) where {R<:RotationGenerator} = Size(supertype(R))

Are these necessary? Given that Rotation{N,T} <: StaticMatrix{N,N,T} (same for RotationGenerator), it seems to me that one can just fall back to the default definitions in StaticArrays(Core). I checked that tests pass when these definitions are removed.

It turns out that these definitions cause invalidation of StaticArrays methods according to SnoopCompile:

julia> using SnoopCompile

julia> using StaticArrays

julia> invalidations = @snoopr using Rotations;

julia> trees = invalidation_trees(invalidations)
1-element Vector{SnoopCompile.MethodInvalidations}:
 inserting Size(::Type{R}) where R<:Rotation @ Rotations ~/.julia/dev/Rotations/src/core_types.jl:11 invalidated:
   backedges: 1: superseding Size(::Type{SA}) where {S<:Tuple, SA<:(StaticArray{S})} @ StaticArraysCore ~/.julia/packages/StaticArraysCore/P6PH7/src/StaticArraysCore.jl:499 with MethodInstance for Size(::Type{SA}) where {S<:Tuple, SA<:(StaticArray{S, Bool})} (1 children)
              2: superseding Size(::Type{SA}) where SA<:StaticArray @ StaticArraysCore ~/.julia/packages/StaticArraysCore/P6PH7/src/StaticArraysCore.jl:498 with MethodInstance for Size(::Type{T} where T<:(StaticArray{<:Tuple, Bool})) (6 children)

I also noticed that one of these definitions causes JET's @report_call to detect an obscure issue when trying to solve a linear system with StaticArrays (I spent a bit of time trying to understand where this was coming from...). These operations are completely unrelated to Rotations.jl, but the issue only happens when Rotations.jl is loaded:

using StaticArrays
using Rotations  # causes error in @report_call
using JET
using LinearAlgebra

A = @SArray rand(2, 2)
b = @SArray rand(2, 3)
A \ b               # OK
@report_opt A \ b   # OK
@report_call A \ b  # error!

═════ 4 possible errors found ═════
┌ \(a::SMatrix{2, 2, Float64, 4}, b::SMatrix{2, 3, Float64, 6}) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/solve.jl:1
│┌ _solve(::Size{(2, 2)}, ::Size{(2, 3)}, a::SMatrix{2, 2, Float64, 4}, b::SMatrix{2, 3, Float64, 6}) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/solve.jl:38
││┌ _solve(::Size{(2, 2)}, ::Size, a::SMatrix{2, 2, Float64, 4}, b::StaticArray) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/solve.jl:38
│││┌ getindex(::StaticArray, ::Colon, ::Union{Colon, Int64, SOneTo, StaticArray{<:Tuple, Int64}}) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/indexing.jl:260
││││┌ index_sizes(::Int64, ::Colon, ::Colon) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/indexing.jl:84
│││││ no matching method found `index_sizes(::Colon, ::Colon)`: StaticArrays.index_sizes(inds::Tuple{Colon, Colon}...)
││││└────────────────────
││││┌ index_sizes(::Int64, ::Colon, ::Int64) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/indexing.jl:84
│││││ no matching method found `index_sizes(::Colon, ::Int64)`: StaticArrays.index_sizes(inds::Tuple{Colon, Int64}...)
││││└────────────────────
││││┌ index_sizes(::Int64, ::Colon, ::SOneTo) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/indexing.jl:84
│││││ no matching method found `index_sizes(::Colon, ::SOneTo)`: StaticArrays.index_sizes(inds::Tuple{Colon, SOneTo}...)
││││└────────────────────
││││┌ index_sizes(::Int64, ::Colon, ::StaticArray{<:Tuple, Int64}) @ StaticArrays /home/polanco2ju/.julia/packages/StaticArrays/oOCPP/src/indexing.jl:84
│││││ no matching method found `index_sizes(::Colon, ::StaticArray{<:Tuple, Int64})`: StaticArrays.index_sizes(inds::Tuple{Colon, StaticArray{<:Tuple, Int64}}...)

From what I understand, this seems to be because the compiler (or JET?) can no longer be sure whether calling Size(...) actually returns a StaticArrays.Size or something else (like a Colon()).

So my question is, would it make sense to remove these Size definitions?

[hyrodium]

Thank you for the issue! I agree with all of your points. Could you create a PR to remove these Size methods?

[jipolanco]

Sure, I'll create a PR!