RFC: use AbstractMatrix as the eltype of Symmetric for array of arrays

[JeffBezanson]

This fixes a crash in the StaticArrays tests on 1.2 and master. It can be reduced to this:

julia> using LinearAlgebra

julia> a = Hermitian([[rand(Complex{Int}) for i = 1 : 2, j = 1 : 2] for row = 1 : 3, col = 1 : 3]);

julia> transpose(a);

The functions symmetric_type and hermitian_type are involved; they are both recursive and call return_type and it seems we just cannot handle it. The eltype of a is currently Union{Array{Complex{Int64},2}, Adjoint{Complex{Int64},Array{Complex{Int64},2}}, Hermitian{Complex{Int64},Array{Complex{Int64},2}}}, which does not seem too useful to me. Here I switch it to AbstractMatrix instead, which works around the problem.

An alternate possible fix is to reduce the complexity of Symmetric and Hermitian getindex. They currently return three different types for diagonal, upper, and lower. I can also fix the crash by reducing this to two. My proposal is to materialize the recursive Symmetric/Hermitian array, removing the third type from Union{S, promote_op(transpose, S), symmetric_type(S)}. That gives a bit more precise type info, but would be more breaking since it changes what getindex returns, while this PR only changes the element type of the Symmetric/Hermitian wrapper.

Any way we can avoid promote_op would be even better, and would also fix it. For example I'd use typeof(transpose(a[1])), and Union{} for empty arrays. But that would break the symmetric_type API, which expects to be able to compute the eltype from just a type.

[JeffBezanson]

I haven't yet figured out why this is crashing (on master, not with this PR). Looking at the exact called objects in gdb, transpose(a) is:

Core.CodeInfo(code=Array{Any, (2,)}[
  Expr(:invoke, (::Type{LinearAlgebra.Transpose{T, S} where S where T})(LinearAlgebra.Hermitian{Union{Array{Base.Complex{Int64}, 2}, LinearAlgebra.Hermitian{Base.Complex{Int64}, Array{Base.Complex{Int64}, 2}}}, Array{Array{Base.Complex{Int64}, 2}, 2}}), LinearAlgebra.Transpose, Core.SlotNumber(id=2)),
  Expr(:unreachable)]

and the MethodInstance/CodeInstance (there's only one inside) called by the invoke is:

  Expr(:new, LinearAlgebra.Transpose{Union{LinearAlgebra.Transpose{Base.Complex{Int64}, Array{Base.Complex{Int64}, 2}}, LinearAlgebra.Transpose{Base.Complex{Int64}, LinearAlgebra.Hermitian{Base.Complex{Int64}, Array{Base.Complex{Int64}, 2}}}}, LinearAlgebra.Hermitian{Union{Array{Base.Complex{Int64}, 2}, LinearAlgebra.Hermitian{Base.Complex{Int64}, Array{Base.Complex{Int64}, 2}}}, Array{Array{Base.Complex{Int64}, 2}, 2}}}, Core.SlotNumber(id=2)),
  Expr(:return, SSAValue(1))]

and it has the expected return type. The type of the invoke in the first function is Union{} though.

[c42f]

Thanks Jeff. Not knowing much about inference I don't immediately have a lot to add here. The workaround looks reasonable as far as it goes. It would be nice to know the root cause.

Just in case anyone is confused by StaticArrays now passing on 1.2, it's because I added a @skip_test for this case because I'd forgotten about this potential workaround.