Use Base.promote instead of promote_tuple_eltype to fik promotion
...issue for element types defined after StaticArrays.
Currently, we have this behavior
julia> struct MyFloat64 <: Real
x::Float64
end
julia> Base.promote_rule(::Type{Float64}, ::Type{MyFloat64}) = MyFloat64
julia> SArray{Tuple{2}}((2.0, MyFloat64(1.0)))
2-element SArray{Tuple{2},Real,1,2} with indices SOneTo(2):
2.0
MyFloat64(1.0)
probably because the generated function promote_tuple_eltype can't handle types defined later than itself. With this PR, the example gives
julia> SArray{Tuple{2}}((2.0, MyFloat64(1.0)))
2-element SArray{Tuple{2},MyFloat64,1,2} with indices SOneTo(2):
MyFloat64(2.0)
MyFloat64(1.0)
The PR generally reduces the number of generated functions.
A remaining issue is https://github.com/JuliaArrays/StaticArrays.jl/blob/4cfeb5859235c67bd0e0ca378d045e135fd32921/test/SHermitianCompact.jl#L228 because there is no promote_rule between the matrix valued elements. @tkoolen Any thoughts on the better way to fix this case?
Cool. Did you look into performance of promote(x...)?
Interesting corner cases to consider include Union element types, or longer tuples, or both. Certainly we wouldn't want generated code to regress for things like SVector((x::Float64, y::Float64, 0::Int)) on any of the Julia 1.0, 1.1, 1.2, or 1.3 series compilers.
probably because the generated function
promote_tuple_eltypecan't handle types defined later than itself.
This is confusing, right? The function generator doesn't currently call promote_type at all... it generates code that can call the latest version by simply expanding out the tuple's type parameters. Does anyone understand what might be going on there?
Cool. Did you look into performance of
promote(x...)?
I did some timings and didn't see any regressions but timing StaticArrays code is always a bit tricky. I guess the most important thing is that I didn't see any new allocations. I assumed that the old issue that made you promote_tuple_eltype would show up as @inferred failures. Is that correct?
Coverage decreased (-0.4%) to 81.158% when pulling c72f42f6c2929f14117b6c84f3d163dfa80a4ee1 on an/promote into 4cfeb5859235c67bd0e0ca378d045e135fd32921 on master.
Doing some archeology, the origin of promote_tuple_eltype was in the very first StaticArrays code commit cef8abc (under then name promote_eltype). That might even have been prior to julia 0.5 so it seems very likely we don't need this hack anymore. But I also don't know exactly what caused Andy to put it in...
lol, nice archaeology. Constructors were done first - promotion behaving like other arrays obviously being pretty important. How time flies :)
In any case we should double check some generated (native) code first but in general I am in favour of removing generated code.
This seems to work, so I guess this can be closed @andreasnoack?
I think the consensus was in favor of this PR even though the motivating example was fixed in my other PR.