Fix accidential API breakage in one() (#763)

It turns out that Rotations relies on one(M) calling the constructor of
M, so transitioning to use of simliar_type in #690 broke Rotations.

It's arguably a bug to call the constructor rather than calling
similar_type, but let's avoid breaking compatibility for now.

(It's a bug because some sets of matrices defined by a type cannot
represent the multiplicative identity, for example the set of
skew-symmetric matrices. StaticArrays should likely assume that generic
operations like one() must devolve to a matrix type which can represent
anything in GL(n,R). For example, SMatrix.)

Author: c42f

src/linalg.jl

@@ -87,30 +87,49 @@ end
 @inline Base.zero(a::SA) where {SA <: StaticArray} = zeros(SA)
 @inline Base.zero(a::Type{SA}) where {SA <: StaticArray} = zeros(SA)
 
+@inline _construct_sametype(a::Type, elements) = a(elements)
+@inline _construct_sametype(a, elements) = typeof(a)(elements)
+
 @inline one(m::StaticMatrixLike) = _one(Size(m), m)
 @inline one(::Type{SM}) where {SM<:StaticMatrixLike}= _one(Size(SM), SM)
 function _one(s::Size, m_or_SM)
     if (length(s) != 2) || (s[1] != s[2])
         throw(DimensionMismatch("multiplicative identity defined only for square matrices"))
     end
-    _scalar_matrix(s, m_or_SM, one(_eltype_or(m_or_SM, Float64)))
+    λ = one(_eltype_or(m_or_SM, Float64))
+    _construct_sametype(m_or_SM, _scalar_matrix_elements(s, λ))
+    # TODO: Bring back the use of _construct_similar here:
+    # _construct_similar(m_or_SM, s, _scalar_matrix_elements(s, λ))
+    #
+    # However, because _construct_similar uses `similar_type`, it will be
+    # breaking for some StaticMatrix types (in particular Rotations.RotMatrix)
+    # which must have similar_type return a general type able to hold any
+    # matrix in the full general linear group.
+    #
+    # (Generally we're stuck here and things like RotMatrix really need to
+    # override one() and zero() themselves: on the one hand, one(RotMatrix)
+    # should return a RotMatrix, but zero(RotMatrix) can not be a RotMatrix.
+    # The best StaticArrays can do is to use similar_type to return an SMatrix
+    # for both of these, and let the more specialized library define the
+    # correct algebraic properties.)
 end
 
 # StaticMatrix(I::UniformScaling)
-(::Type{SM})(I::UniformScaling) where {SM<:StaticMatrix} = _scalar_matrix(Size(SM), SM, I.λ)
+(::Type{SM})(I::UniformScaling) where {SM<:StaticMatrix} =
+    SM(_scalar_matrix_elements(Size(SM), I.λ))
 # The following oddity is needed if we want `SArray{Tuple{2,3}}(I)` to work
 # because we do not have `SArray{Tuple{2,3}} <: StaticMatrix`.
 (::Type{SM})(I::UniformScaling) where {SM<:(StaticArray{Tuple{N,M}} where {N,M})} =
-    _scalar_matrix(Size(SM), SM, I.λ)
+    SM(_scalar_matrix_elements(Size(SM), I.λ))
 
 # Construct a matrix with the scalar λ on the diagonal and zeros off the
 # diagonal. The matrix can be non-square.
-@generated function _scalar_matrix(s::Size{S}, m_or_SM, λ) where {S}
+@generated function _scalar_matrix_elements(s::Size{S}, λ) where {S}
     elements = Symbol[i == j ? :λ : :λzero for i in 1:S[1], j in 1:S[2]]
     return quote
         $(Expr(:meta, :inline))
         λzero = zero(λ)
-        _construct_similar(m_or_SM, s, tuple($(elements...)))
+        tuple($(elements...))
     end
 end
 

test/linalg.jl

@@ -2,6 +2,14 @@ using StaticArrays, Test, LinearAlgebra
 
 using LinearAlgebra: checksquare
 
+# For one() test
+struct RotMat2 <: StaticMatrix{2,2,Float64}
+    elements::NTuple{4,Float64}
+end
+Base.getindex(m::RotMat2, i::Int) = getindex(m.elements, i)
+# Rotation matrices must be unitary so `similar_type` has to return an SMatrix.
+StaticArrays.similar_type(::Union{RotMat2,Type{RotMat2}}) = SMatrix{2,2,Float64,4}
+
 @testset "Linear algebra" begin
 
     @testset "SArray as a (mathematical) vector space" begin
@@ -103,6 +111,11 @@ using LinearAlgebra: checksquare
         @test @inferred(one(MMatrix{2}))::MMatrix == @MMatrix [1.0 0.0; 0.0 1.0]
 
         @test_throws DimensionMismatch one(MMatrix{2,4})
+
+        @test one(RotMat2) isa RotMat2
+        @test one(RotMat2) == SA[1 0; 0 1]
+        # TODO: See comment in _one.
+        @test_broken one(RotMat2) isa SMatrix{2,2,Float64}
     end
 
     @testset "cross()" begin